This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix race between sem_post and semaphore destruction [BZ #12674]
- From: Rich Felker <dalias at libc dot org>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: Siddhesh Poyarekar <siddhesh at redhat dot com>, libc-alpha at sourceware dot org, carlos at redhat dot com
- Date: Thu, 22 May 2014 16:22:24 -0400
- Subject: Re: [PATCH] Fix race between sem_post and semaphore destruction [BZ #12674]
- Authentication-results: sourceware.org; auth=none
- References: <20140521110711 dot GA3598 at spoyarek dot pnq dot redhat dot com> <1400789406 dot 6984 dot 134 dot camel at triegel dot csb>
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