2002-08-28 Marcus Brinkmann <marcus@g10code.de>
authorMarcus Brinkmann <mb@g10code.com>
Wed, 28 Aug 2002 20:31:31 +0000 (20:31 +0000)
committerMarcus Brinkmann <mb@g10code.com>
Wed, 28 Aug 2002 20:31:31 +0000 (20:31 +0000)
* posix-io.c (_gpgme_io_spawn): Use a double-fork approach.
Return 0 on success, -1 on error.
* version.c (_gpgme_get_program_version): Don't wait for the child.
* engine.c (_gpgme_engine_housecleaning): Function removed.
(do_reaping): Likewise.
(_gpgme_engine_add_child_to_reap_list): Likewise.
(struct reap_s): Removed.
(reap_list): Likewise.
(reap_list_lock): Likewise.
* engine.h (_gpgme_engine_io_event): Remove prototypes for
_gpgme_engine_housecleaning and
_gpgme_engine_add_child_to_reap_list.
* rungpg.c (_gpgme_gpg_release): Don't add child to reap list.
(struct gpg_object_s): Remove PID member.
(_gpgme_gpg_new): Don't initialize GPG->pid.
(_gpgme_gpg_spawn): Don't set GPG->pid.
* wait.c (run_idle): Removed.
(gpgme_wait): Run idle_function directly.

gpgme/ChangeLog
gpgme/engine.c
gpgme/engine.h
gpgme/posix-io.c
gpgme/rungpg.c
gpgme/version.c
gpgme/wait.c

index 9379033..bef845b 100644 (file)
@@ -1,3 +1,24 @@
+2002-08-28  Marcus Brinkmann  <marcus@g10code.de>
+
+       * posix-io.c (_gpgme_io_spawn): Use a double-fork approach.
+       Return 0 on success, -1 on error.
+       * version.c (_gpgme_get_program_version): Don't wait for the child.
+       * engine.c (_gpgme_engine_housecleaning): Function removed.
+       (do_reaping): Likewise.
+       (_gpgme_engine_add_child_to_reap_list): Likewise.
+       (struct reap_s): Removed.
+       (reap_list): Likewise.
+       (reap_list_lock): Likewise.
+       * engine.h (_gpgme_engine_io_event): Remove prototypes for
+       _gpgme_engine_housecleaning and
+       _gpgme_engine_add_child_to_reap_list.
+       * rungpg.c (_gpgme_gpg_release): Don't add child to reap list.
+       (struct gpg_object_s): Remove PID member.
+       (_gpgme_gpg_new): Don't initialize GPG->pid.
+       (_gpgme_gpg_spawn): Don't set GPG->pid.
+       * wait.c (run_idle): Removed.
+       (gpgme_wait): Run idle_function directly.
+
 2002-08-21  Marcus Brinkmann  <marcus@g10code.de>
 
        * encrypt-sign.c (encrypt_sign_status_handler): Remove dead
index b33bfe9..5c005e6 100644 (file)
@@ -52,18 +52,6 @@ struct engine_object_s
 };
 
 
-struct reap_s
-{
-  struct reap_s *next;
-  int pid;
-  time_t entered;
-  int term_send;
-};
-
-static struct reap_s *reap_list;
-DEFINE_STATIC_LOCK (reap_list_lock);
-
-
 /* Get the path of the engine for PROTOCOL.  */
 const char *
 _gpgme_engine_get_path (GpgmeProtocol proto)
@@ -627,85 +615,3 @@ _gpgme_engine_io_event (EngineObject engine,
       break;
     }
 }
