redo cond vars again, use sequence numbers

testing revealed that the old implementation, while correct, was
giving way too many spurious wakeups due to races changing the value
of the condition futex. in a test program with 5 threads receiving
broadcast signals, the number of returns from pthread_cond_wait was
roughly 3 times what it should have been (2 spurious wakeups for every
legitimate wakeup). moreover, the magnitude of this effect seems to
grow with the number of threads.

the old implementation may also have had some nasty race conditions
with reuse of the cond var with a new mutex.

the new implementation is based on incrementing a sequence number with
each signal event. this sequence number has nothing to do with the
number of threads intended to be woken; it's only used to provide a
value for the futex wait to avoid deadlock. in theory there is a
danger of race conditions due to the value wrapping around after 2^32
signals. it would be nice to eliminate that, if there's a way.

testing showed no spurious wakeups (though they are of course
possible) with the new implementation, as well as slightly improved
performance.

1 files changed, 28 insertions, 16 deletions

diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c
index 9616dd85..e9b5e2fc 100644
--- a/src/thread/pthread_cond_timedwait.c
+++ b/src/thread/pthread_cond_timedwait.c
@@ -7,17 +7,22 @@ struct cm {
 
 static void unwait(pthread_cond_t *c, pthread_mutex_t *m)
 {
-	int w;
+	/* Removing a waiter is non-trivial if we could be using requeue
+	 * based broadcast signals, due to mutex access issues, etc. */
 
-	/* Cannot leave waiting status if there are any live broadcasters
-	 * which might be inspecting/using the mutex. */
-	while ((w=c->_c_bcast)) __wait(&c->_c_bcast, &c->_c_leavers, w, 0);
+	if (c->_c_mutex == (void *)-1) {
+		a_dec(&c->_c_waiters);
+		return;
+	}
 
-	/* If the waiter count is zero, it must be the case that the
-	 * caller's count has been moved to the mutex due to bcast. */
-	do w = c->_c_waiters;
-	while (w && a_cas(&c->_c_waiters, w, w-1)!=w);
-	if (!w) a_dec(&m->_m_waiters);
+	while (a_swap(&c->_c_lock, 1))
+		__wait(&c->_c_lock, &c->_c_lockwait, 1, 1);
+
+	if (c->_c_waiters) c->_c_waiters--;
+	else a_dec(&m->_m_waiters);
+
+	a_store(&c->_c_lock, 0);
+	if (c->_c_lockwait) __wake(&c->_c_lock, 1, 1);
 }
 
 static void cleanup(void *p)
@@ -30,28 +35,35 @@ 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, tid;
+	int r, e=0, seq;
 
 	if (ts && ts->tv_nsec >= 1000000000UL)
 		return EINVAL;
 
 	pthread_testcancel();
 
-	if (c->_c_mutex != (void *)-1) c->_c_mutex = m;
+	if (c->_c_mutex == (void *)-1) {
+		a_inc(&c->_c_waiters);
+	} else {
+		c->_c_mutex = m;
+		while (a_swap(&c->_c_lock, 1))
+			__wait(&c->_c_lock, &c->_c_lockwait, 1, 1);
+		c->_c_waiters++;
+		a_store(&c->_c_lock, 0);
+		if (c->_c_lockwait) __wake(&c->_c_lock, 1, 1);
+	}
 
-	a_inc(&c->_c_waiters);
-	c->_c_block = tid = pthread_self()->tid;
+	seq = c->_c_seq;
 
 	if ((r=pthread_mutex_unlock(m))) return r;
 
-	do e = __timedwait(&c->_c_block, tid, c->_c_clock, ts, cleanup, &cm, 0);
-	while (c->_c_block == tid && (!e || e==EINTR));
+	do e = __timedwait(&c->_c_seq, seq, c->_c_clock, ts, cleanup, &cm, 0);
+	while (c->_c_seq == seq && (!e || e==EINTR));
 	if (e == EINTR) e = 0;
 
 	unwait(c, m);
 
 	if ((r=pthread_mutex_lock(m))) return r;
 
-	pthread_testcancel();
 	return e;
 }