[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 07 Feb 2016 00:22:04 +0000

Philip Martin wrote on Mon, Feb 01, 2016 at 11:20:04 +0000:
> Philip Martin <philip.martin_at_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
Received on 2016-02-07 01:22:07 CET

This is an archived mail posted to the Subversion Dev mailing list.