[PATCH] test/smtp-dummy: add --background option for backgrounding after listen(2)

[PATCH] test/smtp-dummy: add --background option for backgrounding after listen(2)

[Tomi Ollila]

To avoid the possibility that smtp-dummy doesn't have chance to listen
its bound socket until something tries to send message to it this
option can be used for caller to wait until socket is already listening
for a connection.
---

When test_require_external_prereq() is changed to use associative
array to check whether prereq is missing bash script is often 
so fast that it already waiting for smtp-dummy to exit until
socket is listening. i.e sending QUIT to 127.0.0.1:25025 fails
since there is no listener. (fork(2) & execve(2) count goes to zero
in test_require_external_prereq (from 3 fork()s, 2 execve()s).

 test/smtp-dummy.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 3801a5e..40196dd 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -124,9 +124,21 @@ main (int argc, char *argv[])
 	struct hostent *hostinfo;
 	socklen_t peer_addr_len;
 	int reuse;
+	int bg;
+
+	/* XXX quick implementation -- fix if more functionality is desired. */
+	if (argc >= 2 && strcmp(argv[1], "--background") == 0) {
+		argc--;
+		argv[1] = argv[0];
+		argv++;
+		bg = 1;
+	}
+	else
+		bg = 0;
 
 	if (argc != 2) {
-		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
+		fprintf (stderr, "Usage: %s [--background] <output-file>\n",
+			 argv[0]);
 		return 1;
 	}
 
@@ -179,6 +191,20 @@ main (int argc, char *argv[])
 		return 1;
 	}
 
