chiark / gitweb /
gpg agent lockup fix: Interrupt main loop when active_connections_value==0
[gnupg2.git] / agent / gpg-agent.c
index f5ecea544cb92f8293bdc89830fe788691d9191a..121bb0ed4f6a7e8b9d3486c4855e3811776db57e 100644 (file)
@@ -316,10 +316,10 @@ static int startup_signal_mask_valid;
 #endif
 
 /* Flag to indicate that a shutdown was requested.  */
-static int shutdown_pending;
+static int shutdown_pending; /* xxx threaded accesses lack locking */
 
 /* Counter for the currently running own socket checks.  */
-static int check_own_socket_running;
+static int check_own_socket_running; /* xxx threaded accesses lack locking */
 
 /* Flags to indicate that check_own_socket shall not be called.  */
 static int disable_check_own_socket;
@@ -328,7 +328,7 @@ static int disable_check_own_socket;
 static int is_supervised;
 
 /* Flag to inhibit socket removal in cleanup.  */
-static int inhibit_socket_removal;
+static int inhibit_socket_removal; /* xxx threaded accesses lack locking */
 
 /* It is possible that we are currently running under setuid permissions */
 static int maybe_setuid = 1;
@@ -382,8 +382,12 @@ static char *current_logfile;
    watched. */
 static pid_t parent_pid = (pid_t)(-1);
 
+/* Record the pid of the main thread, for easier signalling */
+static pid_t main_thread_pid = (pid_t)(-1);
+
 /* Number of active connections.  */
-static int active_connections;
+static int active_connections_value;
+static npth_mutex_t active_connections_lock;
 
 /* This object is used to dispatch progress messages from Libgcrypt to
  * the right thread.  Given that we will have at max only a few dozen
@@ -1985,11 +1989,49 @@ get_agent_ssh_socket_name (void)
 }
 
 
+static void
+lock_active_connections (void)
+{
+  int err;
+
+  err = npth_mutex_lock (&active_connections_lock);
+  if (err)
+    log_fatal ("failed to acquire active connection count mutex: %s\n",
+              strerror (err));
+}
+
+static void
+unlock_active_connections (void)
+{
+  int err;
+
+  err = npth_mutex_unlock (&active_connections_lock);
+  if (err)
+    log_fatal ("failed to release active connection count mutex: %s\n",
+              strerror (err));
+}
+
 /* Return the number of active connections. */
 int
 get_agent_active_connection_count (void)
 {
-  return active_connections;
+  int value;
+
+  lock_active_connections();
+  value = active_connections_value;
+  unlock_active_connections();
+  return value;
+}
+
+/* Increment/decrement the number of active connections. */
+static void
+adjust_agent_active_connections (int delta)
+{
+  lock_active_connections();
+  active_connections_value += delta;
+  if (active_connections_value == 0)
+    interrupt_main_thread_loop ();
+  unlock_active_connections();
 }
 
 
@@ -2020,7 +2062,7 @@ get_agent_scd_notify_event (void)
                                  GetCurrentProcess(), &h2,
                                  EVENT_MODIFY_STATE|SYNCHRONIZE, TRUE, 0))
         {
-          log_error ("setting syncronize for scd notify event failed: %s\n",
+          log_error ("setting synchronize for scd notify event failed: %s\n",
                      w32_strerror (-1) );
           CloseHandle (h);
         }
@@ -2264,17 +2306,32 @@ create_directories (void)
 }
 
 
+static int
+need_tick (void)
+{
+#ifdef HAVE_W32_SYSTEM
+  /* We do not know how to interrupt the select loop on Windows, so we
+     always need a short tick there. */
+  return 1;
+#else
+  /* if we were invoked like "gpg-agent cmd arg1 arg2" then we need to
+     watch our parent. */
+  if (parent_pid != (pid_t)(-1))
+    return 1;
+  /* if scdaemon is running, we need to check that it's alive */
+  if (agent_scd_check_running ())
+    return 1;
+  /* otherwise, nothing fine-grained to do. */
+  return 0;
+#endif /*HAVE_W32_SYSTEM*/
+}
+
 
 /* This is the worker for the ticker.  It is called every few seconds
    and may only do fast operations. */
 static void
 handle_tick (void)
 {
-  static time_t last_minute;
-
-  if (!last_minute)
-    last_minute = time (NULL);
-
   /* Check whether the scdaemon has died and cleanup in this case. */
   agent_scd_check_aliveness ();
 
@@ -2293,16 +2350,6 @@ handle_tick (void)
         }
     }
 #endif /*HAVE_W32_SYSTEM*/
