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 <[email protected]>

1 files 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.  */