This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING^2][RESEND][BZ #14958] Fix Concurrent reader deadlock in pthread_rwlock_rdlock().


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]