[PATCH 2/5] dirmngr: hkp: avoid race condition when some hosts die

[Daniel Kahn Gillmor]

On Tue 2016-11-01 07:06:06 -0400, Werner Koch wrote:
> On Tue,  1 Nov 2016 01:33, dkg at fifthhorseman.net said:
>
>> Multiple threads may write to hosttable[x]->dead while
>> select_random_host() is running.  For example, a housekeeping thread
>
> Nope.  Please look at the code:
>
>   for (idx=0, tblsize=0; (pidx = table[idx]) != -1; idx++)
>     if (hosttable[pidx] && !hosttable[pidx]->dead)
>       tblsize++;
>   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
>     pidx = tbl[get_uint_nonce () % tblsize];
>
> and explain where you see a syscall which may switch to another thread.
> Neither xtrymalloc nor get_unit_nonce are such syscalls or supposed to
> use nPth wrapper syscalls.

Sorry, i wasn't aware that the only place where a thread could execute
concurrently was in a call into the kernel.  Will npth never provide
concurrent execution on any multiprocessor architecture?

I also wasn't aware that xtrymalloc() was guaranteed to never call
brk().

At any rate, while i think this particular patch improves readability
and maintainability of the code, i'm happy to rebase this series without
it -- the only difference, i suppose, is that i'll have to remove the
log_info() in host_is_alive() so that we don't accidentally create a
syscall while testing (i'll also add a comment warning that this
function must never be modified to make syscalls so that future editors
don't get tripped up).

Would you like me to send such a rebased patch queue?

        --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 930 bytes
Desc: not available
URL: </pipermail/attachments/20161101/3fdfac37/attachment.sig>