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 6002c53507be55152328caacd3394614daa0dffd..dec91164065d844b67b2af27d8994b24e64e2d53 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 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;
 }

diff --git a/src/thread/pthread_cond_signal.c b/src/thread/pthread_cond_signal.c
index e8ed71cc6a1f6a7c140501c994841cbcb1e2f28a..b26b8ce55e5507dc28b28e0f38595388295e3f7f 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 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;
 }

diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c
index ec5aa6f467c1bae07b70f8ba805f7bfc97cde612..c71edc95bc8274473de5056447d384eea9af4c62 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 r, e=0;
+       int r, e, tid;
 
        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;
 
-       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;