-
-\f
-void
-_gpgme_engine_add_child_to_reap_list (void *buf, int buflen, pid_t pid)
-{
-  /* Reuse the memory, so that we don't need to allocate another
-     memory block and to handle errors.  */
-  struct reap_s *child = buf;
-
-  assert (buflen >= sizeof *child);
-  memset (child, 0, sizeof *child);
-  child->pid = pid;
-  child->entered = time (NULL);
-  LOCK (reap_list_lock);
-  child->next = reap_list;
-  reap_list = child;
-  UNLOCK (reap_list_lock);
-}
-
-static void
-do_reaping (void)
-{
-  struct reap_s *r, *rlast;
-  static time_t last_check;
-  time_t cur_time = time (NULL);
-
-  /* A race does not matter here.  */
-  if (!last_check)
-    last_check = time (NULL);
-
-  if (last_check >= cur_time)
-    return;  /* We check only every second.  */
-
-  /* Fixme: it would be nice if to have a TRYLOCK here.  */
-  LOCK (reap_list_lock);
-  for (r = reap_list, rlast = NULL; r; rlast = r, r = r ? r->next : NULL)
-    {
-      int dummy1, dummy2;
-
-      if (_gpgme_io_waitpid (r->pid, 0, &dummy1, &dummy2))
-       {
-         /* The process has terminated - remove it from the queue.  */
-         void *p = r;
-         if (!rlast)
-           {
-             reap_list = r->next;
-             r = reap_list;
-            }
-         else
-           {
-             rlast->next = r->next;
-             r = rlast;
-            }
-         xfree (p);
-        }
-      else if (!r->term_send)
-       {
-         if (r->entered + 1 >= cur_time)
-           {
-             _gpgme_io_kill (r->pid, 0);
-             r->term_send = 1;
-             r->entered = cur_time;
-            }
-        }
-      else
-       {
-         /* Give it 5 second before we are going to send the killer.  */
-         if (r->entered + 5 >= cur_time)
-           {
-             _gpgme_io_kill (r->pid, 1);
-             r->entered = cur_time; /* Just in case we have to repeat it.  */
-            }
-        }
-    }
-  UNLOCK (reap_list_lock);  
-}
-
-void
-_gpgme_engine_housecleaning (void)
-{
-  do_reaping ();
-}
index acc3367..d237509 100644 (file)
@@ -84,7 +84,4 @@ void _gpgme_engine_set_io_cbs (EngineObject engine,
 void _gpgme_engine_io_event (EngineObject engine,
                             GpgmeEventIO type, void *type_data);
 
-void _gpgme_engine_add_child_to_reap_list (void *buf, int buflen, pid_t pid);
-void _gpgme_engine_housecleaning (void);
-
 #endif /* ENGINE_H */
index c5f7978..d0fd5f8 100644 (file)
@@ -146,6 +146,7 @@ _gpgme_io_set_nonblocking (int fd)
 }
 
 
