[PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[Tomi Ollila]

From: Tomi Ollila <too@iki.fi>

When shell executes background process using '&' the scheduling of
that new process is arbitrary. It could be that smtp-dummy doesn't
get execution time to listen() it's server socket until some other
process attempts to connect() to it. The --background option in
smtp-dummy makes it to go background *after* it started to listen
its server socket.

When --background option is used, the line "smtp_dummy_pid='<pid>'"
is printed to stdout from where shell can eval it.
---

This is v3 of id:"1323766883-17607-1-git-send-email-tomi.ollila@iki.fi"

addressing (some) Dmitry's comments.

 test/smtp-dummy.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 86d4316..bb13668 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -119,6 +119,7 @@ do_smtp_to_file (FILE *peer, FILE *output)
 int
 main (int argc, char *argv[])
 {
+	const char * progname;
 	char *output_filename;
 	FILE *peer_file, *output;
 	int sock, peer, err;
@@ -126,9 +127,31 @@ main (int argc, char *argv[])
 	struct hostent *hostinfo;
 	socklen_t peer_addr_len;
 	int reuse;
+	int background;
+
+	progname = argv[0];
+
+	background = 0;
+	for (; argc >= 2; argc--, argv++) {
+		if (argv[1][0] != '-')
+			break;
+		if (strcmp (argv[1], "--") == 0) {
+			argc--;
+			argv++;
+			break;
+		}
+		if (strcmp (argv[1], "--background") == 0) {
+			background = 1;
+			continue;
+		}
+		fprintf(stderr, "%s: unregognized option '%s'\n",
+			progname, argv[1]);
+		return 1;
+	}
 
 	if (argc != 2) {
-		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
+		fprintf (stderr,
+			 "Usage: %s [--background] <output-file>\n", progname);
 		return 1;
 	}
 
@@ -181,6 +204,36 @@ main (int argc, char *argv[])
 		return 1;
 	}
 
+	if (background) {
+		int pid = fork ();
+		if (pid > 0) {
+			printf ("smtp_dummy_pid='%d'\n", pid);
+			fflush (stdout);
+			close (sock);
+			return 0;
+		}
+		if (pid < 0) {
+			fprintf (stderr, "Error: fork() failed: %s\n",
+				 strerror (errno));
+			close (sock);
+			return 1;
+		}
+		/* Reached if pid == 0 (the child process). */
+		/* Close stdout so that the one interested in pid value will
+		   also get EOF. */
+		close (STDOUT_FILENO);
+		/* dup2() will re-reserve fd of stdout (1) (opportunistically),
+		   in case fd of stderr (2) is open. If that was not open we
+		   don't care fd of stdout (1) either. */
+		dup2 (STDERR_FILENO, STDOUT_FILENO);
+
+		/* This process is now out of reach of shell's job control.
+		   To resolve the rare but possible condition where this
+		   "daemon" is started but never connected this process will
+		   (only) have 30 seconds to exist. */
+		alarm (30);
+	}
+
 	peer_addr_len = sizeof (peer_addr);
 	peer = accept (sock, (struct sockaddr *) &peer_addr, &peer_addr_len);
 	if (peer == -1) {
-- 
1.7.11.4

[PATCH V3 2/2] test/test-lib.sh: take the --background feature in smtp-dummy into use

[Tomi Ollila]

From: Tomi Ollila <too@iki.fi>

The use of --background option (instead of shell '&') ensures that
smtp-dummy is listening its server socket until execution of shell
script can continue, thus the client will always have socket where
to connect.

smtp-dummy outputs smtp_dummy_pid variable in shell assignment format;
eval'ing that output makes that variable available for the shell.

As the smtp-dummy instance is no longer child process of the script
the SIGKILL signal sent to it will ensure it is going away in case
the mail sender fails to connect to smtp-dummy.
---
 test/test-lib.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f34b1fb..c902b2c 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -403,8 +403,11 @@ emacs_deliver_message ()
     shift 2
     # before we can send a message, we have to prepare the FCC maildir
     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
-    $TEST_DIRECTORY/smtp-dummy sent_message &
-    smtp_dummy_pid=$!
+    # eval'ing smtp-dummy --background will set smtp_dummy_pid
+    smtp_dummy_pid=
+    eval `$TEST_DIRECTORY/smtp-dummy --background sent_message`
+    test -n "$smtp_dummy_pid" || return 1
+
     test_emacs \
 	"(let ((message-send-mail-function 'message-smtpmail-send-it)
 	       (smtpmail-smtp-server \"localhost\")
@@ -419,9 +422,11 @@ emacs_deliver_message ()
 	   (insert \"${body}\")
 	   $@
 	   (message-send-and-exit))"
-    # opportunistically quit smtp-dummy in case above fails.
-    { echo QUIT > /dev/tcp/localhost/25025; } 2>/dev/null
-    wait ${smtp_dummy_pid}
+
+    # In case message was sent properly, client waits for confirmation
+    # before exiting and resuming control here; therefore making sure
+    # that server exits by sending (KILL) signal to it is safe.
+    kill -9 $smtp_dummy_pid
     notmuch new >/dev/null
 }
 
-- 
1.7.11.4

Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[Michal Sojka]

Hi Tomi,

On Tue, Sep 18 2012, Tomi Ollila wrote:
> From: Tomi Ollila <too@iki.fi>
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.
>
> When --background option is used, the line "smtp_dummy_pid='<pid>'"
> is printed to stdout from where shell can eval it.
> ---
>
> This is v3 of id:"1323766883-17607-1-git-send-email-tomi.ollila@iki.fi"
>
> addressing (some) Dmitry's comments.
>
>  test/smtp-dummy.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
> index 86d4316..bb13668 100644
> --- a/test/smtp-dummy.c
> +++ b/test/smtp-dummy.c
> @@ -119,6 +119,7 @@ do_smtp_to_file (FILE *peer, FILE *output)
>  int
>  main (int argc, char *argv[])
>  {
> +	const char * progname;
>  	char *output_filename;
>  	FILE *peer_file, *output;
>  	int sock, peer, err;
> @@ -126,9 +127,31 @@ main (int argc, char *argv[])
>  	struct hostent *hostinfo;
>  	socklen_t peer_addr_len;
>  	int reuse;
> +	int background;
> +
> +	progname = argv[0];
> +
> +	background = 0;
> +	for (; argc >= 2; argc--, argv++) {
> +		if (argv[1][0] != '-')

This is ugly, but all right. I do not think we need to have high
standards for this piece of code :)

> +			break;
> +		if (strcmp (argv[1], "--") == 0) {
> +			argc--;
> +			argv++;
> +			break;
> +		}
> +		if (strcmp (argv[1], "--background") == 0) {
> +			background = 1;
> +			continue;
> +		}
> +		fprintf(stderr, "%s: unregognized option '%s'\n",
> +			progname, argv[1]);
> +		return 1;
> +	}
>
>  	if (argc != 2) {
> -		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
> +		fprintf (stderr,
> +			 "Usage: %s [--background] <output-file>\n", progname);
>  		return 1;
>  	}
>
> @@ -181,6 +204,36 @@ main (int argc, char *argv[])
>  		return 1;
>  	}
>
> +	if (background) {
> +		int pid = fork ();
> +		if (pid > 0) {
> +			printf ("smtp_dummy_pid='%d'\n", pid);
> +			fflush (stdout);
> +			close (sock);
> +			return 0;
> +		}
> +		if (pid < 0) {
> +			fprintf (stderr, "Error: fork() failed: %s\n",
> +				 strerror (errno));
> +			close (sock);
> +			return 1;
> +		}
> +		/* Reached if pid == 0 (the child process). */
> +		/* Close stdout so that the one interested in pid value will
> +		   also get EOF. */
> +		close (STDOUT_FILENO);
> +		/* dup2() will re-reserve fd of stdout (1) (opportunistically),
> +		   in case fd of stderr (2) is open. If that was not open we
> +		   don't care fd of stdout (1) either. */
> +		dup2 (STDERR_FILENO, STDOUT_FILENO);
> +
> +		/* This process is now out of reach of shell's job control.
> +		   To resolve the rare but possible condition where this
> +		   "daemon" is started but never connected this process will
> +		   (only) have 30 seconds to exist. */
> +		alarm (30);

30 seconds is probably enough. If it turns out that we need longer
timeout in some scenario, I'd suggest to call alarm() also when
something is received on the socket to extend the timeout.

Overall, both patches LGTM.

-Michal

Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[Ethan Glasser-Camp]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> From: Tomi Ollila <too@iki.fi>
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

Ethan

Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> From: Tomi Ollila <too@iki.fi>
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.

series pushed

d