+	if (bg) {
+	    switch (fork ()) {
+	    case 0:
+		break;
+	    case -1:
+		fprintf (stderr, "Error: fork() failed: %s\n",
+			 strerror (errno));
+		close (sock);
+		return 1;
+	    default:
+		return 0;
+	    }
+	}
+
 	peer_addr_len = sizeof (peer_addr);
 	peer = accept (sock, (struct sockaddr *) &peer_addr, &peer_addr_len);
 	if (peer == -1) {
-- 
1.7.7.3

Re: [PATCH] test/smtp-dummy: add --background option for backgrounding after listen(2)

[Austin Clements]

Quoth Tomi Ollila on Dec 12 at  4:57 pm:
> To avoid the possibility that smtp-dummy doesn't have chance to listen
> its bound socket until something tries to send message to it this
> option can be used for caller to wait until socket is already listening
> for a connection.
> ---
> 
> When test_require_external_prereq() is changed to use associative
> array to check whether prereq is missing bash script is often 
> so fast that it already waiting for smtp-dummy to exit until
> socket is listening. i.e sending QUIT to 127.0.0.1:25025 fails
> since there is no listener. (fork(2) & execve(2) count goes to zero
> in test_require_external_prereq (from 3 fork()s, 2 execve()s).

Perhaps smtp-dummy should always work this way?  There's a race even
in the working case, since Emacs could try to connect to the
smtp-dummy before it's listening (perhaps Emacs retries in that case,
but I couldn't find any code to do so).

How are you going to wait on the dummy, since you don't know its PID
any more?

[PATCH 1/2] test/smtp-dummy: add --background option for going background after listen(2)

[Tomi Ollila]

To avoid the possibility that smtp-dummy doesn't have chance to bind
its listening socket until something tries to send message to it this
option makes caller wait until socket is already listening for connections.

In case this --background option is used, the pid of running smtp-dummy
is printed on stdout.
---
 test/smtp-dummy.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 3801a5e..9126c00 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -124,9 +124,21 @@ main (int argc, char *argv[])
 	struct hostent *hostinfo;
 	socklen_t peer_addr_len;
 	int reuse;
+	int bg;
+
+	/* XXX Quick implementation -- fix if more functionality is desired. */
+	if (argc >= 2 && strcmp(argv[1], "--background") == 0) {
+		argc--;
+		argv[1] = argv[0];
+		argv++;
+		bg = 1;
+	}
+	else
+		bg = 0;
 
 	if (argc != 2) {
-		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
+		fprintf (stderr, "Usage: %s [--background] <output-file>\n",
+			 argv[0]);
 		return 1;
 	}
 
@@ -179,6 +191,27 @@ main (int argc, char *argv[])
 		return 1;
 	}
 
+	if (bg) {
+		int pid = fork ();
+		if (pid > 0) {
+			printf ("%d\n", pid);
+			return 0;
+		}
+		if (pid < 0) {
+			fprintf (stderr, "Error: fork() failed: %s\n",
+				 strerror (errno));
+			close (sock);
+			return 1;
+	    	}
+		/* Reached if pid == 0. */
+		/* Close stdout so that the one interested in pid value will
+		   also get EOF. */
+		close (1);
+		/* dup2() will re-reserve fd 1 (opportunistically, in case fd 2
+		   is open. If that was not open we don't care fd 1 either.) */
+		dup2 (2, 1);
+	}
+
 	peer_addr_len = sizeof (peer_addr);
 	peer = accept (sock, (struct sockaddr *) &peer_addr, &peer_addr_len);
 	if (peer == -1) {
-- 
1.7.7.3

[PATCH 2/2] test/test-lib.sh: launch smtp-dummy with --background and finally kill(1) it.

[Tomi Ollila]

Take the new --background option of smtp-dummy to use so that it is known
there is smtpd listener ready when it is needed. As the smtp-dummy instance
is no longer child process of the script sending SIGKILL to it is the only
way to make sure the instance exits when required.
---
 test/test-lib.sh |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 6be93fe..a857afe 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -382,8 +382,9 @@ 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=$!
+
+    smtp_dummy_pid=$($TEST_DIRECTORY/smtp-dummy --background sent_message) \
+	|| return
     test_emacs \
 	"(let ((message-send-mail-function 'message-smtpmail-send-it)
 	       (smtpmail-smtp-server \"localhost\")
@@ -398,9 +399,10 @@ 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}
+    # Cannot wait, not our child. 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.7.3

[PATCH 1/2] test/smtp-dummy: add --background option for going background after listen(2)

[Tomi Ollila]

To avoid the possibility that smtp-dummy doesn't have chance to bind
its listening socket until something tries to send message to it this
option makes caller wait until socket is already listening for connections.

In case this --background option is used, the pid of running smtp-dummy
is printed on stdout.
---

Resent after whitespace-cleanup in patch 1/2 (this patch).

 test/smtp-dummy.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 3801a5e..9126c00 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -124,9 +124,21 @@ main (int argc, char *argv[])
 	struct hostent *hostinfo;
 	socklen_t peer_addr_len;
 	int reuse;
+	int bg;
+
+	/* XXX Quick implementation -- fix if more functionality is desired. */
+	if (argc >= 2 && strcmp(argv[1], "--background") == 0) {
+		argc--;
+		argv[1] = argv[0];
+		argv++;
+		bg = 1;
+	}
+	else
+		bg = 0;
 
 	if (argc != 2) {
-		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
+		fprintf (stderr, "Usage: %s [--background] <output-file>\n",
+			 argv[0]);
 		return 1;
 	}
 
@@ -179,7 +191,27 @@ main (int argc, char *argv[])
 		return 1;
 	}
 
+	if (bg) {
+		int pid = fork ();
+		if (pid > 0) {
+			printf ("%d\n", pid);
+			return 0;
+		}
+		if (pid < 0) {
+			fprintf (stderr, "Error: fork() failed: %s\n",
+				 strerror (errno));
+			close (sock);
+			return 1;
+		}
+		/* Reached if pid == 0. */
+		/* Close stdout so that the one interested in pid value will
+		   also get EOF. */
+		close (1);
+		/* dup2() will re-reserve fd 1 (opportunistically, in case fd 2
+		   is open. If that was not open we don't care fd 1 either.) */
+		dup2 (2, 1);
+	}
+
 	peer_addr_len = sizeof (peer_addr);
 	peer = accept (sock, (struct sockaddr *) &peer_addr, &peer_addr_len);
 	if (peer == -1) {
-- 
1.7.7.3

[PATCH 2/2] test/test-lib.sh: launch smtp-dummy with --background and finally kill(1) it.

[Tomi Ollila]

Take the new --background option of smtp-dummy to use so that it is known
there is smtpd listener ready when it is needed. As the smtp-dummy instance
is no longer child process of the script sending SIGKILL to it is the only
way to make sure the instance exits when required.
---

Resent after whitespace-cleanup in patch 1/2 (the other one).

 test/test-lib.sh |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 6be93fe..a857afe 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -382,8 +382,9 @@ 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=$!
+
+    smtp_dummy_pid=$($TEST_DIRECTORY/smtp-dummy --background sent_message) \
+	|| return
     test_emacs \
 	"(let ((message-send-mail-function 'message-smtpmail-send-it)
 	       (smtpmail-smtp-server \"localhost\")
@@ -398,9 +399,10 @@ 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}
+    # Cannot wait, not our child. 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.7.3

Re: [PATCH 1/2] test/smtp-dummy: add --background option for going background after listen(2)

[Dmitry Kurochkin]

Hi Tomi.

On Tue, 13 Dec 2011 11:01:22 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> To avoid the possibility that smtp-dummy doesn't have chance to bind
> its listening socket until something tries to send message to it this
> option makes caller wait until socket is already listening for connections.
> 
> In case this --background option is used, the pid of running smtp-dummy
> is printed on stdout.
> ---
> 
> Resent after whitespace-cleanup in patch 1/2 (this patch).
> 
>  test/smtp-dummy.c |   35 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 34 insertions(+), 1 deletions(-)
> 
> diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
> index 3801a5e..9126c00 100644
> --- a/test/smtp-dummy.c
> +++ b/test/smtp-dummy.c
> @@ -124,9 +124,21 @@ main (int argc, char *argv[])
>  	struct hostent *hostinfo;
>  	socklen_t peer_addr_len;
>  	int reuse;
> +	int bg;

I would rename bg to background, but that is not important.

> +
> +	/* XXX Quick implementation -- fix if more functionality is desired. */
> +	if (argc >= 2 && strcmp(argv[1], "--background") == 0) {
> +		argc--;
> +		argv[1] = argv[0];
> +		argv++;
> +		bg = 1;
> +	}
> +	else
> +		bg = 0;
>  

Sorry, this code looks ugly and unnecessary complex to me.  I really do
not like messing with argc and argv.  Perhaps something like this would
be better:

  if (argc != 2 && argc != 3)
    usage();
    return 1;

  if (argc > 2) {
    if (argv[1] == background)
      bg = 1;
    else
      usage();
      return 1;
  }

  output = argv[argc - 1];

>  	if (argc != 2) {
> -		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
> +		fprintf (stderr, "Usage: %s [--background] <output-file>\n",
> +			 argv[0]);
>  		return 1;
>  	}
>  
> @@ -179,7 +191,27 @@ main (int argc, char *argv[])
>  		return 1;
>  	}
>  
> +	if (bg) {
> +		int pid = fork ();
> +		if (pid > 0) {
> +			printf ("%d\n", pid);
> +			return 0;
> +		}
> +		if (pid < 0) {
> +			fprintf (stderr, "Error: fork() failed: %s\n",
> +				 strerror (errno));
> +			close (sock);
> +			return 1;
> +		}
> +		/* Reached if pid == 0. */
> +		/* Close stdout so that the one interested in pid value will
> +		   also get EOF. */
> +		close (1);

Please use STDOUT_FILENO instead of 1.

> +		/* dup2() will re-reserve fd 1 (opportunistically, in case fd 2
> +		   is open. If that was not open we don't care fd 1 either.) */
> +		dup2 (2, 1);

And STDERR_FILENO and STDOUT_FILENO here.  I would prefer to see "stdout"
and "stderr" instead of "1" and "2" in the comments as well.

Regards,
  Dmitry

> +	}
> +
>  	peer_addr_len = sizeof (peer_addr);
>  	peer = accept (sock, (struct sockaddr *) &peer_addr, &peer_addr_len);
>  	if (peer == -1) {
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch