summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2011-09-26 13:14:41 -0400
committerRich Felker <dalias@aerifal.cx>2011-09-26 13:14:41 -0400
commit3bec53e0d3bb5e74d2e2dca34f50aadfaf832607 (patch)
tree0ecc760d1a47571f61869bc43b243b4c4a2fdcab
parent1fa05210100caefc8546746e08358d81739f4b41 (diff)
downloadmusl-3bec53e0d3bb5e74d2e2dca34f50aadfaf832607.tar.gz
musl-3bec53e0d3bb5e74d2e2dca34f50aadfaf832607.tar.bz2
musl-3bec53e0d3bb5e74d2e2dca34f50aadfaf832607.tar.xz
musl-3bec53e0d3bb5e74d2e2dca34f50aadfaf832607.zip
another cond var fix: requeue count race condition
lock out new waiters during the broadcast. otherwise the wait count added to the mutex might be lower than the actual number of waiters moved, and wakeups may be lost. this issue could also be solved by temporarily setting the mutex waiter count higher than any possible real count, then relying on the kernel to tell us how many waiters were requeued, and updating the counts afterwards. however the logic is more complex, and i don't really trust the kernel. the solution here is also nice in that it replaces some atomic cas loops with simple non-atomic ops under lock.
-rw-r--r--src/thread/pthread_cond_broadcast.c8
-rw-r--r--src/thread/pthread_cond_timedwait.c20
2 files changed, 13 insertions, 15 deletions
diff --git a/src/thread/pthread_cond_broadcast.c b/src/thread/pthread_cond_broadcast.c
index 9c6a462b..848e288f 100644
--- a/src/thread/pthread_cond_broadcast.c
+++ b/src/thread/pthread_cond_broadcast.c
@@ -22,12 +22,8 @@ int pthread_cond_broadcast(pthread_cond_t *c)
m = c->_c_mutex;
/* Move waiter count to the mutex */
- for (;;) {
- int w = c->_c_waiters2;
- a_fetch_add(&m->_m_waiters, w);
- if (a_cas(&c->_c_waiters2, w, 0) == w) break;
- a_fetch_add(&m->_m_waiters, -w);
- }
+ a_fetch_add(&m->_m_waiters, c->_c_waiters2);
+ c->_c_waiters2 = 0;
/* Perform the futex requeue, waking one waiter unless we know
* that the calling thread holds the mutex. */
diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c
index e3dc8147..db2744ba 100644
--- a/src/thread/pthread_cond_timedwait.c
+++ b/src/thread/pthread_cond_timedwait.c
@@ -7,8 +7,6 @@ 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. */
@@ -20,10 +18,8 @@ static void unwait(pthread_cond_t *c, pthread_mutex_t *m)
while (a_swap(&c->_c_lock, 1))
__wait(&c->_c_lock, &c->_c_lockwait, 1, 1);
- /* Atomically decrement waiters2 if positive, else mutex waiters. */
- do w = c->_c_waiters2;
- while (w && a_cas(&c->_c_waiters2, w, w-1)!=w);
- if (!w) a_dec(&m->_m_waiters);
+ if (c->_c_waiters2) c->_c_waiters2--;
+ else a_dec(&m->_m_waiters);
a_store(&c->_c_lock, 0);
if (c->_c_lockwait) __wake(&c->_c_lock, 1, 1);
@@ -46,10 +42,16 @@ int pthread_cond_timedwait(pthread_cond_t *c, pthread_mutex_t *m, const struct t
pthread_testcancel();
- if (c->_c_mutex != (void *)-1) c->_c_mutex = m;
-
a_inc(&c->_c_waiters);
- a_inc(&c->_c_waiters2);
+
+ if (c->_c_mutex != (void *)-1) {
+ c->_c_mutex = m;
+ while (a_swap(&c->_c_lock, 1))
+ __wait(&c->_c_lock, &c->_c_lockwait, 1, 1);
+ c->_c_waiters2++;
+ a_store(&c->_c_lock, 0);
+ if (c->_c_lockwait) __wake(&c->_c_lock, 1, 1);
+ }
seq = c->_c_seq;