This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PING^2][RESEND][BZ #14958] Fix Concurrent reader deadlock in pthread_rwlock_rdlock().
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: libc-alpha at sourceware dot org
- Cc: daniel dot stodden at gmail dot com
- Date: Wed, 26 Feb 2014 12:27:36 +0100
- Subject: [PING^2][RESEND][BZ #14958] Fix Concurrent reader deadlock in pthread_rwlock_rdlock().
- Authentication-results: sourceware.org; auth=none
- References: <20140213155231 dot GA16546 at domone dot podge> <20140219130037 dot GC28501 at domone dot podge>
On Wed, Feb 19, 2014 at 02:00:37PM +0100, OndÅej BÃlka wrote:
> ping
>
> On Thu, Feb 13, 2014 at 04:52:31PM +0100, OndÅej BÃlka wrote:
> > Hi, this is another bug with patch in bugzilla. I changed there few
> > style issues. Original description follows:
> >
> > Rich, I pushed a candidate fix to
> > https://github.com/dns42/glibc/commit/b7725621b5e2101d4218be0d09bd55a67b4d3512
> >
> > It explores variant 1 for upstream.
> >
> > Code presently looks correct to me, but I won't get to test it before
> > later. I'll post an update accordingly :)
> >
> > Additional comments for discussion:
> >
> > - Note that prerequisites can be checked comparatively precisely.
> > Possibly more precisely than you anticipated, taking
> > __nr_readers_queued, __nr_writers_queued and __nr_readers
> > into account, all at the same time.
> >
> > - This works because both queues need to come together, then
> > preempted, any other case looks clean to me.
> >
> > All queued loops take the ll lock and atomically decrement, then
> > succeed. So the whole combination of __nr_*_queued and __nr_readers
> > can work as a discriminant.
> >
> > - As you mentioned, the timedlock cases indeed deserve special
> > attention.
> >
> > I think the way they've been written would render the assumptions
> > made by the above patch correct as well.
> >
> > But only if rwlock_timedlock calls are really correct.
> >
> > Can it be that lll_futex_timed_wait does *not*
> > leave an inherent race between timeout and a (too-)late wakeup call?
> > In the same way pthread_cond_timedwait must?
> >
> > I'm not a futex-expert, but the problem here is that I currently
> > don't believe this is even possible. In other words: How does
> > present pthread_rwlock_unlock tell that any of __nr_writers_queued
> > have been woken up before timing out?
> >
> > I almost suspect it can't, doesn't, and -- since timedrwlock makes
> > no attempt to restart the queue on timeout either,
> > suffers from a similar race. But this would probably belong into
> > another ticket.
> >
> > Apologies in advance if I'm mistaken.
> >
> >
> > * nptl/pthread_rwlock_rdlock.c (__pthread_rwlock_rdlock):
> > Wake threads when necessary.
> >
> > diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
> > index 3773f7d..b2b679f 100644
> > --- a/nptl/pthread_rwlock_rdlock.c
> > +++ b/nptl/pthread_rwlock_rdlock.c
> > @@ -29,7 +29,7 @@ int
> > __pthread_rwlock_rdlock (rwlock)
> > pthread_rwlock_t *rwlock;
> > {
> > - int result = 0;
> > + int result = 0, wake = 0;
> >
> > LIBC_PROBE (rdlock_entry, 1, rwlock);
> >
> > @@ -44,6 +44,14 @@ __pthread_rwlock_rdlock (rwlock)
> > && (!rwlock->__data.__nr_writers_queued
> > || PTHREAD_RWLOCK_PREFER_READER_P (rwlock)))
> > {
> > + /* May have prempted a queued writer. Since readers are
> > + preferred, proceed. But wake any queued readers, too. */
> > + wake = !rwlock->__data.__nr_readers
> > + && rwlock->__data.__nr_writers_queued
> > + && rwlock->__data.__nr_readers_queued;
> > + if (__glibc_unlikely (wake))
> > + ++rwlock->__data.__readers_wakeup;
> > +
> > /* Increment the reader counter. Avoid overflow. */
> > if (__glibc_unlikely (++rwlock->__data.__nr_readers == 0))
> > {
> > @@ -93,6 +101,10 @@ __pthread_rwlock_rdlock (rwlock)
> > /* We are done, free the lock. */
> > lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared);
> >
> > + if (__glibc_unlikely (wake))
> > + lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX,
> > + rwlock->__data.__shared);
> > +
> > return result;
> > }
> >
>
> --
>
> manager in the cable duct
--
HTTPD Error 4004 : very old Intel cpu - insufficient processing power