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.

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.

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;

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;
+      /* Intermediate child to prevent zombie processes.  */
-      char errbuf[512];
+      pid_t pid;
-      int *fdp;
 
-      /* Dup handles to stdin/stdout. */
+      if ((pid = fork ()) == 0)
-      if (rp[1] != STDOUT_FILENO)
 	{
-	  if (dup2 (rp[1], STDOUT_FILENO) == -1)
+	  /* Child.  */
-	    {
-	      LOG ("dup2 failed in child: %s\n", strerror (errno));
-	      _exit (4);
-	    }
-	}
-      if (wp[0] != STDIN_FILENO)
-	{
-	  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
+	  int i, n;
-         passed to the child. */
+	  char errbuf[512];
-      fdp = fd_child_list;
+	  int *fdp;
-      if (fdp)
-        {
-          for (; *fdp != -1 && *fdp != STDERR_FILENO; fdp++)
-            ;
-        }
-      if (!fdp || *fdp == -1)
-        {
-	  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);
-	    }
-	}
 
-
+	  /* Dup handles to stdin/stdout. */
-      /* Close all files which will not be duped and are not in the
+	  if (rp[1] != STDOUT_FILENO)
-         fd_child_list. */
+	    {
-      n = sysconf (_SC_OPEN_MAX);
+	      if (dup2 (rp[1], STDOUT_FILENO) == -1)
-      if (n < 0)
+		{
-        n = MAX_OPEN_FDS;
+		  LOG ("dup2 failed in child: %s\n", strerror (errno));
-      for (i=0; i < n; i++)
+		  _exit (4);
-        {
+		}
-          if ( i == STDIN_FILENO || i == STDOUT_FILENO || i == STDERR_FILENO)
+	    }
-            continue;
+	  if (wp[0] != STDIN_FILENO)
+	    {
+	      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)
 	    {
-	      while (*fdp != -1 && *fdp != i)
+	      for (; *fdp != -1 && *fdp != STDERR_FILENO; fdp++)
-		fdp++;
+		;
+	    }
+	  if (!fdp || *fdp == -1)
+	    {
+	      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);
+		}
 	    }
 
-          if (!(fdp && *fdp != -1))
-            close(i);
-        }
-      errno = 0;
 
-      execv (name, argv); 
+	  /* Close all files which will not be duped and are not in the
-      /* oops - use the pipe to tell the parent about it */
+	     fd_child_list. */
-      snprintf (errbuf, sizeof(errbuf)-1, "ERR %d can't exec `%s': %.50s\n",
+	  n = sysconf (_SC_OPEN_MAX);
-                ASSUAN_Problem_Starting_Server, name, strerror (errno));
+	  if (n < 0)
-      errbuf[sizeof(errbuf)-1] = 0;
+	    n = MAX_OPEN_FDS;
-      writen (1, errbuf, strlen (errbuf));
+	  for (i=0; i < n; i++)
-      _exit (4);
+	    {
+	      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);
+	    }
+
+	  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]);