Re: [PATCH] Fix race between sem_post and semaphore destruction [BZ #12674]

[Rich Felker <dalias at libc dot org>]

On Thu, May 22, 2014 at 05:35:27AM +0530, Siddhesh Poyarekar wrote:
> On 22 May 2014 04:13, Rich Felker <dalias@libc.org> wrote:
> > I think the case I had in mind goes like this:
> >
> > 1. Thread A and B are waiting, val is -1.
> > 2. Thread C posts the semaphore and wakes up thread A, val is now 0.
> 
> This would be:
> 
> 2. Thread C posts the semaphore and val is now 1.  It sees that the
> old val was -1 and calls futex_wake, which wakes up thread A.
> 
> Similar to your implementation, sem_post increments the value by 1 if
> it is non-negative and by 2 if it is negative, so sem_post will never
> result in the value becoming 0.  However, there is one way in which
> the value will become 0, which is in sem_wait.  sem_wait will reset
> the value to 0 when it sees no waiters.  This could technically race
> with another waiter that has not yet called futex_wait, but that
> situation fixes itself since the futex_wait will return with
> EWOULDBLOCK and fix the value to -1 and call futex_wait again.

OK I think your reasoning on this sounds correct.

> > You cannot modify sem_post to set the value to -1 rather than 0 if
> > there's a waiter, because of two issues; one is fundamental and the
> > other is merely a performance concern:
> 
> sem_wait sets the value to -1, not sem_post.  sem_post always sets the
> value to something positive.

Yes, and sem_wait can legitimately inspect the waiters count since the
semaphore is still in use, and use it to determine whether to set -1
or 0. In fact my implementation is doing that now (in sem_trywait) so
it may be possible to just remove the secondary check in my code with
no other changes.

BTW the other confusing case I seem to remember is that waiters can
decrement without the semaphore value decrementing, as a result of
EINTR or ETIMEDOUT. This *might* have an impact on the logic but I
don't see right off how it would, and it's been a while since I put
much thought into it.

Rich