summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2011-09-23 22:58:45 -0400
committerRich Felker <dalias@aerifal.cx>2011-09-23 22:58:45 -0400
commit97c5b5a87c3d9df54278e1073d6177f77536bd32 (patch)
treef93711dd0ab36697fb5a56d81b4f439a7b8eca68
parentc41a76f58ce0238172effe982f2cee7bbd2a60a4 (diff)
downloadmusl-97c5b5a87c3d9df54278e1073d6177f77536bd32.tar.gz
musl-97c5b5a87c3d9df54278e1073d6177f77536bd32.tar.bz2
musl-97c5b5a87c3d9df54278e1073d6177f77536bd32.tar.xz
musl-97c5b5a87c3d9df54278e1073d6177f77536bd32.zip
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.
-rw-r--r--src/thread/pthread_cond_broadcast.c5
-rw-r--r--src/thread/pthread_cond_signal.c5
-rw-r--r--src/thread/pthread_cond_timedwait.c13
3 files changed, 12 insertions, 11 deletions
diff --git a/src/thread/pthread_cond_broadcast.c b/src/thread/pthread_cond_broadcast.c
index 6002c535..dec91164 100644
--- 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 e8ed71cc..b26b8ce5 100644
--- 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 ec5aa6f4..c71edc95 100644
--- 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;