+/* Returns 0 on success, -1 on error.  */
 int
 _gpgme_io_spawn (const char *path, char **argv,
                 struct spawn_fd_item_s *fd_child_list,
@@ -155,6 +156,7 @@ _gpgme_io_spawn (const char *path, char **argv,
   DEFINE_STATIC_LOCK (fixed_signals_lock);
   pid_t pid;
   int i;
+  int status, signo;
 
   LOCK (fixed_signals_lock);
   if (!fixed_signals)
@@ -179,73 +181,85 @@ _gpgme_io_spawn (const char *path, char **argv,
 
   if (!pid)
     {
-      /* Child.  */
-      int duped_stdin = 0;
-      int duped_stderr = 0;
-
-      /* First close all fds which will not be duped.  */
-      for (i=0; fd_child_list[i].fd != -1; i++)
-       if (fd_child_list[i].dup_to == -1)
-         close (fd_child_list[i].fd);
-
-      /* And now dup and close the rest.  */
-      for (i=0; fd_child_list[i].fd != -1; i++)
+      /* Intermediate child to prevent zombie processes.  */
+      if ((pid = fork ()) == 0)
        {
-         if (fd_child_list[i].dup_to != -1)
-           {
-             if (dup2 (fd_child_list[i].fd,
-                        fd_child_list[i].dup_to) == -1)
-               {
-                 DEBUG1 ("dup2 failed in child: %s\n", strerror (errno));
-                 _exit (8);
-                }
-             if (fd_child_list[i].dup_to == 0)
-               duped_stdin=1;
-             if (fd_child_list[i].dup_to == 2)
-               duped_stderr=1;
+         /* Child.  */
+         int duped_stdin = 0;
+         int duped_stderr = 0;
+
+         /* First close all fds which will not be duped.  */
+         for (i=0; fd_child_list[i].fd != -1; i++)
+           if (fd_child_list[i].dup_to == -1)
              close (fd_child_list[i].fd);
-            }
-        }
 
-      if (!duped_stdin || !duped_stderr)
-       {
-         int fd = open ("/dev/null", O_RDWR);
-         if (fd == -1)
+         /* And now dup and close the rest.  */
+         for (i=0; fd_child_list[i].fd != -1; i++)
            {
-             DEBUG1 ("can't open `/dev/null': %s\n", strerror (errno));
-             _exit (8);
-            }
-         /* Make sure that the process has a connected stdin.  */
-         if (!duped_stdin)
+             if (fd_child_list[i].dup_to != -1)
+               {
+                 if (dup2 (fd_child_list[i].fd,
+                           fd_child_list[i].dup_to) == -1)
+                   {
+                     DEBUG1 ("dup2 failed in child: %s\n", strerror (errno));
+                     _exit (8);
+                   }
+                 if (fd_child_list[i].dup_to == 0)
+                   duped_stdin=1;
+                 if (fd_child_list[i].dup_to == 2)
+                   duped_stderr=1;
+                 close (fd_child_list[i].fd);
+               }
+           }
+         
+         if (!duped_stdin || !duped_stderr)
            {
-             if (dup2 (fd, 0) == -1)
+             int fd = open ("/dev/null", O_RDWR);
+             if (fd == -1)
                {
-                 DEBUG1("dup2(/dev/null, 0) failed: %s\n",
-                        strerror (errno));
+                 DEBUG1 ("can't open `/dev/null': %s\n", strerror (errno));
                  _exit (8);
-                }
-            }
-         if (!duped_stderr)
-           if (dup2 (fd, 2) == -1)
-             {
-               DEBUG1 ("dup2(dev/null, 2) failed: %s\n", strerror (errno));
-               _exit (8);
-             }
-         close (fd);
-       }
+               }
+             /* Make sure that the process has a connected stdin.  */
+             if (!duped_stdin)
+               {
+                 if (dup2 (fd, 0) == -1)
+                   {
+                     DEBUG1("dup2(/dev/null, 0) failed: %s\n",
+                            strerror (errno));
+                     _exit (8);
+                   }
+               }
+             if (!duped_stderr)
+               if (dup2 (fd, 2) == -1)
+                 {
+                   DEBUG1 ("dup2(dev/null, 2) failed: %s\n", strerror (errno));
+                   _exit (8);
+                 }
+             close (fd);
+           }
     
-      execv ( path, argv );
-      /* Hmm: in that case we could write a special status code to the
-        status-pipe.  */
-      DEBUG1 ("exec of `%s' failed\n", path);
-      _exit (8);
-    } /* End child.  */
+         execv ( path, argv );
+         /* Hmm: in that case we could write a special status code to the
+            status-pipe.  */
+         DEBUG1 ("exec of `%s' failed\n", path);
+         _exit (8);
+       } /* End child.  */
+      if (pid == -1)
+       _exit (1);
+      else
+       _exit (0);
+    }
     
+  _gpgme_io_waitpid (pid, 1, &status, &signo);
+  if (status)
+    return -1;
+
   /* .dup_to is not used in the parent list.  */
-  for (i=0; fd_parent_list[i].fd != -1; i++)
+  for (i = 0; fd_parent_list[i].fd != -1; i++)
     close (fd_parent_list[i].fd);
 
-  return (int) pid;
+  return 0;
 }
 
 
index 68842ec..bcfe13e 100644 (file)
@@ -100,8 +100,6 @@ struct gpg_object_s
   char **argv;  
   struct fd_data_map_s *fd_data_map;
 
