2004-12-16 Marcus Brinkmann <marcus@g10code.de>

	* assuan-pipe-connect.c (do_finish): Do not wait for child to finish.
	(assuan_pipe_connect): Use double-fork approach.
	* assuan-connect.c (assuan_disconnect): Do not write BYE to the
	status line.

diff --git a/assuan/ChangeLog b/assuan/ChangeLog
index cc383539b2ac957673c7d39fe90376e0a2839be0..1cf2b2ac73a4a2a4846ee0167f0fdc3059979b98 100644 (file)
--- a/assuan/ChangeLog
+++ b/assuan/ChangeLog
@@ -1,3 +1,10 @@
+2004-12-16  Marcus Brinkmann  <marcus@g10code.de>
+
+       * assuan-pipe-connect.c (do_finish): Do not wait for child to finish.
+       (assuan_pipe_connect): Use double-fork approach.
+       * assuan-connect.c (assuan_disconnect): Do not write BYE to the
+       status line.
+
 2004-12-07  Marcus Brinkmann  <marcus@g10code.de>
 
        * README.1st: Add copyright notice.

diff --git a/assuan/assuan-connect.c b/assuan/assuan-connect.c
index 009aaab43dacd504714ca0b403ef0ee8bbf55352..d260e927777c6945912c294770bd171a02a864d6 100644 (file)
--- a/assuan/assuan-connect.c
+++ b/assuan/assuan-connect.c
@@ -39,7 +39,11 @@ assuan_disconnect (ASSUAN_CONTEXT ctx)
 {
   if (ctx)
     {
+#if 0
+      /* This may not work if the pipe is full and the other end is
+        blocked.  */
       assuan_write_line (ctx, "BYE");
+#endif
       ctx->finish_handler (ctx);
       ctx->deinit_handler (ctx);
       ctx->deinit_handler = NULL;

diff --git a/assuan/assuan-pipe-connect.c b/assuan/assuan-pipe-connect.c
index a3ddc2241583b6ec14ce94349fab250df47cbc45..f79884cf6f55bd1433f61be77fe24686832823e7 100644 (file)
--- a/assuan/assuan-pipe-connect.c
+++ b/assuan/assuan-pipe-connect.c
@@ -80,8 +80,11 @@ do_finish (ASSUAN_CONTEXT ctx)
     }
   if (ctx->pid != -1)
     {
+#if 0
+      /* This is already done by the double fork.  */
       waitpid (ctx->pid, NULL, 0);  /* FIXME Check return value.  */
       ctx->pid = -1;
+#endif
     }
   return 0;
 }