-
-  /* Code to be run from time to time.  */
-#if CHECK_OWN_SOCKET_INTERVAL > 0
-  if (last_minute + CHECK_OWN_SOCKET_INTERVAL <= time (NULL))
-    {
-      check_own_socket ();
-      last_minute = time (NULL);
-    }
-#endif
-
 }
 
 
@@ -2337,7 +2384,7 @@ agent_sigusr2_action (void)
 
 #ifndef HAVE_W32_SYSTEM
 /* The signal handler for this program.  It is expected to be run in
-   its own trhead and not in the context of a signal handler.  */
+   its own thread and not in the context of a signal handler.  */
 static void
 handle_signal (int signo)
 {
@@ -2361,12 +2408,16 @@ handle_signal (int signo)
       agent_sigusr2_action ();
       break;
 
+      /* nothing to do here, just take an extra cycle on the select loop */
+    case SIGCONT:
+      break;
+
     case SIGTERM:
       if (!shutdown_pending)
         log_info ("SIGTERM received - shutting down ...\n");
       else
         log_info ("SIGTERM received - still %i open connections\n",
-                 active_connections);
+                 get_agent_active_connection_count());
       shutdown_pending++;
       if (shutdown_pending > 2)
         {
@@ -2601,7 +2652,7 @@ putty_message_thread (void *arg)
 static void *
 do_start_connection_thread (ctrl_t ctrl)
 {
-  active_connections++;
+  adjust_agent_active_connections(+1);
   agent_init_default_ctrl (ctrl);
   if (opt.verbose && !DBG_IPC)
     log_info (_("handler 0x%lx for fd %d started\n"),
@@ -2614,7 +2665,7 @@ do_start_connection_thread (ctrl_t ctrl)
 
   agent_deinit_default_ctrl (ctrl);
   xfree (ctrl);
-  active_connections--;
+  adjust_agent_active_connections(-1);
   return NULL;
 }
 
@@ -2681,7 +2732,7 @@ start_connection_thread_ssh (void *arg)
   if (check_nonce (ctrl, &socket_nonce_ssh))
     return NULL;
 
-  active_connections++;
+  adjust_agent_active_connections(+1);
   agent_init_default_ctrl (ctrl);
   if (opt.verbose)
     log_info (_("ssh handler 0x%lx for fd %d started\n"),
@@ -2694,11 +2745,27 @@ start_connection_thread_ssh (void *arg)
 
   agent_deinit_default_ctrl (ctrl);
   xfree (ctrl);
-  active_connections--;
+  adjust_agent_active_connections(-1);
   return NULL;
 }
 
 
+void interrupt_main_thread_loop (void)
+{
+#ifndef HAVE_W32_SYSTEM
+  kill (main_thread_pid, SIGCONT);
+#endif
+}
+
+/* helper function for readability: test whether a given struct
+   timespec is set to all-zeros */
+static inline int
+tv_is_set (struct timespec tv)
+{
+  return tv.tv_sec || tv.tv_nsec;
+}
+
+
 /* Connection handler loop.  Wait for connection requests and spawn a
    thread after accepting a connection.  */
 static void
@@ -2716,9 +2783,11 @@ handle_connections (gnupg_fd_t listen_fd,
   gnupg_fd_t fd;
   int nfd;
   int saved_errno;
+  int idx;
   struct timespec abstime;
   struct timespec curtime;
   struct timespec timeout;
+  struct timespec *select_timeout;
 #ifdef HAVE_W32_SYSTEM
   HANDLE events[2];
   unsigned int events_set;
@@ -2734,7 +2803,19 @@ handle_connections (gnupg_fd_t listen_fd,
     { "browser", start_connection_thread_browser },
     { "ssh",    start_connection_thread_ssh   }
   };
+  struct {
+    struct timespec interval;
+    void (*func) (void);
+    struct timespec next;
+  } timertbl[] = {
+    { { TIMERTICK_INTERVAL, 0 }, handle_tick },
+    { { CHECK_OWN_SOCKET_INTERVAL, 0 }, check_own_socket }
+  };
 
+  ret = npth_mutex_init (&active_connections_lock, NULL);
+  if (ret)
+    log_fatal ("error allocating active connections mutex: %s\n",
+              strerror (ret));
 
   ret = npth_attr_init(&tattr);
   if (ret)
@@ -2748,8 +2829,10 @@ handle_connections (gnupg_fd_t listen_fd,
   npth_sigev_add (SIGUSR1);
   npth_sigev_add (SIGUSR2);
   npth_sigev_add (SIGINT);
+  npth_sigev_add (SIGCONT);
   npth_sigev_add (SIGTERM);
   npth_sigev_fini ();
+  main_thread_pid = getpid ();
 #else
 # ifdef HAVE_W32CE_SYSTEM
   /* Use a dummy event. */
@@ -2761,6 +2844,7 @@ handle_connections (gnupg_fd_t listen_fd,
 # endif
 #endif
 
+
   if (disable_check_own_socket)
     my_inotify_fd = -1;
   else if ((err = gnupg_inotify_watch_socket (&my_inotify_fd, socket_name)))
@@ -2823,15 +2907,12 @@ handle_connections (gnupg_fd_t listen_fd,
   listentbl[2].l_fd = listen_fd_browser;
   listentbl[3].l_fd = listen_fd_ssh;
 
-  npth_clock_gettime (&abstime);
-  abstime.tv_sec += TIMERTICK_INTERVAL;
-
   for (;;)
     {
       /* Shutdown test.  */
       if (shutdown_pending)
         {
-          if (active_connections == 0)
+          if (get_agent_active_connection_count() == 0)
             break; /* ready */
 
           /* Do not accept new connections but keep on running the
@@ -2854,18 +2935,52 @@ handle_connections (gnupg_fd_t listen_fd,
          thus a simple assignment is fine to copy the entire set.  */
       read_fdset = fdset;
 
+      /* avoid a fine-grained timer if we don't need one: */
+      timertbl[0].interval.tv_sec = need_tick () ? TIMERTICK_INTERVAL : 0;
+      /* avoid waking up to check sockets if we can count on inotify */
+      timertbl[1].interval.tv_sec = (my_inotify_fd == -1) ? CHECK_OWN_SOCKET_INTERVAL : 0;
+
+      /* loop through all timers, fire any registered functions, and
+         plan next timer to trigger */
       npth_clock_gettime (&curtime);
-      if (!(npth_timercmp (&curtime, &abstime, <)))
-       {
-         /* Timeout.  */
-         handle_tick ();
-         npth_clock_gettime (&abstime);
-         abstime.tv_sec += TIMERTICK_INTERVAL;
-       }
-      npth_timersub (&abstime, &curtime, &timeout);
+      abstime.tv_sec = abstime.tv_nsec = 0;
+      for (idx=0; idx < DIM(timertbl); idx++)
+        {
+          /* schedule any unscheduled timers */
+          if ((!tv_is_set (timertbl[idx].next)) && tv_is_set (timertbl[idx].interval))
+            npth_timeradd (&timertbl[idx].interval, &curtime, &timertbl[idx].next);
+          /* if a timer is due, fire it ... */
+          if (tv_is_set (timertbl[idx].next))
+            {
+              if (!(npth_timercmp (&curtime, &timertbl[idx].next, <)))
+                {
+                  timertbl[idx].func ();
+                  npth_clock_gettime (&curtime);
+                  /* ...and reschedule it, if desired: */
+                  if (tv_is_set (timertbl[idx].interval))
+                    npth_timeradd (&timertbl[idx].interval, &curtime, &timertbl[idx].next);
+                  else
+                    timertbl[idx].next.tv_sec = timertbl[idx].next.tv_nsec = 0;
+                }
+            }
+          /* accumulate next timer to come due in abstime: */
+          if (tv_is_set (timertbl[idx].next) &&
+              ((!tv_is_set (abstime)) ||
+               (npth_timercmp (&abstime, &timertbl[idx].next, >))))
+            abstime = timertbl[idx].next;
+        }
+      /* choose a timeout for the select loop: */
+      if (tv_is_set (abstime))
+        {
+          npth_timersub (&abstime, &curtime, &timeout);
+          select_timeout = &timeout;
+        }
+      else
+          select_timeout = NULL;
+      
 
 #ifndef HAVE_W32_SYSTEM
-      ret = npth_pselect (nfd+1, &read_fdset, NULL, NULL, &timeout,
+      ret = npth_pselect (nfd+1, &read_fdset, NULL, NULL, select_timeout,
                           npth_sigev_sigmask ());
       saved_errno = errno;
 
@@ -2875,7 +2990,7 @@ handle_connections (gnupg_fd_t listen_fd,
           handle_signal (signo);
       }
 #else
-      ret = npth_eselect (nfd+1, &read_fdset, NULL, NULL, &timeout,
+      ret = npth_eselect (nfd+1, &read_fdset, NULL, NULL, select_timeout,
                           events, &events_set);
       saved_errno = errno;
 
@@ -2898,7 +3013,6 @@ handle_connections (gnupg_fd_t listen_fd,
 
       if (!shutdown_pending)
         {
-          int idx;
           ctrl_t ctrl;
           npth_t thread;