Philip Martin wrote on Mon, Feb 01, 2016 at 11:20:04 +0000:
> Philip Martin <philip.mar...@wandisco.com> writes:
>
> > That change is broken for two reasons:
> >
> > - the same handler is used for all those signals but we always exit via
> > SIGINT
> >
> > - except apr_signal does not return APR_SUCCESS so we never exit via
> > SIGINT.
>
> Fixed by r1727916.
> + if (cancelled)
> + {
> + apr_signal(signum_cancelled, SIG_DFL);
> + /* No APR support for getpid() so cannot use apr_proc_kill(). */
> + kill(getpid(), signum_cancelled);
> + }
There seems to be a race condition here: if this block is entered with
(signum_cancelled == SIGINT), and between the apr_signal() call and
evaluating the arguments to the kill() call, a SIGTERM is received and
changes the value of signum_cancelled, the kill() call will send
a SIGTERM signal to current process while the SIGTERM handler will still
be SIG_IGN (the value installed by our handler).
I think we have two options to fix this: either make signum_cancelled
a write-once variable (never write to it if it's nonzero), or have the
handler install SIG_IGN handlers for all signals we catch, not just for
one of them. Or perhaps both? The former approach seems more robust
but the latter seems like a good idea in its own right.
> To see the effect of this run the following at a sh prompt:
>
> while :;do echo sleep;sleep 3;echo log;svn log
> http://svn.apache.org/repos/asf;done
>
> Using control-C to send SIGINT will exit the loop only if sent while
> sleep is running. A SIGINT during log will cancel the svn command but
> the loop will continue and a second SIGINT during sleep will be needed
> to exit the loop.
Cheers,
Daniel
