[Pkg-gnupg-commit] [gnupg2] 07/08: Help dirmngr and gpg-agent idle better by default
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Thu Nov 10 18:16:10 UTC 2016
This is an automated email from the git hooks/post-receive script.
dkg pushed a commit to branch master
in repository gnupg2.
commit 862db4e6000aed6805b0f3875bbf01d58c71fc01
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date: Thu Nov 10 07:25:05 2016 -0800
Help dirmngr and gpg-agent idle better by default
---
...-dirmngr-More-win32-system-daemon-cleanup.patch | 48 +++++
...-Avoid-potential-race-condition-when-some.patch | 77 +++++++
...3-dimrngr-Avoid-need-for-hkp-housekeeping.patch | 228 +++++++++++++++++++++
.../0144-dirmngr-Drop-useless-housekeeping.patch | 205 ++++++++++++++++++
...-dirmngr-Lazily-launch-ldap-reaper-thread.patch | 119 +++++++++++
...gent-Create-framework-of-scheduled-timers.patch | 192 +++++++++++++++++
...-threads-to-interrupt-main-select-loop-wi.patch | 101 +++++++++
...gent-Avoid-tight-timer-tick-when-possible.patch | 87 ++++++++
...-scheduled-checks-on-socket-when-inotify-.patch | 26 +++
debian/patches/series | 9 +
10 files changed, 1092 insertions(+)
diff --git a/debian/patches/dirmngr-idling/0141-dirmngr-More-win32-system-daemon-cleanup.patch b/debian/patches/dirmngr-idling/0141-dirmngr-More-win32-system-daemon-cleanup.patch
new file mode 100644
index 0000000..a173651
--- /dev/null
+++ b/debian/patches/dirmngr-idling/0141-dirmngr-More-win32-system-daemon-cleanup.patch
@@ -0,0 +1,48 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Fri, 28 Oct 2016 23:54:43 -0400
+Subject: dirmngr: More win32 system daemon cleanup.
+
+* dirmngr/dirmngr.c (handle_tick): remove win32 tests for
+shutdown_pending, no longer needed.
+
+--
+
+In d83ba4897bf217d1045c58d1b99e52bd31c58812, we removed the
+Windows-specific system daemon features, where shutdown_pending was
+set from w32_service_control(). shutdown_pending is now never
+assigned outside of handle_signal() or within an inotify test, neither
+of which are available on win32.
+
+As a result, this stanza in handle_tick() should be dead code, and can
+be removed to keep things simple.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ dirmngr/dirmngr.c | 14 --------------
+ 1 file changed, 14 deletions(-)
+
+diff --git a/dirmngr/dirmngr.c b/dirmngr/dirmngr.c
+index 6c04e76..042ac45 100644
+--- a/dirmngr/dirmngr.c
++++ b/dirmngr/dirmngr.c
+@@ -1805,20 +1805,6 @@ time_for_housekeeping_p (time_t curtime)
+ static void
+ handle_tick (void)
+ {
+- /* Under Windows we don't use signals and need a way for the loop to
+- check for the shutdown flag. */
+-#ifdef HAVE_W32_SYSTEM
+- if (shutdown_pending)
+- log_info (_("SIGTERM received - shutting down ...\n"));
+- if (shutdown_pending > 2)
+- {
+- log_info (_("shutdown forced\n"));
+- log_info ("%s %s stopped\n", strusage(11), strusage(13) );
+- cleanup ();
+- dirmngr_exit (0);
+- }
+-#endif /*HAVE_W32_SYSTEM*/
+-
+ if (time_for_housekeeping_p (gnupg_get_time ()))
+ {
+ npth_t thread;
diff --git a/debian/patches/dirmngr-idling/0142-dirmngr-hkp-Avoid-potential-race-condition-when-some.patch b/debian/patches/dirmngr-idling/0142-dirmngr-hkp-Avoid-potential-race-condition-when-some.patch
new file mode 100644
index 0000000..40e1dbf
--- /dev/null
+++ b/debian/patches/dirmngr-idling/0142-dirmngr-hkp-Avoid-potential-race-condition-when-some.patch
@@ -0,0 +1,77 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Sat, 29 Oct 2016 01:25:05 -0400
+Subject: dirmngr: hkp: Avoid potential race condition when some hosts die.
+
+* dirmngr/ks-engine-hkp.c (select_random_host): Use atomic pass
+through the host table instead of risking out-of-bounds write.
+
+--
+
+Multiple threads may write to hosttable[x]->dead while
+select_random_host() is running. For example, a housekeeping thread
+might clear the ->dead bit on some entries, or another connection to
+dirmngr might manually mark a host as alive.
+
+If one or more hosts are resurrected between the two loops over a
+given table in select_random_host(), then the allocation of tbl might
+not be large enough, resulting in a write past the end of tbl on the
+second loop.
+
+This change collapses the two loops into a single loop to avoid this
+discrepancy: each host's "dead" bit is now only checked once.
+
+As Werner points out, this isn't currently strictly necessary, since
+npth will not switch threads unless a blocking system call is made,
+and no blocking system call is made in these two loops.
+
+However, in a subsequent change in this series, we will call a
+function in this loop, and that function may sometimes write(2), or
+call other functions, which may themselves block. Keeping this as a
+single-pass loop avoids the need to keep track of what might block and
+what might not.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ dirmngr/ks-engine-hkp.c | 21 ++++++++++-----------
+ 1 file changed, 10 insertions(+), 11 deletions(-)
+
+diff --git a/dirmngr/ks-engine-hkp.c b/dirmngr/ks-engine-hkp.c
+index 3b5e75d..f17afb5 100644
+--- a/dirmngr/ks-engine-hkp.c
++++ b/dirmngr/ks-engine-hkp.c
+@@ -209,25 +209,24 @@ host_in_pool_p (int *pool, int tblidx)
+ static int
+ select_random_host (int *table)
+ {
+- int *tbl;
+- size_t tblsize;
++ int *tbl = NULL;
++ size_t tblsize = 0;
+ int pidx, idx;
+
+ /* We create a new table so that we randomly select only from
+ currently alive hosts. */
+- for (idx=0, tblsize=0; (pidx = table[idx]) != -1; idx++)
++ for (idx=0; (pidx = table[idx]) != -1; idx++)
+ if (hosttable[pidx] && !hosttable[pidx]->dead)
+- tblsize++;
++ {
++ tblsize++;
++ tbl = xtryrealloc(tbl, tblsize * sizeof *tbl);
++ if (!tbl)
++ return -1; /* memory allocation failed! */
++ tbl[tblsize-1] = pidx;
++ }
+ if (!tblsize)
+ return -1; /* No hosts. */
+
+- tbl = xtrymalloc (tblsize * sizeof *tbl);
+- if (!tbl)
+- return -1;
+- for (idx=0, tblsize=0; (pidx = table[idx]) != -1; idx++)
+- if (hosttable[pidx] && !hosttable[pidx]->dead)
+- tbl[tblsize++] = pidx;
+-
+ if (tblsize == 1) /* Save a get_uint_nonce. */
+ pidx = tbl[0];
+ else
diff --git a/debian/patches/dirmngr-idling/0143-dimrngr-Avoid-need-for-hkp-housekeeping.patch b/debian/patches/dirmngr-idling/0143-dimrngr-Avoid-need-for-hkp-housekeeping.patch
new file mode 100644
index 0000000..82883e6
--- /dev/null
+++ b/debian/patches/dirmngr-idling/0143-dimrngr-Avoid-need-for-hkp-housekeeping.patch
@@ -0,0 +1,228 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Sat, 29 Oct 2016 02:00:50 -0400
+Subject: dimrngr: Avoid need for hkp housekeeping.
+
+* dirmngr/ks-engine-hkp.c (host_is_alive): New function. Test whether
+host is alive and resurrects it if it has been dead long enough.
+(select_random_host, map_host, ks_hkp_mark_host): Use host_is_alive
+instead of testing hostinfo_t->dead directly.
+(ks_hkp_housekeeping): Remove function, no longer needed.
+* dirmngr/dirmngr.c (housekeeping_thread): Remove call to
+ks_hkp_housekeeping.
+
+--
+
+Rather than resurrecting hosts upon scheduled resurrection times, test
+whether hosts should be resurrected as they're inspected for being
+dead. This removes the need for explicit housekeeping, and makes host
+resurrections happen "just in time", rather than being clustered on
+HOUSEKEEPING_INTERVAL seconds.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ dirmngr/dirmngr.c | 4 ---
+ dirmngr/dirmngr.h | 4 ---
+ dirmngr/ks-engine-hkp.c | 73 ++++++++++++++++++++++++-------------------------
+ 3 files changed, 36 insertions(+), 45 deletions(-)
+
+diff --git a/dirmngr/dirmngr.c b/dirmngr/dirmngr.c
+index 042ac45..63561c1 100644
+--- a/dirmngr/dirmngr.c
++++ b/dirmngr/dirmngr.c
+@@ -1751,11 +1751,9 @@ static void *
+ housekeeping_thread (void *arg)
+ {
+ static int sentinel;
+- time_t curtime;
+
+ (void)arg;
+
+- curtime = gnupg_get_time ();
+ if (sentinel)
+ {
+ log_info ("housekeeping is already going on\n");
+@@ -1765,8 +1763,6 @@ housekeeping_thread (void *arg)
+ if (opt.verbose > 1)
+ log_info ("starting housekeeping\n");
+
+- ks_hkp_housekeeping (curtime);
+-
+ if (opt.verbose > 1)
+ log_info ("ready with housekeeping\n");
+ sentinel--;
+diff --git a/dirmngr/dirmngr.h b/dirmngr/dirmngr.h
+index 3b26c33..7ab0f57 100644
+--- a/dirmngr/dirmngr.h
++++ b/dirmngr/dirmngr.h
+@@ -186,10 +186,6 @@ void dirmngr_sighup_action (void);
+ const char* dirmngr_get_current_socket_name (void);
+
+
+-/*-- Various housekeeping functions. --*/
+-void ks_hkp_housekeeping (time_t curtime);
+-
+-
+ /*-- server.c --*/
+ ldap_server_t get_ldapservers_from_ctrl (ctrl_t ctrl);
+ ksba_cert_t get_cert_local (ctrl_t ctrl, const char *issuer);
+diff --git a/dirmngr/ks-engine-hkp.c b/dirmngr/ks-engine-hkp.c
+index f17afb5..98de1ee 100644
+--- a/dirmngr/ks-engine-hkp.c
++++ b/dirmngr/ks-engine-hkp.c
+@@ -203,6 +203,25 @@ host_in_pool_p (int *pool, int tblidx)
+ }
+
+
++static int
++host_is_alive (hostinfo_t hi, time_t curtime)
++{
++ if (!hi)
++ return 0;
++ if (!hi->dead)
++ return 1;
++ if (!hi->died_at)
++ return 0; /* manually marked dead */
++ if (hi->died_at + RESURRECT_INTERVAL <= curtime
++ || hi->died_at > curtime)
++ {
++ hi->dead = 0;
++ log_info ("resurrected host '%s'", hi->name);
++ return 1;
++ }
++ return 0;
++}
++
+ /* Select a random host. Consult TABLE which indices into the global
+ hosttable. Returns index into TABLE or -1 if no host could be
+ selected. */
+@@ -212,11 +231,13 @@ select_random_host (int *table)
+ int *tbl = NULL;
+ size_t tblsize = 0;
+ int pidx, idx;
++ time_t curtime;
+
++ curtime = gnupg_get_time ();
+ /* We create a new table so that we randomly select only from
+ currently alive hosts. */
+ for (idx=0; (pidx = table[idx]) != -1; idx++)
+- if (hosttable[pidx] && !hosttable[pidx]->dead)
++ if (hosttable[pidx] && host_is_alive (hosttable[pidx], curtime))
+ {
+ tblsize++;
+ tbl = xtryrealloc(tbl, tblsize * sizeof *tbl);
+@@ -392,6 +413,7 @@ map_host (ctrl_t ctrl, const char *name, int force_reselect,
+ gpg_error_t err = 0;
+ hostinfo_t hi;
+ int idx;
++ time_t curtime;
+
+ *r_host = NULL;
+ if (r_httpflags)
+@@ -543,6 +565,7 @@ map_host (ctrl_t ctrl, const char *name, int force_reselect,
+ xfree (reftbl);
+ }
+
++ curtime = gnupg_get_time ();
+ hi = hosttable[idx];
+ if (hi->pool)
+ {
+@@ -559,7 +582,7 @@ map_host (ctrl_t ctrl, const char *name, int force_reselect,
+ if (force_reselect)
+ hi->poolidx = -1;
+ else if (hi->poolidx >= 0 && hi->poolidx < hosttable_size
+- && hosttable[hi->poolidx] && hosttable[hi->poolidx]->dead)
++ && hosttable[hi->poolidx] && !host_is_alive (hosttable[hi->poolidx], curtime))
+ hi->poolidx = -1;
+
+ /* Select a host if needed. */
+@@ -583,7 +606,7 @@ map_host (ctrl_t ctrl, const char *name, int force_reselect,
+ assert (hi);
+ }
+
+- if (hi->dead)
++ if (!host_is_alive (hi, curtime))
+ {
+ log_error ("host '%s' marked as dead\n", hi->name);
+ if (r_poolname)
+@@ -688,7 +711,8 @@ ks_hkp_mark_host (ctrl_t ctrl, const char *name, int alive)
+ {
+ gpg_error_t err = 0;
+ hostinfo_t hi, hi2;
+- int idx, idx2, idx3, n;
++ int idx, idx2, idx3, n, is_alive;
++ time_t curtime;
+
+ if (!name || !*name || !strcmp (name, "localhost"))
+ return 0;
+@@ -697,13 +721,15 @@ ks_hkp_mark_host (ctrl_t ctrl, const char *name, int alive)
+ if (idx == -1)
+ return gpg_error (GPG_ERR_NOT_FOUND);
+
++ curtime = gnupg_get_time ();
+ hi = hosttable[idx];
+- if (alive && hi->dead)
++ is_alive = host_is_alive (hi, curtime);
++ if (alive && !is_alive)
+ {
+ hi->dead = 0;
+ err = ks_printf_help (ctrl, "marking '%s' as alive", name);
+ }
+- else if (!alive && !hi->dead)
++ else if (!alive && is_alive)
+ {
+ hi->dead = 1;
+ hi->died_at = 0; /* Manually set dead. */
+@@ -735,14 +761,15 @@ ks_hkp_mark_host (ctrl_t ctrl, const char *name, int alive)
+
+ hi2 = hosttable[n];
+ if (!hi2)
+- ;
+- else if (alive && hi2->dead)
++ continue;
++ is_alive = host_is_alive (hi2, curtime);
++ if (alive && !is_alive)
+ {
+ hi2->dead = 0;
+ err = ks_printf_help (ctrl, "marking '%s' as alive",
+ hi2->name);
+ }
+- else if (!alive && !hi2->dead)
++ else if (!alive && is_alive)
+ {
+ hi2->dead = 1;
+ hi2->died_at = 0; /* Manually set dead. */
+@@ -944,34 +971,6 @@ ks_hkp_resolve (ctrl_t ctrl, parsed_uri_t uri)
+ }
+
+
+-/* Housekeeping function called from the housekeeping thread. It is
+- used to mark dead hosts alive so that they may be tried again after
+- some time. */
+-void
+-ks_hkp_housekeeping (time_t curtime)
+-{
+- int idx;
+- hostinfo_t hi;
+-
+- for (idx=0; idx < hosttable_size; idx++)
+- {
+- hi = hosttable[idx];
+- if (!hi)
+- continue;
+- if (!hi->dead)
+- continue;
+- if (!hi->died_at)
+- continue; /* Do not resurrect manually shot hosts. */
+- if (hi->died_at + RESURRECT_INTERVAL <= curtime
+- || hi->died_at > curtime)
+- {
+- hi->dead = 0;
+- log_info ("resurrected host '%s'", hi->name);
+- }
+- }
+-}
+-
+-
+ /* Send an HTTP request. On success returns an estream object at
+ R_FP. HOSTPORTSTR is only used for diagnostics. If HTTPHOST is
+ not NULL it will be used as HTTP "Host" header. If POST_CB is not
diff --git a/debian/patches/dirmngr-idling/0144-dirmngr-Drop-useless-housekeeping.patch b/debian/patches/dirmngr-idling/0144-dirmngr-Drop-useless-housekeeping.patch
new file mode 100644
index 0000000..407de23
--- /dev/null
+++ b/debian/patches/dirmngr-idling/0144-dirmngr-Drop-useless-housekeeping.patch
@@ -0,0 +1,205 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Sat, 29 Oct 2016 02:15:08 -0400
+Subject: dirmngr: Drop useless housekeeping.
+
+* dirmngr/dirmngr.c (handle_tick, time_for_housekeeping_p,
+housekeeping_thread): Remove, no longer needed.
+(handle_connections): Drop any attempt at a timeout, since no
+housekeeping is necessary.
+
+--
+
+The housekeeping thread no longer does anything, and the main loop was
+waking up every two seconds for no good reason. The code is simpler
+and the runtime is more efficient if we drop this.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ dirmngr/dirmngr.c | 120 +++---------------------------------------------------
+ 1 file changed, 5 insertions(+), 115 deletions(-)
+
+diff --git a/dirmngr/dirmngr.c b/dirmngr/dirmngr.c
+index 63561c1..a5f2570 100644
+--- a/dirmngr/dirmngr.c
++++ b/dirmngr/dirmngr.c
+@@ -289,20 +289,6 @@ static int disable_check_own_socket;
+ /* Counter for the active connections. */
+ static int active_connections;
+
+-/* The timer tick used for housekeeping stuff. For Windows we use a
+- longer period as the SetWaitableTimer seems to signal earlier than
+- the 2 seconds. All values are in seconds. */
+-#if defined(HAVE_W32CE_SYSTEM)
+-# define TIMERTICK_INTERVAL (60)
+-#elif defined(HAVE_W32_SYSTEM)
+-# define TIMERTICK_INTERVAL (4)
+-#else
+-# define TIMERTICK_INTERVAL (2)
+-#endif
+-
+-#define HOUSEKEEPING_INTERVAL (600)
+-
+-
+ /* This union is used to avoid compiler warnings in case a pointer is
+ 64 bit and an int 32 bit. We store an integer in a pointer and get
+ it back later (npth_getspecific et al.). */
+@@ -1746,83 +1732,6 @@ handle_signal (int signo)
+ #endif /*!HAVE_W32_SYSTEM*/
+
+
+-/* Thread to do the housekeeping. */
+-static void *
+-housekeeping_thread (void *arg)
+-{
+- static int sentinel;
+-
+- (void)arg;
+-
+- if (sentinel)
+- {
+- log_info ("housekeeping is already going on\n");
+- return NULL;
+- }
+- sentinel++;
+- if (opt.verbose > 1)
+- log_info ("starting housekeeping\n");
+-
+- if (opt.verbose > 1)
+- log_info ("ready with housekeeping\n");
+- sentinel--;
+- return NULL;
+-
+-}
+-
+-
+-#if GPGRT_GCC_HAVE_PUSH_PRAGMA
+-# pragma GCC push_options
+-# pragma GCC optimize ("no-strict-overflow")
+-#endif
+-static int
+-time_for_housekeeping_p (time_t curtime)
+-{
+- static time_t last_housekeeping;
+-
+- if (!last_housekeeping)
+- last_housekeeping = curtime;
+-
+- if (last_housekeeping + HOUSEKEEPING_INTERVAL <= curtime
+- || last_housekeeping > curtime /*(be prepared for y2038)*/)
+- {
+- last_housekeeping = curtime;
+- return 1;
+- }
+- return 0;
+-}
+-#if GPGRT_GCC_HAVE_PUSH_PRAGMA
+-# pragma GCC pop_options
+-#endif
+-
+-
+-/* This is the worker for the ticker. It is called every few seconds
+- and may only do fast operations. */
+-static void
+-handle_tick (void)
+-{
+- if (time_for_housekeeping_p (gnupg_get_time ()))
+- {
+- npth_t thread;
+- npth_attr_t tattr;
+- int err;
+-
+- err = npth_attr_init (&tattr);
+- if (err)
+- log_error ("error preparing housekeeping thread: %s\n", strerror (err));
+- else
+- {
+- npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED);
+- err = npth_create (&thread, &tattr, housekeeping_thread, NULL);
+- if (err)
+- log_error ("error spawning housekeeping thread: %s\n",
+- strerror (err));
+- npth_attr_destroy (&tattr);
+- }
+- }
+-}
+-
+-
+ /* Check the nonce on a new connection. This is a NOP unless we are
+ using our Unix domain socket emulation under Windows. */
+ static int
+@@ -1922,9 +1831,6 @@ handle_connections (assuan_fd_t listen_fd)
+ gnupg_fd_t fd;
+ int nfd, ret;
+ fd_set fdset, read_fdset;
+- struct timespec abstime;
+- struct timespec curtime;
+- struct timespec timeout;
+ int saved_errno;
+ #ifdef HAVE_INOTIFY_INIT
+ int my_inotify_fd;
+@@ -1966,9 +1872,7 @@ handle_connections (assuan_fd_t listen_fd)
+ #endif /*HAVE_INOTIFY_INIT*/
+
+
+- /* Setup the fdset. It has only one member. This is because we use
+- pth_select instead of pth_accept to properly sync timeouts with
+- to full second. */
++ /* Setup the fdset. */
+ FD_ZERO (&fdset);
+ FD_SET (FD2INT (listen_fd), &fdset);
+ nfd = FD2INT (listen_fd);
+@@ -1981,9 +1885,6 @@ handle_connections (assuan_fd_t listen_fd)
+ }
+ #endif /*HAVE_INOTIFY_INIT*/
+
+- npth_clock_gettime (&abstime);
+- abstime.tv_sec += TIMERTICK_INTERVAL;
+-
+ /* Main loop. */
+ for (;;)
+ {
+@@ -1994,31 +1895,21 @@ handle_connections (assuan_fd_t listen_fd)
+ break; /* ready */
+
+ /* Do not accept new connections but keep on running the
+- loop to cope with the timer events. */
++ select loop to wait for signals (e.g. SIGCHLD). */
+ FD_ZERO (&fdset);
+ }
+
+ /* Take a copy of the fdset. */
+ read_fdset = fdset;
+
+- 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);
+-
+ #ifndef HAVE_W32_SYSTEM
+- ret = npth_pselect (nfd+1, &read_fdset, NULL, NULL, &timeout, npth_sigev_sigmask());
++ ret = npth_pselect (nfd+1, &read_fdset, NULL, NULL, NULL, npth_sigev_sigmask());
+ saved_errno = errno;
+
+ while (npth_sigev_get_pending(&signo))
+ handle_signal (signo);
+ #else
+- ret = npth_eselect (nfd+1, &read_fdset, NULL, NULL, &timeout, NULL, NULL);
++ ret = npth_eselect (nfd+1, &read_fdset, NULL, NULL, NULL, NULL, NULL);
+ saved_errno = errno;
+ #endif
+
+@@ -2032,8 +1923,7 @@ handle_connections (assuan_fd_t listen_fd)
+
+ if (ret <= 0)
+ {
+- /* Interrupt or timeout. Will be handled when calculating the
+- next timeout. */
++ /* Interrupt. Will be handled at the top of the next loop. */
+ continue;
+ }
+
diff --git a/debian/patches/dirmngr-idling/0145-dirmngr-Lazily-launch-ldap-reaper-thread.patch b/debian/patches/dirmngr-idling/0145-dirmngr-Lazily-launch-ldap-reaper-thread.patch
new file mode 100644
index 0000000..849e488
--- /dev/null
+++ b/debian/patches/dirmngr-idling/0145-dirmngr-Lazily-launch-ldap-reaper-thread.patch
@@ -0,0 +1,119 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Mon, 31 Oct 2016 19:52:31 -0400
+Subject: dirmngr: Lazily launch ldap reaper thread.
+
+* dirmngr/dirmngr.c (main): Avoid calling ldap_wrapper_launch_thread()
+Before we need it.
+* dirmngr/ldap-wrapper.c (ldap_wrapper): Call
+ldap_wrapper_launch_thread() just in time (before any attempt to use
+an ldap subprocess).
+
+--
+
+A dirmngr process that never looks anything up in LDAP has no need for
+a reaper thread, but one was started automatically. This thread wakes
+up every two seconds to look for ldap processes that might never have
+been running. We won't start more than one reaper thread for any
+given dirmngr due to the static int "done" in
+ldap_wrapper_launch_thread(), so it's safe to call this every time
+there is a use of ldap_wrapper.
+
+If someone wants to do further dirmngr optimizations for ldap users,
+the reaper thread itself could use dynamically-calculated timeouts
+(and probably needs to be alerted dynamically when a new ldap
+subprocess is available so it can re-calculate those timeouts).
+
+Note: It's not clear to me how to test ldap access effectively; i know
+of no public ldap services that i can verify against, and i do not run
+my own ldap servers. If someone has a publicly-available ldap server
+that developers can run tests against, i would be happy to hear about
+it.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ dirmngr/dirmngr.c | 18 ------------------
+ dirmngr/ldap-wrapper.c | 7 ++++---
+ 2 files changed, 4 insertions(+), 21 deletions(-)
+
+diff --git a/dirmngr/dirmngr.c b/dirmngr/dirmngr.c
+index a5f2570..f4589f4 100644
+--- a/dirmngr/dirmngr.c
++++ b/dirmngr/dirmngr.c
+@@ -972,9 +972,6 @@ main (int argc, char **argv)
+ thread_init ();
+ cert_cache_init ();
+ crl_cache_init ();
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ start_command_handler (ASSUAN_INVALID_FD);
+ shutdown_reaper ();
+ }
+@@ -1009,9 +1006,6 @@ main (int argc, char **argv)
+ thread_init ();
+ cert_cache_init ();
+ crl_cache_init ();
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ handle_connections (3);
+ assuan_sock_close (3);
+ shutdown_reaper ();
+@@ -1209,9 +1203,6 @@ main (int argc, char **argv)
+ thread_init ();
+ cert_cache_init ();
+ crl_cache_init ();
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ handle_connections (fd);
+ assuan_sock_close (fd);
+ shutdown_reaper ();
+@@ -1221,9 +1212,6 @@ main (int argc, char **argv)
+ /* Just list the CRL cache and exit. */
+ if (argc)
+ wrong_args ("--list-crls");
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ crl_cache_init ();
+ crl_cache_list (es_stdout);
+ }
+@@ -1237,9 +1225,6 @@ main (int argc, char **argv)
+ thread_init ();
+ cert_cache_init ();
+ crl_cache_init ();
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ if (!argc)
+ rc = crl_cache_load (&ctrlbuf, NULL);
+ else
+@@ -1263,9 +1248,6 @@ main (int argc, char **argv)
+ thread_init ();
+ cert_cache_init ();
+ crl_cache_init ();
+-#if USE_LDAP
+- ldap_wrapper_launch_thread ();
+-#endif /*USE_LDAP*/
+ rc = crl_fetch (&ctrlbuf, argv[0], &reader);
+ if (rc)
+ log_error (_("fetching CRL from '%s' failed: %s\n"),
+diff --git a/dirmngr/ldap-wrapper.c b/dirmngr/ldap-wrapper.c
+index b9931a0..ed7d24d 100644
+--- a/dirmngr/ldap-wrapper.c
++++ b/dirmngr/ldap-wrapper.c
+@@ -654,9 +654,10 @@ ldap_wrapper (ctrl_t ctrl, ksba_reader_t *reader, const char *argv[])
+ only viable solutions are either to have another thread
+ responsible for logging the messages or to add an option to the
+ wrapper module to do the logging on its own. Given that we anyway
+- need a way to rip the child process and this is best done using a
+- general ripping thread, that thread can do the logging too. */
+-
++ need a way to reap the child process and this is best done using a
++ general reaping thread, that thread can do the logging too. */
++ ldap_wrapper_launch_thread ();
++
+ *reader = NULL;
+
+ /* Files: We need to prepare stdin and stdout. We get stderr from
diff --git a/debian/patches/gpg-agent-idling/0137-agent-Create-framework-of-scheduled-timers.patch b/debian/patches/gpg-agent-idling/0137-agent-Create-framework-of-scheduled-timers.patch
new file mode 100644
index 0000000..92054b8
--- /dev/null
+++ b/debian/patches/gpg-agent-idling/0137-agent-Create-framework-of-scheduled-timers.patch
@@ -0,0 +1,192 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Mon, 31 Oct 2016 21:27:36 -0400
+Subject: agent: Create framework of scheduled timers.
+
+agent/gpg-agent.c (handle_tick): Remove intermittent call to
+check_own_socket.
+(tv_is_set): Add inline helper function for readability.
+(handle_connections) Create general table of pending scheduled
+timeouts.
+
+--
+
+handle_tick() does fine-grained, rapid activity. check_own_socket()
+is supposed to happen at a different interval.
+
+Mixing the two of them makes it a requirement that one interval be a
+multiple of the other, which isn't ideal if there are different delay
+strategies that we might want in the future.
+
+Creating an extensible regular timer framework in handle_connections
+should make it possible to have any number of cadenced timers fire
+regularly, without requiring that they happen in cadences related to
+each other.
+
+It should also make it possible to dynamically change the cadence of
+any regularly-scheduled timeout.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ agent/gpg-agent.c | 87 ++++++++++++++++++++++++++++++++++++-------------------
+ 1 file changed, 58 insertions(+), 29 deletions(-)
+
+diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
+index a8ab103..5d4611f 100644
+--- a/agent/gpg-agent.c
++++ b/agent/gpg-agent.c
+@@ -2252,11 +2252,6 @@ create_directories (void)
+ 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 ();
+
+@@ -2275,16 +2270,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
+-
+ }
+
+
+@@ -2681,6 +2666,15 @@ start_connection_thread_ssh (void *arg)
+ }
+
+
++/* 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
+@@ -2698,9 +2692,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;
+@@ -2716,6 +2712,14 @@ 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_attr_init(&tattr);
+@@ -2805,9 +2809,6 @@ 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. */
+@@ -2825,18 +2826,47 @@ handle_connections (gnupg_fd_t listen_fd,
+ thus a simple assignment is fine to copy the entire set. */
+ read_fdset = fdset;
+
++ /* 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;
+
+@@ -2846,7 +2876,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;
+
+@@ -2869,7 +2899,6 @@ handle_connections (gnupg_fd_t listen_fd,
+
+ if (!shutdown_pending)
+ {
+- int idx;
+ ctrl_t ctrl;
+ npth_t thread;
+
diff --git a/debian/patches/gpg-agent-idling/0138-agent-Allow-threads-to-interrupt-main-select-loop-wi.patch b/debian/patches/gpg-agent-idling/0138-agent-Allow-threads-to-interrupt-main-select-loop-wi.patch
new file mode 100644
index 0000000..a1015bd
--- /dev/null
+++ b/debian/patches/gpg-agent-idling/0138-agent-Allow-threads-to-interrupt-main-select-loop-wi.patch
@@ -0,0 +1,101 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Tue, 1 Nov 2016 00:45:23 -0400
+Subject: agent: Allow threads to interrupt main select loop with SIGCONT.
+
+* agent/gpg-agent.c (interrupt_main_thread_loop): New function on
+non-windows platforms, allows other threads to interrupt the main loop
+if there's something that the main loop might be interested in.
+
+--
+
+For example, the main loop might be interested in changes in program
+state that affect the timers it expects to see.
+
+I don't know how to do this on Windows platforms, but i welcome any
+proposed improvements.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ agent/agent.h | 1 +
+ agent/gpg-agent.c | 19 ++++++++++++++++++-
+ 2 files changed, 19 insertions(+), 1 deletion(-)
+
+diff --git a/agent/agent.h b/agent/agent.h
+index 9ba7dc8..bca4e0b 100644
+--- a/agent/agent.h
++++ b/agent/agent.h
+@@ -347,6 +347,7 @@ void *get_agent_scd_notify_event (void);
+ #endif
+ void agent_sighup_action (void);
+ int map_pk_openpgp_to_gcry (int openpgp_algo);
++void interrupt_main_thread_loop (void);
+
+ /*-- command.c --*/
+ gpg_error_t agent_inq_pinentry_launched (ctrl_t ctrl, unsigned long pid,
+diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
+index 5d4611f..2240543 100644
+--- a/agent/gpg-agent.c
++++ b/agent/gpg-agent.c
+@@ -383,6 +383,9 @@ 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;
+
+@@ -2002,7 +2005,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);
+ }
+@@ -2328,6 +2331,10 @@ 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");
+@@ -2666,6 +2673,13 @@ start_connection_thread_ssh (void *arg)
+ }
+
+
++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
+@@ -2734,8 +2748,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. */
+@@ -2747,6 +2763,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)))
diff --git a/debian/patches/gpg-agent-idling/0139-agent-Avoid-tight-timer-tick-when-possible.patch b/debian/patches/gpg-agent-idling/0139-agent-Avoid-tight-timer-tick-when-possible.patch
new file mode 100644
index 0000000..f4a396a
--- /dev/null
+++ b/debian/patches/gpg-agent-idling/0139-agent-Avoid-tight-timer-tick-when-possible.patch
@@ -0,0 +1,87 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Tue, 1 Nov 2016 00:14:10 -0400
+Subject: agent: Avoid tight timer tick when possible.
+
+* agent/gpg-agent.c (need_tick): Evaluate whether the short-phase
+handle_tick() is needed.
+(handle_connections): On each cycle of the select loop, adjust whether
+we should call handle_tick() or not.
+* agent/call-scd.c (start_scd): Call interrupt_main_thread_loop() once
+the scdaemon thread context has started up.
+
+--
+
+With this change, an idle gpg-agent that has no scdaemon running only
+wakes up once a minute (to check_own_socket).
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ agent/call-scd.c | 4 +++-
+ agent/gpg-agent.c | 25 ++++++++++++++++++++++++-
+ 2 files changed, 27 insertions(+), 2 deletions(-)
+
+diff --git a/agent/call-scd.c b/agent/call-scd.c
+index ba59c18..1ac0f6b 100644
+--- a/agent/call-scd.c
++++ b/agent/call-scd.c
+@@ -407,7 +407,9 @@ start_scd (ctrl_t ctrl)
+
+ primary_scd_ctx = ctx;
+ primary_scd_ctx_reusable = 0;
+-
++ /* notify the main loop that something has changed */
++ interrupt_main_thread_loop ();
++
+ leave:
+ xfree (abs_homedir);
+ if (err)
+diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
+index 2240543..506bc95 100644
+--- a/agent/gpg-agent.c
++++ b/agent/gpg-agent.c
+@@ -2249,6 +2249,26 @@ 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. */
+@@ -2307,7 +2327,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)
+ {
+@@ -2843,6 +2863,9 @@ 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;
++
+ /* loop through all timers, fire any registered functions, and
+ plan next timer to trigger */
+ npth_clock_gettime (&curtime);
diff --git a/debian/patches/gpg-agent-idling/0140-agent-Avoid-scheduled-checks-on-socket-when-inotify-.patch b/debian/patches/gpg-agent-idling/0140-agent-Avoid-scheduled-checks-on-socket-when-inotify-.patch
new file mode 100644
index 0000000..482b818
--- /dev/null
+++ b/debian/patches/gpg-agent-idling/0140-agent-Avoid-scheduled-checks-on-socket-when-inotify-.patch
@@ -0,0 +1,26 @@
+From: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+Date: Tue, 1 Nov 2016 00:57:44 -0400
+Subject: agent: Avoid scheduled checks on socket when inotify is working.
+
+* agent/gpg-agent.c (handle_connections): When inotify is working, we
+do not need to schedule a timer to evaluate whether we control our own
+socket or not.
+
+Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
+---
+ agent/gpg-agent.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
+index 506bc95..4f2d711 100644
+--- a/agent/gpg-agent.c
++++ b/agent/gpg-agent.c
+@@ -2865,6 +2865,8 @@ handle_connections (gnupg_fd_t listen_fd,
+
+ /* 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 */
diff --git a/debian/patches/series b/debian/patches/series
index 30f2fab..0811b76 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -134,3 +134,12 @@ easy-keyservers/0119-dirmngr-Use-a-default-keyserver-if-none-is-explicitl.patch
0134-Change-all-http-www.gnu.org-in-license-notices-to-ht.patch
0135-common-w32-Simplify-locking.patch
0136-dirmngr-Improve-concurrency-in-the-non-adns-case.patch
+gpg-agent-idling/0137-agent-Create-framework-of-scheduled-timers.patch
+gpg-agent-idling/0138-agent-Allow-threads-to-interrupt-main-select-loop-wi.patch
+gpg-agent-idling/0139-agent-Avoid-tight-timer-tick-when-possible.patch
+gpg-agent-idling/0140-agent-Avoid-scheduled-checks-on-socket-when-inotify-.patch
+dirmngr-idling/0141-dirmngr-More-win32-system-daemon-cleanup.patch
+dirmngr-idling/0142-dirmngr-hkp-Avoid-potential-race-condition-when-some.patch
+dirmngr-idling/0143-dimrngr-Avoid-need-for-hkp-housekeeping.patch
+dirmngr-idling/0144-dirmngr-Drop-useless-housekeeping.patch
+dirmngr-idling/0145-dirmngr-Lazily-launch-ldap-reaper-thread.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gnupg2.git
More information about the Pkg-gnupg-commit
mailing list