[Bug libc/14958] Concurrent reader deadlock in pthread_rwlock_rdlock()

["daniel.stodden at gmail dot com" <sourceware-bugzilla at sourceware dot org>]

http://sourceware.org/bugzilla/show_bug.cgi?id=14958

--- Comment #3 from Daniel Stodden <daniel.stodden at gmail dot com> 2012-12-16 09:12:29 UTC ---
(In reply to comment #2)
>
> 1. The easy way is to just always use broadcast wakes and let readers and
> writers duke it out for who gets the lock next. NPTL's synchronization
> primitives are riddled with bugs from trying to be too smart (avoiding spurious
> wakes, etc.) and missing important corner cases, and I would not be sad to see
> all this ripped out in favor of something that's definitively robust; however,
> I recognize I'm probably in the minority here.

Yes. This is also the way we're working around the issue until a general fix
becomes available.

The snipped below depends too much on nptl internals to be recommendable
for general use. Posted because it might save the day for those who happen to 
know exactly what they link against.

#include <pthread.h>
#include <limits.h>
#include <unistd.h>
#include <syscall.h>
#include <linux/futex.h>

int
pthread_rwlock_rdlock(pthread_rwlock_t *rwlock)
{
    extern typeof(pthread_rwlock_rdlock) __pthread_rwlock_rdlock;
    int err;

    err = __pthread_rwlock_rdlock(rwlock);
    if (err)
        return err;

    /* NB. Up to least eglibc 2.15, an nptl read lock acquired 
         * doesn't guarantee that all readers expecting to
     * run get woken up. Fixed by rerunning the wakeup call from
     * within. */

    if (rwlock->__data.__nr_readers_queued) {
        /* NB. nasty to resort to atomic ops, but this spares
         * us reimplementing the ll lock while maintaining
         * increment stride. */
        (void)__sync_add_and_fetch(&rwlock->__data.__readers_wakeup, 1);

        /* NB. 'in libc.so or ld.so all futexes are private.' */
        syscall(SYS_futex,
            &rwlock->__data.__readers_wakeup, INT_MAX,
            FUTEX_WAKE | FUTEX_PRIVATE_FLAG,
            NULL, NULL, 0);
    }

    return 0;
}

> 2. The way that's harder to get right but that should give better performance
> is for the reader that "steals" the lock from waiting writers to detect this
> situation and broadcast wake the futex so that all the other waiting readers
> also wake up and get a chance to obtain read locks. It seems highly nontrivial
> to get all the corner cases right here, especially since waiting writers can be
> performing a timedlock operation and could be gone by the time the reader
> "steals" the lock from them.

I agree that detecting writer preemption based on present state is not
possible.

Still, a modified version more carefully negotiating between queued/woken
writers and lock release (to maintain writer starvation avoidance) certainly
could be made. Especially since arbitration through a single mutex makes this
relatively straightforward to implement.

(I must say I was quite suprised how much the current implementation is
avoiding fetch/add or cas operations to achieve this.)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.