fix ABA race in cond vars, improve them overall

previously, a waiter could miss the 1->0 transition of block if
another thread set block to 1 again after the signal function set
block to 0. we now use the caller's thread id as a unique token to
store in block, which no other thread will ever write there. this
ensures that if block still contains the tid, no signal has occurred.
spurious wakeups will of course occur whenever there is a spurious
return from the futex wait and another thread has begun waiting on the
cond var. this should be a rare occurrence except perhaps in the
presence of interrupting signal handlers.

signal/bcast operations have been improved by noting that they need
not avoid inspecting the cond var's memory after changing the futex
value. because the standard allows spurious wakeups, there is no way
for an application to distinguish between a spurious wakeup just
before another thread called signal/bcast, and the deliberate wakeup
resulting from the signal/bcast call. thus the woken thread must
assume that the signalling thread may still be waiting to act on the
cond var, and therefore it cannot destroy/unmap the cond var.

diff --git a/src/thread/pthread_cond_broadcast.c b/src/thread/pthread_cond_broadcast.c
index 6002c53..dec9116 100644 (file)
--- a/src/thread/pthread_cond_broadcast.c
+++ b/src/thread/pthread_cond_broadcast.c
@@ -2,8 +2,7 @@
 
 int pthread_cond_broadcast(pthread_cond_t *c)
 {
 
 int pthread_cond_broadcast(pthread_cond_t *c)
 {

-       int w = c->_c_waiters;
-       if (a_swap(&c->_c_block, 0) || w)
-               __wake(&c->_c_block, -1, 0);
+       a_store(&c->_c_block, 0);
+       if (c->_c_waiters) __wake(&c->_c_block, -1, 0);

        return 0;
 }
        return 0;
 }

diff --git a/src/thread/pthread_cond_signal.c b/src/thread/pthread_cond_signal.c
index e8ed71c..b26b8ce 100644 (file)
--- a/src/thread/pthread_cond_signal.c
+++ b/src/thread/pthread_cond_signal.c
@@ -2,8 +2,7 @@
 
 int pthread_cond_signal(pthread_cond_t *c)
 {
 
 int pthread_cond_signal(pthread_cond_t *c)
 {

-       int w = c->_c_waiters;
-       if (a_swap(&c->_c_block, 0) || w)
-               __wake(&c->_c_block, 1, 0);
+       a_store(&c->_c_block, 0);
+       if (c->_c_waiters) __wake(&c->_c_block, 1, 0);

        return 0;
 }
        return 0;
 }

diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c
index ec5aa6f..c71edc9 100644 (file)
--- a/src/thread/pthread_cond_timedwait.c
+++ b/src/thread/pthread_cond_timedwait.c
@@ -15,19 +15,22 @@ static void cleanup(void *p)
 int pthread_cond_timedwait(pthread_cond_t *c, pthread_mutex_t *m, const struct timespec *ts)
 {
        struct cm cm = { .c=c, .m=m };
 int pthread_cond_timedwait(pthread_cond_t *c, pthread_mutex_t *m, const struct timespec *ts)
 {
        struct cm cm = { .c=c, .m=m };

-       int r, e=0;
+       int r, e, tid;

 
        if (ts && ts->tv_nsec >= 1000000000UL)
                return EINVAL;
 
        pthread_testcancel();
 
 
        if (ts && ts->tv_nsec >= 1000000000UL)
                return EINVAL;
 
        pthread_testcancel();
 

-       c->_c_block = 1;
+       a_inc(&c->_c_waiters);
+       c->_c_block = tid = pthread_self()->tid;
+

        if ((r=pthread_mutex_unlock(m))) return r;
 
        if ((r=pthread_mutex_unlock(m))) return r;
 

-       a_inc(&c->_c_waiters);
-       do e = __timedwait(&c->_c_block, 1, c->_c_clock, ts, cleanup, &cm, 0);
-       while (e == EINTR);
+       do e = __timedwait(&c->_c_block, tid, c->_c_clock, ts, cleanup, &cm, 0);
+       while (c->_c_block == tid && (!e || e==EINTR));
+       if (e == EINTR) e = 0;
+

        a_dec(&c->_c_waiters);
 
        if ((r=pthread_mutex_lock(m))) return r;
        a_dec(&c->_c_waiters);
 
        if ((r=pthread_mutex_lock(m))) return r;