security-and-privacy/gnupg
security-and-privacy/gnupg
1
0
Fork 0
mirror of git://git.gnupg.org/gnupg.git synced 2025-02-01 16:33:02 +01:00
Code Activity

dirmngr: Sleep in the ldap wrapper thread.

Browse Source 
* dirmngr/ldap-wrapper.c (wrapper_list): Rename to reaper_list.
(ldap_reaper_thread): Protect all list modification with a mutex.  Use
a condition var to wake up the reaper thread.

Signed-off-by: Werner Koch <wk@gnupg.org>
This commit is contained in:
Werner Koch 2018-04-27 15:20:45 +02:00
parent f9fbfc64e4
commit a598bbeeaf
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
1 changed files with 312 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

287
dirmngr/ldap-wrapper.c
View File

@ -111,14 +111,20 @@ struct wrapper_context_s
/* We keep a global list of spawned wrapper process. A separate thread /* We keep a global list of spawned wrapper process. A separate
makes use of this list to log error messages and to watch out for * thread makes use of this list to log error messages and to watch
finished processes. */ * out for finished processes. Access to list is protected by a
static struct wrapper_context_s *wrapper_list; * mutex. The condition variable is used to wakeup the reaper
* thread. */
static struct wrapper_context_s *reaper_list;
static npth_mutex_t reaper_list_mutex = NPTH_MUTEX_INITIALIZER;
static npth_cond_t reaper_run_cond = NPTH_COND_INITIALIZER;
/* We need to know whether we are shutting down the process. */ /* We need to know whether we are shutting down the process. */
static int shutting_down; static int shutting_down;
/* Close the estream fp and set it to NULL. */ /* Close the estream fp and set it to NULL. */
#define SAFE_CLOSE(fp) \ #define SAFE_CLOSE(fp) \
do { estream_t _fp = fp; es_fclose (_fp); fp = NULL; } while (0) do { estream_t _fp = fp; es_fclose (_fp); fp = NULL; } while (0)
@ -126,6 +132,26 @@ static int shutting_down;
static void
lock_reaper_list (void)
{
if (npth_mutex_lock (&reaper_list_mutex))
log_fatal ("%s: failed to acquire mutex: %s\n", __func__,
gpg_strerror (gpg_error_from_syserror ()));
}
static void
unlock_reaper_list (void)
{
if (npth_mutex_unlock (&reaper_list_mutex))
log_fatal ("%s: failed to release mutex: %s\n", __func__,
gpg_strerror (gpg_error_from_syserror ()));
}
/* Read a fixed amount of data from READER into BUFFER. */ /* Read a fixed amount of data from READER into BUFFER. */
static gpg_error_t static gpg_error_t
read_buffer (ksba_reader_t reader, unsigned char *buffer, size_t count) read_buffer (ksba_reader_t reader, unsigned char *buffer, size_t count)
@ -234,8 +260,13 @@ read_log_data (struct wrapper_context_s *ctx)
if (rc || !n) /* Error or EOF. */ if (rc || !n) /* Error or EOF. */
{ {
if (rc) if (rc)
{
gpg_error_t err = gpg_error_from_syserror ();
if (gpg_err_code (err) == GPG_ERR_EAGAIN)
return 0;
log_error (_("error reading log from ldap wrapper %d: %s\n"), log_error (_("error reading log from ldap wrapper %d: %s\n"),
(int)ctx->pid, strerror (errno)); (int)ctx->pid, gpg_strerror (err));
}
print_log_line (ctx, NULL); /* Flush. */ print_log_line (ctx, NULL); /* Flush. */
SAFE_CLOSE (ctx->log_fp); SAFE_CLOSE (ctx->log_fp);
return 1; return 1;
@ -252,7 +283,7 @@ read_log_data (struct wrapper_context_s *ctx)
/* This function is run by a separate thread to maintain the list of /* This function is run by a separate thread to maintain the list of
wrappers and to log error messages from these wrappers. */ wrappers and to log error messages from these wrappers. */
void * void *
ldap_wrapper_thread (void *dummy) ldap_reaper_thread (void *dummy)
{ {
gpg_error_t err; gpg_error_t err;
struct wrapper_context_s *ctx; struct wrapper_context_s *ctx;
@ -263,7 +294,7 @@ ldap_wrapper_thread (void *dummy)
int millisecs; int millisecs;
gpgrt_poll_t *fparray = NULL; gpgrt_poll_t *fparray = NULL;
int fparraysize = 0; int fparraysize = 0;
int count; int count, i;
int ret; int ret;
time_t exptime; time_t exptime;
@ -272,15 +303,65 @@ ldap_wrapper_thread (void *dummy)
npth_clock_gettime (&abstime); npth_clock_gettime (&abstime);
abstime.tv_sec += TIMERTICK_INTERVAL; abstime.tv_sec += TIMERTICK_INTERVAL;
/* FIXME: When we are idle (i.e. !COUNT) we should not use the
* TIMERTICK_INTERVAL but wait on a mutex to avoid unnecessary
* wakeups. */
restart:
for (;;) for (;;)
{ {
int any_action = 0; int any_action = 0;
/* Wait until we are needed and then setup the FPARRAY. */
/* Note: There is one unlock inside the block! */
lock_reaper_list ();
{
while (!reaper_list && !shutting_down)
{
if (npth_cond_wait (&reaper_run_cond, &reaper_list_mutex))
log_error ("ldap-reaper: waiting on condition failed: %s\n",
gpg_strerror (gpg_error_from_syserror ()));
}
for (count = 0, ctx = reaper_list; ctx; ctx = ctx->next)
if (ctx->log_fp)
count++;
if (count > fparraysize || !fparray)
{
/* Need to realloc the array. We simply discard it and
* replace it by a new one. */
xfree (fparray);
fparray = xtrycalloc (count? count : 1, sizeof *fparray);
if (!fparray)
{
err = gpg_error_from_syserror ();
log_error ("ldap-reaper can't allocate poll array: %s"
" - waiting 1s\n", gpg_strerror (err));
/* Note: Here we unlock and continue! */
unlock_reaper_list ();
npth_sleep (1);
continue;
}
fparraysize = count;
}
for (count = 0, ctx = reaper_list; ctx; ctx = ctx->next)
{
if (ctx->log_fp)
{
log_assert (count < fparraysize);
fparray[count].stream = ctx->log_fp;
fparray[count].want_read = 1;
fparray[count].ignore = 0;
ctx->reaper_idx = count;
count++;
}
else
{
ctx->reaper_idx = -1;
fparray[count].ignore = 1;
}
}
for (i=count; i < fparraysize; i++)
fparray[i].ignore = 1;
}
unlock_reaper_list (); /* Note the one unlock inside the block. */
/* Compute the next timeout. */
npth_clock_gettime (&curtime); npth_clock_gettime (&curtime);
if (!(npth_timercmp (&curtime, &abstime, <))) if (!(npth_timercmp (&curtime, &abstime, <)))
{ {
@ -294,54 +375,10 @@ ldap_wrapper_thread (void *dummy)
if (millisecs < 0) if (millisecs < 0)
millisecs = 1; millisecs = 1;
/* Setup FPARRAY. */
for (count = 0, ctx = wrapper_list; ctx; ctx = ctx->next)
if (ctx->log_fp)
count++;
if (DBG_EXTPROG) if (DBG_EXTPROG)
{
log_debug ("ldap-reaper: next run (count=%d size=%d, timeout=%d)\n", log_debug ("ldap-reaper: next run (count=%d size=%d, timeout=%d)\n",
count, fparraysize, millisecs); count, fparraysize, millisecs);
if (count > fparraysize || !fparray)
{
/* Need to realloc the array. We simply discard it and
* replace it by a new one. */
xfree (fparray);
fparray = xtrycalloc (count? count : 1, sizeof *fparray);
if (!fparray)
{
err = gpg_error_from_syserror ();
log_error ("ldap-reaper can't allocate poll array: %s"
" - waiting 1s\n", gpg_strerror (err));
npth_sleep (1);
continue;
}
fparraysize = count;
}
for (count = 0, ctx = wrapper_list; ctx; ctx = ctx->next)
{
if (ctx->log_fp)
{
if (count > fparraysize)
{
/* Another thread added more items to WRAPPER_LIST.
* Note that the calloc above is a system call and
* thus may caused a context switch. */
goto restart;
}
fparray[count].stream = ctx->log_fp;
fparray[count].want_read = 1;
fparray[count].ignore = 0;
ctx->reaper_idx = count;
count++;
}
else
ctx->reaper_idx = -1;
}
for (; count < fparraysize; count++)
fparray[count].ignore = 1;
if (DBG_EXTPROG)
{
for (count=0; count < fparraysize; count++) for (count=0; count < fparraysize; count++)
if (!fparray[count].ignore) if (!fparray[count].ignore)
log_debug ("ldap-reaper: fp[%d] stream=%p want=%d\n", log_debug ("ldap-reaper: fp[%d] stream=%p want=%d\n",
@ -368,8 +405,15 @@ ldap_wrapper_thread (void *dummy)
{ {
for (count=0; count < fparraysize; count++) for (count=0; count < fparraysize; count++)
if (!fparray[count].ignore) if (!fparray[count].ignore)
log_debug ("ldap-reaper: fp[%d] stream=%p got=%d\n", log_debug ("ldap-reaper: fp[%d] stream=%p r=%d %c%c%c%c%c%c%c\n",
count, fparray[count].stream, fparray[count].got_read); count, fparray[count].stream, ret,
fparray[count].got_read? 'r':'-',
fparray[count].got_write?'w':'-',
fparray[count].got_oob? 'o':'-',
fparray[count].got_rdhup?'H':'-',
fparray[count].got_err? 'e':'-',
fparray[count].got_hup? 'h':'-',
fparray[count].got_nval? 'n':'-');
} }
/* All timestamps before exptime should be considered expired. */ /* All timestamps before exptime should be considered expired. */
@ -377,15 +421,13 @@ ldap_wrapper_thread (void *dummy)
if (exptime > INACTIVITY_TIMEOUT) if (exptime > INACTIVITY_TIMEOUT)
exptime -= INACTIVITY_TIMEOUT; exptime -= INACTIVITY_TIMEOUT;
/* Note that there is no need to lock the list because we always lock_reaper_list ();
add entries at the head (with a pending event status) and {
thus traversing the list will even work if we have a context for (ctx = reaper_list; ctx; ctx = ctx->next)
switch in waitpid. */
for (ctx = wrapper_list; ctx; ctx = ctx->next)
{ {
/* Check whether there is any logging to be done. We need /* Check whether there is any logging to be done. We need
* to check FPARRAYSIZE because it can be 0 in case es_poll * to check FPARRAYSIZE because it can be 0 in case
* returned a timeout. */ * es_poll returned a timeout. */
if (fparraysize && ctx->log_fp && ctx->reaper_idx >= 0) if (fparraysize && ctx->log_fp && ctx->reaper_idx >= 0)
{ {
log_assert (ctx->reaper_idx < fparraysize); log_assert (ctx->reaper_idx < fparraysize);
@ -452,12 +494,12 @@ ldap_wrapper_thread (void *dummy)
} }
/* If something has been printed to the log file or we got an /* If something has been printed to the log file or we got an
EOF from a wrapper, we now print the list of active * EOF from a wrapper, we now print the list of active
wrappers. */ * wrappers. */
if (any_action && DBG_EXTPROG) if (any_action && DBG_EXTPROG)
{ {
log_debug ("ldap worker stati:\n"); log_debug ("ldap worker stati:\n");
for (ctx = wrapper_list; ctx; ctx = ctx->next) for (ctx = reaper_list; ctx; ctx = ctx->next)
log_debug (" c=%p pid=%d/%d rdr=%p logfp=%p" log_debug (" c=%p pid=%d/%d rdr=%p logfp=%p"
" ctrl=%p/%d la=%lu rdy=%d\n", " ctrl=%p/%d la=%lu rdy=%d\n",
ctx, ctx,
@ -467,25 +509,27 @@ ldap_wrapper_thread (void *dummy)
(unsigned long)ctx->stamp, ctx->ready); (unsigned long)ctx->stamp, ctx->ready);
} }
/* An extra loop to check whether ready marked wrappers may be
/* Use an extra loop to check whether ready marked wrappers may * removed. We may only do so if the ksba reader object is
* be removed. We may only do so if the ksba reader object is
* not anymore in use or we are in shutdown state. */ * not anymore in use or we are in shutdown state. */
again: again:
for (ctx_prev=NULL, ctx=wrapper_list; ctx; ctx_prev=ctx, ctx=ctx->next) for (ctx_prev=NULL, ctx=reaper_list; ctx; ctx_prev=ctx, ctx=ctx->next)
{
if (ctx->ready if (ctx->ready
&& ((!ctx->log_fp && !ctx->reader) || shutting_down)) && ((!ctx->log_fp && !ctx->reader) || shutting_down))
{ {
if (ctx_prev) if (ctx_prev)
ctx_prev->next = ctx->next; ctx_prev->next = ctx->next;
else else
wrapper_list = ctx->next; reaper_list = ctx->next;
destroy_wrapper (ctx); destroy_wrapper (ctx);
/* We need to restart because destroy_wrapper might have
* done a context switch. */
goto again; goto again;
} }
} }
}
unlock_reaper_list ();
}
/*NOTREACHED*/ /*NOTREACHED*/
return NULL; /* Make the compiler happy. */ return NULL; /* Make the compiler happy. */
} }
@ -494,7 +538,7 @@ ldap_wrapper_thread (void *dummy)
/* Start the reaper thread for the ldap wrapper. */ /* Start the reaper thread for the ldap wrapper. */
void void
ldap_wrapper_launch_thread (void) ldap_reaper_launch_thread (void)
{ {
static int done; static int done;
npth_attr_t tattr; npth_attr_t tattr;
@ -505,14 +549,21 @@ ldap_wrapper_launch_thread (void)
return; return;
done = 1; done = 1;
#ifdef HAVE_W32_SYSTEM
/* Static init does not yet work in W32 nPth. */
if (npth_cond_init (&reaper_run_cond, NULL))
log_fatal ("%s: failed to init condition variabale: %s\n",
__func__, gpg_strerror (gpg_error_from_syserror ()));
#endif
npth_attr_init (&tattr); npth_attr_init (&tattr);
npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED); npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED);
err = npth_create (&thread, &tattr, ldap_wrapper_thread, NULL); if (npth_create (&thread, &tattr, ldap_reaper_thread, NULL))
if (err)
{ {
log_error (_("error spawning ldap wrapper reaper thread: %s\n"), err = gpg_error_from_syserror ();
strerror (err) ); log_error ("error spawning ldap reaper reaper thread: %s\n",
gpg_strerror (err) );
dirmngr_exit (1); dirmngr_exit (1);
} }
npth_setname_np (thread, "ldap-reaper"); npth_setname_np (thread, "ldap-reaper");
@ -526,9 +577,15 @@ ldap_wrapper_launch_thread (void)
void void
ldap_wrapper_wait_connections () ldap_wrapper_wait_connections ()
{ {
lock_reaper_list ();
{
shutting_down = 1; shutting_down = 1;
/* FIXME: This is a busy wait. */ if (npth_cond_signal (&reaper_run_cond))
while (wrapper_list) log_error ("%s: Ooops: signaling condition failed: %s\n",
__func__, gpg_strerror (gpg_error_from_syserror ()));
}
unlock_reaper_list ();
while (reaper_list)
npth_usleep (200); npth_usleep (200);
} }
@ -543,12 +600,14 @@ ldap_wrapper_release_context (ksba_reader_t reader)
if (!reader ) if (!reader )
return; return;
for (ctx=wrapper_list; ctx; ctx=ctx->next) lock_reaper_list ();
{
for (ctx=reaper_list; ctx; ctx=ctx->next)
if (ctx->reader == reader) if (ctx->reader == reader)
{ {
if (DBG_EXTPROG) if (DBG_EXTPROG)
log_debug ("releasing ldap worker c=%p pid=%d/%d rdr=%p ctrl=%p/%d\n", log_debug ("releasing ldap worker c=%p pid=%d/%d rdr=%p"
ctx, " ctrl=%p/%d\n", ctx,
(int)ctx->pid, (int)ctx->printable_pid, (int)ctx->pid, (int)ctx->printable_pid,
ctx->reader, ctx->reader,
ctx->ctrl, ctx->ctrl? ctx->ctrl->refcount:0); ctx->ctrl, ctx->ctrl? ctx->ctrl->refcount:0);
@ -561,12 +620,15 @@ ldap_wrapper_release_context (ksba_reader_t reader)
ctx->ctrl = NULL; ctx->ctrl = NULL;
} }
if (ctx->fp_err) if (ctx->fp_err)
log_info (_("reading from ldap wrapper %d failed: %s\n"), log_info ("%s: reading from ldap wrapper %d failed: %s\n",
ctx->printable_pid, gpg_strerror (ctx->fp_err)); __func__, ctx->printable_pid, gpg_strerror (ctx->fp_err));
break; break;
} }
}
unlock_reaper_list ();
} }
/* Cleanup all resources held by the connection associated with /* Cleanup all resources held by the connection associated with
CTRL. This is used after a cancel to kill running wrappers. */ CTRL. This is used after a cancel to kill running wrappers. */
void void
@ -574,7 +636,9 @@ ldap_wrapper_connection_cleanup (ctrl_t ctrl)
{ {
struct wrapper_context_s *ctx; struct wrapper_context_s *ctx;
for (ctx=wrapper_list; ctx; ctx=ctx->next) lock_reaper_list ();
{
for (ctx=reaper_list; ctx; ctx=ctx->next)
if (ctx->ctrl && ctx->ctrl == ctrl) if (ctx->ctrl && ctx->ctrl == ctrl)
{ {
ctx->ctrl->refcount--; ctx->ctrl->refcount--;
@ -582,9 +646,11 @@ ldap_wrapper_connection_cleanup (ctrl_t ctrl)
if (ctx->pid != (pid_t)(-1)) if (ctx->pid != (pid_t)(-1))
gnupg_kill_process (ctx->pid); gnupg_kill_process (ctx->pid);
if (ctx->fp_err) if (ctx->fp_err)
log_info (_("reading from ldap wrapper %d failed: %s\n"), log_info ("%s: reading from ldap wrapper %d failed: %s\n",
ctx->printable_pid, gpg_strerror (ctx->fp_err)); __func__, ctx->printable_pid, gpg_strerror (ctx->fp_err));
} }
}
unlock_reaper_list ();
} }
@ -650,6 +716,12 @@ reader_callback (void *cb_value, char *buffer, size_t count, size_t *nread)
if (millisecs < 0) if (millisecs < 0)
millisecs = 1; millisecs = 1;
if (DBG_EXTPROG)
{
log_debug ("%s: fp[0] stream=%p want=%d\n",
__func__, fparray[0].stream,fparray[0].want_read);
}
ret = es_poll (fparray, DIM (fparray), millisecs); ret = es_poll (fparray, DIM (fparray), millisecs);
if (ret < 0) if (ret < 0)
{ {
@ -659,6 +731,18 @@ reader_callback (void *cb_value, char *buffer, size_t count, size_t *nread)
SAFE_CLOSE (ctx->fp); SAFE_CLOSE (ctx->fp);
return -1; return -1;
} }
if (DBG_EXTPROG)
{
log_debug ("%s: fp[0] stream=%p r=%d %c%c%c%c%c%c%c\n",
__func__, fparray[0].stream, ret,
fparray[0].got_read? 'r':'-',
fparray[0].got_write?'w':'-',
fparray[0].got_oob? 'o':'-',
fparray[0].got_rdhup?'H':'-',
fparray[0].got_err? 'e':'-',
fparray[0].got_hup? 'h':'-',
fparray[0].got_nval? 'n':'-');
}
if (!ret) if (!ret)
{ {
/* Timeout. Will be handled when calculating the next timeout. */ /* Timeout. Will be handled when calculating the next timeout. */
@ -672,9 +756,16 @@ reader_callback (void *cb_value, char *buffer, size_t count, size_t *nread)
if (es_read (ctx->fp, buffer, nleft, &n)) if (es_read (ctx->fp, buffer, nleft, &n))
{ {
ctx->fp_err = gpg_error_from_syserror (); ctx->fp_err = gpg_error_from_syserror ();
if (gpg_err_code (ctx->fp_err) == GPG_ERR_EAGAIN)
ctx->fp_err = 0;
else
{
log_error ("%s: error reading: %s (%d)\n",
__func__, gpg_strerror (ctx->fp_err), ctx->fp_err);
SAFE_CLOSE (ctx->fp); SAFE_CLOSE (ctx->fp);
return -1; return -1;
} }
}
else if (!n) /* EOF */ else if (!n) /* EOF */
{ {
if (nleft == count) if (nleft == count)
@ -726,7 +817,7 @@ ldap_wrapper (ctrl_t ctrl, ksba_reader_t *reader, const char *argv[])
wrapper module to do the logging on its own. Given that we anyway wrapper module to do the logging on its own. Given that we anyway
need a way to reap the child process and this is best done using a 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. */ general reaping thread, that thread can do the logging too. */
ldap_wrapper_launch_thread (); ldap_reaper_launch_thread ();
*reader = NULL; *reader = NULL;
@ -799,9 +890,17 @@ ldap_wrapper (ctrl_t ctrl, ksba_reader_t *reader, const char *argv[])
} }
/* Hook the context into our list of running wrappers. */ /* Hook the context into our list of running wrappers. */
lock_reaper_list ();
{
ctx->reader = *reader; ctx->reader = *reader;
ctx->next = wrapper_list; ctx->next = reaper_list;
wrapper_list = ctx; reaper_list = ctx;
if (npth_cond_signal (&reaper_run_cond))
log_error ("ldap-wrapper: Ooops: signaling condition failed: %s (%d)\n",
gpg_strerror (gpg_error_from_syserror ()), errno);
}
unlock_reaper_list ();
if (DBG_EXTPROG) if (DBG_EXTPROG)
log_debug ("ldap wrapper %d started (%p, %s)\n", log_debug ("ldap wrapper %d started (%p, %s)\n",
(int)ctx->pid, ctx->reader, pgmname); (int)ctx->pid, ctx->reader, pgmname);