@@ -155,6 +158,8 @@ assuan_pipe_connect (ASSUAN_CONTEXT *ctx, const char *name, char *const argv[],
   (*ctx)->deinit_handler = do_deinit;
   (*ctx)->finish_handler = do_finish;
 
+  /* FIXME: Use _gpgme_io_spawn.  The PID stored here is actually
+     soon useless.  */
   (*ctx)->pid = fork ();
   if ((*ctx)->pid < 0)
     {
@@ -168,82 +173,98 @@ assuan_pipe_connect (ASSUAN_CONTEXT *ctx, const char *name, char *const argv[],
 
   if ((*ctx)->pid == 0)
     {
-      int i, n;
-      char errbuf[512];
-      int *fdp;
+      /* Intermediate child to prevent zombie processes.  */
+      pid_t pid;
 
-      /* Dup handles to stdin/stdout. */
-      if (rp[1] != STDOUT_FILENO)
+      if ((pid = fork ()) == 0)
        {
-         if (dup2 (rp[1], STDOUT_FILENO) == -1)
+         /* Child.  */
+
+         int i, n;
+         char errbuf[512];
+         int *fdp;
+
+         /* Dup handles to stdin/stdout. */
+         if (rp[1] != STDOUT_FILENO)
            {
-             LOG ("dup2 failed in child: %s\n", strerror (errno));
-             _exit (4);
+             if (dup2 (rp[1], STDOUT_FILENO) == -1)
+               {
+                 LOG ("dup2 failed in child: %s\n", strerror (errno));
+                 _exit (4);
+               }
            }
-       }
-      if (wp[0] != STDIN_FILENO)
-       {
-         if (dup2 (wp[0], STDIN_FILENO) == -1)
+         if (wp[0] != STDIN_FILENO)
            {
-             LOG ("dup2 failed in child: %s\n", strerror (errno));
-             _exit (4);
+             if (dup2 (wp[0], STDIN_FILENO) == -1)
+               {
+                 LOG ("dup2 failed in child: %s\n", strerror (errno));
+                 _exit (4);
+               }
            }
-       }
-
-      /* Dup stderr to /dev/null unless it is in the list of FDs to be
-         passed to the child. */
-      fdp = fd_child_list;
-      if (fdp)
-        {
-          for (; *fdp != -1 && *fdp != STDERR_FILENO; fdp++)
-            ;
-        }
-      if (!fdp || *fdp == -1)
-        {
-         int fd = open ("/dev/null", O_WRONLY);
-         if (fd == -1)
+         
+         /* Dup stderr to /dev/null unless it is in the list of FDs to be
+            passed to the child. */
+         fdp = fd_child_list;
+         if (fdp)
            {
-             LOG ("can't open `/dev/null': %s\n", strerror (errno));
-             _exit (4);
+             for (; *fdp != -1 && *fdp != STDERR_FILENO; fdp++)
+               ;
            }
-         if (dup2 (fd, STDERR_FILENO) == -1)
+         if (!fdp || *fdp == -1)
            {
-             LOG ("dup2(dev/null, 2) failed: %s\n", strerror (errno));
-             _exit (4);
+             int fd = open ("/dev/null", O_WRONLY);
+             if (fd == -1)
+               {
+                 LOG ("can't open `/dev/null': %s\n", strerror (errno));
+                 _exit (4);
+               }
+             if (dup2 (fd, STDERR_FILENO) == -1)
+               {
+                 LOG ("dup2(dev/null, 2) failed: %s\n", strerror (errno));
+                 _exit (4);
+               }
            }
-       }
 
 
-      /* Close all files which will not be duped and are not in the
-         fd_child_list. */
-      n = sysconf (_SC_OPEN_MAX);
-      if (n < 0)
-        n = MAX_OPEN_FDS;
-      for (i=0; i < n; i++)
-        {
-          if ( i == STDIN_FILENO || i == STDOUT_FILENO || i == STDERR_FILENO)
-            continue;
-         fdp = fd_child_list;
-         if (fdp)
+         /* Close all files which will not be duped and are not in the
+            fd_child_list. */
+         n = sysconf (_SC_OPEN_MAX);
+         if (n < 0)
+           n = MAX_OPEN_FDS;
+         for (i=0; i < n; i++)
            {
-             while (*fdp != -1 && *fdp != i)
-               fdp++;
+             if ( i == STDIN_FILENO || i == STDOUT_FILENO || i == STDERR_FILENO)
+               continue;
+             fdp = fd_child_list;
+             if (fdp)
+               {
+                 while (*fdp != -1 && *fdp != i)
+                   fdp++;
+               }
+             
+             if (!(fdp && *fdp != -1))
+               close(i);
            }
 
-          if (!(fdp && *fdp != -1))
-            close(i);
-        }
-      errno = 0;
-
-      execv (name, argv); 
-      /* oops - use the pipe to tell the parent about it */
-      snprintf (errbuf, sizeof(errbuf)-1, "ERR %d can't exec `%s': %.50s\n",
-                ASSUAN_Problem_Starting_Server, name, strerror (errno));
-      errbuf[sizeof(errbuf)-1] = 0;
-      writen (1, errbuf, strlen (errbuf));
-      _exit (4);
+         errno = 0;
+
+         execv (name, argv); 
+         /* oops - use the pipe to tell the parent about it */
+         snprintf (errbuf, sizeof(errbuf)-1, "ERR %d can't exec `%s': %.50s\n",
+                   ASSUAN_Problem_Starting_Server, name, strerror (errno));
+         errbuf[sizeof(errbuf)-1] = 0;
+         writen (1, errbuf, strlen (errbuf));
+         _exit (4);
+       } /* End child.  */
+      if (pid == -1)
+       _exit (1);
+      else
+       _exit (0);
     }
 
+  waitpid ((*ctx)->pid, NULL, 0);
+  (*ctx)->pid = -1;
+
   close (rp[1]);
   close (wp[0]);