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 10:10:06PM +0200, Torvald Riegel wrote:
> > - sem_getvalue is patched to lie about the actual value if it sees the
> >   -1 and return 0.
> 
> OT: We should add a note there about having to clarify the ordering
> guarantees that this gives.  This is effectively an mo_relaxed load, so
> very weak ordering guarantees; OTOH, POSIX seems to want very strong
> ordering guarantees for most of its sync operations.  So, I think we
> either need to clarify in POSIX or make this at least an acquire load or
> such.

AFAIK POSIX does not impose any synchronization on sem_getvalue.

> > diff --git a/nptl/sysdeps/unix/sysv/linux/sem_post.c b/nptl/sysdeps/unix/sysv/linux/sem_post.c
> > index 4906adf..0ff1699 100644
> > --- a/nptl/sysdeps/unix/sysv/linux/sem_post.c
> > +++ b/nptl/sysdeps/unix/sysv/linux/sem_post.c
> > @@ -30,24 +30,35 @@ int
> >  __new_sem_post (sem_t *sem)
> >  {
> >    struct new_sem *isem = (struct new_sem *) sem;
> > +  int incr, is_private = isem->private;
> >  
> >    __typeof (isem->value) cur;
> >    do
> >      {
> >        cur = isem->value;
> > +      incr = 1 + (cur < 0);
> >        if (isem->value == SEM_VALUE_MAX)
> >  	{
> >  	  __set_errno (EOVERFLOW);
> >  	  return -1;
> >  	}
> >      }
> > -  while (atomic_compare_and_exchange_bool_rel (&isem->value, cur + 1, cur));
> > +  while (atomic_compare_and_exchange_bool_rel (&isem->value, cur + incr, cur));
> >  
> >    atomic_full_barrier ();
> 
> Why do you still need the full barrier?  AFAICS, it was necessary before
> because of the Dekker-like synchronization between nwaiters and value --
> but you've changed that (or not?).

Per POSIX, all functions which synchronize memory are full barriers.

Rich