This is the mail archive of the
mailing list for the glibc project.
Re: [RFC] Propose fix for race conditions in pthread cancellation (bz#12683)
- From: Rich Felker <dalias at aerifal dot cx>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>, "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Fri, 12 Sep 2014 21:58:16 -0400
- Subject: Re: [RFC] Propose fix for race conditions in pthread cancellation (bz#12683)
- Authentication-results: sourceware.org; auth=none
- References: <5410C70E dot 70207 at linux dot vnet dot ibm dot com> <1410533066 dot 4967 dot 96 dot camel at triegel dot csb> <20140912153235 dot GP23797 at brightrain dot aerifal dot cx> <1410538290 dot 4967 dot 111 dot camel at triegel dot csb> <20140912171721 dot GR23797 at brightrain dot aerifal dot cx> <1410561872 dot 4967 dot 126 dot camel at triegel dot csb>
On Sat, Sep 13, 2014 at 12:44:32AM +0200, Torvald Riegel wrote:
> On Fri, 2014-09-12 at 13:17 -0400, Rich Felker wrote:
> > On Fri, Sep 12, 2014 at 06:11:30PM +0200, Torvald Riegel wrote:
> > > On Fri, 2014-09-12 at 11:32 -0400, Rich Felker wrote:
> > > > On Fri, Sep 12, 2014 at 04:44:26PM +0200, Torvald Riegel wrote:
> > > > > On Wed, 2014-09-10 at 18:47 -0300, Adhemerval Zanella wrote:
> > > > > "8. Adjust 'lowlevellock.h' arch-specific implementations to provide
> > > > > cancelable futex calls (used in libpthread code)."
> > > > >
> > > > > Only FUTEX_WAIT should be cancellable, I suppose.
> > > > >
> > > > > I've been thinking about whether it would be easier for the
> > > > > implementation of cancellable pthreads functions to just call a
> > > > > noncancellable FUTEX_WAIT and handle EINTR properly (by not retrying the
> > > > > wait but calling pthread_testcancel instead so that any pending
> > > > > cancellation is acted upon). However, this seems to save less
> > > > > complexity than just doing for FUTEX_WAIT what we do for all the other
> > > > > cancellable syscalls, so it's probably not worth it.
> > > >
> > > > This is not viable; it has race conditions, of the exact type which
> > > > the futex syscall and the new cancellation system are designed to
> > > > avoid.
> > >
> > > The new scheme would be used, but would always set the cancellation as
> > > pending. So what I thought about would primarily change the way we act
> > > on cancellation, not when it's considered to have happened. There's no
> > > real side effect in waiting, so futex_wait is a special case.
> > The problem is that you end up waiting unboundedly long, possibly
> > forever. Consider cancellation of a pthread_cond_wait that will never
> > be signaled, where the cancellation request arrives in the race
> > window.
> What I had in mind would interrupt the syscall like the new scheme
> proposes to do, so cancellation would happen like in the normal case.
> It would not be just checking a flag before/after a syscall. What would
> be different is changing that we just return the EINTR to the calling
> pthread code (e.g., the pthread_cond_wait) and letting it do the
> cancelling, instead of setting up a pthread_cond_wait-specific
> cancellation handler to do that.
> But that probably gives too little benefit compared to having a special
> case, so I doubt it's worth it. Nor worth a long discussion I guess :)
Actually, cancellation of pthread_cond_[timed]wait is complicated.
Depending on how unblocking a waiter works, it's possible that the
thread being cancelled has already "consumed the signal", and
therefore can't act on cancellation. This is a case where the program
counter at cancellation signal time is not sufficient to determine if
cancellation can be acted upon; the decision needs to be made later
based on userspace criteria (cond var state), not based on the
completion or non-completion of the futex syscall.
So, your idea about wanting futex to return even when cancelled is
basically right. However, EINTR cannot be the mechanism for this since
the signal has to be non-interrupting (like basically everything else,
futex restarts automatically for SA_RESTART signal handlers; you never
see EINTR). Even if you could get EINTR, there would still be a race
condition where you would fail to ever wakeup.
Addressing this in my implementation in musl is pending. I have a
patch that solves it via longjmp out of the cancellation handler, but
this is ugly and costly in the common case where cancellation never
happens. A nicer design I'm working on is having an alternate
cancellation mode where, rather than acting on cancellation when it's
triggered, the handler will disable cancellation then adjust the saved
program counter so that, on return from the signal handler, the
syscall appears to have failed with ECANCELED. The caller can then
decide what to do: just reenabling cancellation leaves it pending, or
it can be call pthread_testcancel() after reenabling and restoring the
cancellation mode in order to act on it.