-  int pid; /* we can't use pid_t because we don't use it in Windoze */
-
   /* stuff needed for pipemode */
   struct
   {
@@ -264,8 +262,6 @@ _gpgme_gpg_new (GpgObject *r_gpg)
   gpg->cmd.linked_data = NULL;
   gpg->cmd.linked_idx = -1;
 
-  gpg->pid = -1;
-
   /* Allocate the read buffer for the status pipe.  */
   gpg->status.bufsize = 1024;
   gpg->status.readpos = 0;
@@ -345,10 +341,7 @@ _gpgme_gpg_release (GpgObject gpg)
   free_fd_data_map (gpg->fd_data_map);
   if (gpg->cmd.fd != -1)
     _gpgme_io_close (gpg->cmd.fd);
-  if (gpg->pid != -1)
-    _gpgme_engine_add_child_to_reap_list (gpg, sizeof *gpg, gpg->pid);
-  else
-    xfree (gpg);
+  xfree (gpg);
 }
 
 void
@@ -841,7 +834,7 @@ _gpgme_gpg_spawn (GpgObject gpg, void *opaque)
 {
   GpgmeError rc;
   int i, n;
-  int pid;
+  int status;
   struct spawn_fd_item_s *fd_child_list, *fd_parent_list;
 
   if (!gpg)
@@ -916,13 +909,12 @@ _gpgme_gpg_spawn (GpgObject gpg, void *opaque)
   fd_parent_list[n].fd = -1;
   fd_parent_list[n].dup_to = -1;
 
-  pid = _gpgme_io_spawn (_gpgme_get_gpg_path (),
-                        gpg->argv, fd_child_list, fd_parent_list);
+  status = _gpgme_io_spawn (_gpgme_get_gpg_path (),
+                           gpg->argv, fd_child_list, fd_parent_list);
   xfree (fd_child_list);
-  if (pid == -1)
+  if (status == -1)
     return mk_error (Exec_Error);
 
-  gpg->pid = pid;
   if (gpg->pm.used)
     gpg->pm.active = 1;
 
index 2b2874b..836a597 100644 (file)
@@ -219,12 +219,11 @@ _gpgme_get_program_version (const char *const path)
   int linelen = 0;
   char *mark = NULL;
   int rp[2];
-  pid_t pid;
   int nread;
   char *argv[] = {(char *) path, "--version", 0};
   struct spawn_fd_item_s pfd[] = { {0, -1}, {-1, -1} };
   struct spawn_fd_item_s cfd[] = { {-1, 1 /* STDOUT_FILENO */}, {-1, -1} };
-  int status, signal;
+  int status;
 
   if (!path)
     return NULL;
@@ -235,8 +234,8 @@ _gpgme_get_program_version (const char *const path)
   pfd[0].fd = rp[1];
   cfd[0].fd = rp[1];
 
-  pid = _gpgme_io_spawn (path, argv, cfd, pfd);
-  if (pid < 0)
+  status = _gpgme_io_spawn (path, argv, cfd, pfd);
+  if (status < 0)
     {
       _gpgme_io_close (rp[0]);
       _gpgme_io_close (rp[1]);
@@ -261,7 +260,6 @@ _gpgme_get_program_version (const char *const path)
   while (nread > 0 && linelen < LINELENGTH - 1);
 
   _gpgme_io_close (rp[0]);
-  _gpgme_io_waitpid (pid, 1, &status, &signal);
 
   if (mark)
     {
index 69c0a11..1178649 100644 (file)
@@ -52,8 +52,6 @@ struct wait_item_s
   int dir;
 };
 
-static void run_idle (void);
-
 \f
 void
 _gpgme_fd_table_init (fd_table_t fdt)
@@ -133,14 +131,6 @@ gpgme_register_idle (GpgmeIdleFunc idle)
   return old_idle;
 }
 
-static void
-run_idle ()
-{
-  _gpgme_engine_housecleaning ();
-  if (idle_function)
-    idle_function ();
-}
-
 \f
 /* Wait on all file descriptors listed in FDT and process them using
    the registered callbacks.  Returns -1 on error (with errno set), 0
@@ -254,8 +244,8 @@ gpgme_wait (GpgmeCtx ctx, GpgmeError *status, int hang)
        }
       UNLOCK (ctx_done_list_lock);
 
-      if (hang)
-       run_idle ();
+      if (hang && idle_function)
+       idle_function ();
     }
   while (hang && (!ctx || !ctx->cancel));