From b073b6dac3e809f6755e44f11d7b7f747ce9cbb3 Mon Sep 17 00:00:00 2001 From: Marcus Brinkmann Date: Wed, 28 Aug 2002 20:31:31 +0000 Subject: [PATCH] 2002-08-28 Marcus Brinkmann * 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. --- trunk/gpgme/ChangeLog | 21 +++++++ trunk/gpgme/engine.c | 94 ------------------------------- trunk/gpgme/engine.h | 3 - trunk/gpgme/posix-io.c | 124 +++++++++++++++++++++++------------------ trunk/gpgme/rungpg.c | 18 ++---- trunk/gpgme/version.c | 8 +-- trunk/gpgme/wait.c | 14 +---- 7 files changed, 100 insertions(+), 182 deletions(-) diff --git a/trunk/gpgme/ChangeLog b/trunk/gpgme/ChangeLog index 9379033..bef845b 100644 --- a/trunk/gpgme/ChangeLog +++ b/trunk/gpgme/ChangeLog @@ -1,3 +1,24 @@ +2002-08-28 Marcus Brinkmann + + * 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 * encrypt-sign.c (encrypt_sign_status_handler): Remove dead diff --git a/trunk/gpgme/engine.c b/trunk/gpgme/engine.c index b33bfe9..5c005e6 100644 --- a/trunk/gpgme/engine.c +++ b/trunk/gpgme/engine.c @@ -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; } } - - -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 (); -} diff --git a/trunk/gpgme/engine.h b/trunk/gpgme/engine.h index acc3367..d237509 100644 --- a/trunk/gpgme/engine.h +++ b/trunk/gpgme/engine.h @@ -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 */ diff --git a/trunk/gpgme/posix-io.c b/trunk/gpgme/posix-io.c index c5f7978..d0fd5f8 100644 --- a/trunk/gpgme/posix-io.c +++ b/trunk/gpgme/posix-io.c @@ -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; } diff --git a/trunk/gpgme/rungpg.c b/trunk/gpgme/rungpg.c index 68842ec..bcfe13e 100644 --- a/trunk/gpgme/rungpg.c +++ b/trunk/gpgme/rungpg.c @@ -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; diff --git a/trunk/gpgme/version.c b/trunk/gpgme/version.c index 2b2874b..836a597 100644 --- a/trunk/gpgme/version.c +++ b/trunk/gpgme/version.c @@ -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) { diff --git a/trunk/gpgme/wait.c b/trunk/gpgme/wait.c index 69c0a11..1178649 100644 --- a/trunk/gpgme/wait.c +++ b/trunk/gpgme/wait.c @@ -52,8 +52,6 @@ struct wait_item_s int dir; }; -static void run_idle (void); - 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 (); -} - /* 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)); -- 2.26.2