subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <...@daniel.shahaf.name>
Subject Re: svn commit: r1727621 - in /subversion/trunk/subversion: svn/svn.c svnadmin/svnadmin.c svnbench/svnbench.c svnfsfs/svnfsfs.c svnlook/svnlook.c svnrdump/svnrdump.c svnsync/svnsync.c
Date Sun, 07 Feb 2016 00:22:04 GMT
Philip Martin wrote on Mon, Feb 01, 2016 at 11:20:04 +0000:
> Philip Martin <philip.martin@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

Mime
View raw message