From 685b782a18adb90bbf78956682e4e7f89fed678c Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 16 Jun 2015 18:08:32 +0200 Subject: [PATCH] dirmngr: Cleanup zombies and fix hang on shutdown. * dirmngr/ldap-wrapper.c (ldap_wrapper_thread): Move nfds computation into the loop. Check the queue also on timeout. Close log_fd and reader context on EOF or error. -- The major bug here was that on an EOF of the log fd the log fd was not closed and thus the final queue item removal could not work. Checking the queue on a timeout is not really necessary but it help in case there is a race condition lingering. GnuPG-bug-id: 1838, 1978 Signed-off-by: Werner Koch --- dirmngr/ldap-wrapper.c | 63 +++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/dirmngr/ldap-wrapper.c b/dirmngr/ldap-wrapper.c index 30bbda3ab..0dcc7baf5 100644 --- a/dirmngr/ldap-wrapper.c +++ b/dirmngr/ldap-wrapper.c @@ -259,26 +259,12 @@ ldap_wrapper_thread (void *dummy) struct timespec abstime; struct timespec curtime; struct timespec timeout; - int saved_errno; - fd_set fdset, read_fdset; + fd_set fdset; int ret; time_t exptime; (void)dummy; - FD_ZERO (&fdset); - nfds = -1; - for (ctx = wrapper_list; ctx; ctx = ctx->next) - { - if (ctx->log_fd != -1) - { - FD_SET (ctx->log_fd, &fdset); - if (ctx->log_fd > nfds) - nfds = ctx->log_fd; - } - } - nfds++; - npth_clock_gettime (&abstime); abstime.tv_sec += TIMERTICK_INTERVAL; @@ -286,38 +272,42 @@ ldap_wrapper_thread (void *dummy) { int any_action = 0; - /* POSIX says that fd_set should be implemented as a structure, - thus a simple assignment is fine to copy the entire set. */ - read_fdset = fdset; - npth_clock_gettime (&curtime); if (!(npth_timercmp (&curtime, &abstime, <))) { /* Inactivity is checked below. Nothing else to do. */ - // handle_tick (); npth_clock_gettime (&abstime); abstime.tv_sec += TIMERTICK_INTERVAL; } npth_timersub (&abstime, &curtime, &timeout); + FD_ZERO (&fdset); + nfds = -1; + for (ctx = wrapper_list; ctx; ctx = ctx->next) + { + if (ctx->log_fd != -1) + { + FD_SET (ctx->log_fd, &fdset); + if (ctx->log_fd > nfds) + nfds = ctx->log_fd; + } + } + nfds++; + /* FIXME: For Windows, we have to use a reader thread on the pipe that signals an event (and a npth_select_ev variant). */ - ret = npth_pselect (nfds + 1, &read_fdset, NULL, NULL, &timeout, NULL); - saved_errno = errno; - - if (ret == -1 && saved_errno != EINTR) + ret = npth_pselect (nfds + 1, &fdset, NULL, NULL, &timeout, NULL); + if (ret == -1) { - log_error (_("npth_select failed: %s - waiting 1s\n"), - strerror (saved_errno)); - npth_sleep (1); + if (errno != EINTR) + { + log_error (_("npth_select failed: %s - waiting 1s\n"), + strerror (errno)); + npth_sleep (1); + } continue; } - if (ret <= 0) - /* Interrupt or timeout. Will be handled when calculating the - next timeout. */ - continue; - /* All timestamps before exptime should be considered expired. */ exptime = time (NULL); if (exptime > INACTIVITY_TIMEOUT) @@ -331,10 +321,15 @@ ldap_wrapper_thread (void *dummy) for (ctx = wrapper_list; ctx; ctx = ctx->next) { /* Check whether there is any logging to be done. */ - if (nfds && ctx->log_fd != -1 && FD_ISSET (ctx->log_fd, &read_fdset)) + if (nfds && ctx->log_fd != -1 && FD_ISSET (ctx->log_fd, &fdset)) { if (read_log_data (ctx)) - any_action = 1; + { + ksba_reader_release (ctx->reader); + ctx->reader = NULL; + SAFE_CLOSE (ctx->log_fd); + any_action = 1; + } } /* Check whether the process is still running. */