[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] [RFC] Fix pthread timeout handling and cancellation issues
|
From: |
Richard Braun |
|
Subject: |
[PATCH] [RFC] Fix pthread timeout handling and cancellation issues |
|
Date: |
Sat, 2 Feb 2013 16:44:20 +0100 |
Here is the fixed version of my current work about fixing functions
that block for a limited time (e.g. pthread_cond_timedwait), and
cancellation, with additional description of what it is about.
This patch solves two issues. The first one is cancellation handling
when a cancellation request is sent before reaching a cancellation
point (namely, pthread_cond_{timed,}wait). Cancellation is implemented
by pushing an appropriate cleanup handler and switching to
PTHREAD_CANCEL_ASYNCHRONOUS type. The main problem is that it doesn't
handle pending requests, only a cancellation that occurs while blocking.
Other problems occur when trying to correctly handle a timeout and a
cancellation request through the cleanup routine.
The other issue is correctly handling timeouts. This problem was already
well known, as explained by the following comment :
"FIXME: What do we do if we get a wakeup message before we disconnect
ourself? It may remain until the next time we block. */"
In addition, the prevp thread member is inconsistently used. It is
sometimes accessed while protected by the appropriate queue lock to
determine whether a thread is still queued, while at times, threads
are unqueued without holding a lock, as in pthread_cond_broadcast :
/* We can safely walk the list of waiting threads without holding
the lock since it is now decoupled from the condition. */
__pthread_dequeuing_iterate (wakeup, wakeup)
__pthread_wakeup (wakeup);
This is the root cause that triggers some assertion failures.
The solution brought by this patch is to consistently use the prevp link
to determine if both a thread has been unqueued and if a wakeup message
has been sent (both are needed to wake up a thread). A thread unblocked
because of a timeout can now accurately determine if it needs to drain
its message queue. A direct improvement is that the message queue size
can be limited to one message, and wakeups are guaranteed to be
non-blocking, which allows safely calling __pthread_wakeup from critical
sections.
As it now affects the cleanup cancellation routine of
__pthread_cond_timedwait_internal, cancellation was reworked as well.
Cancellation type is forced to PTHREAD_CANCEL_DEFERRED during the call,
and actually checked on both entry and return. A hook is set by the
blocking thread so that the waker doesn't need to know about the call
implementation. Cancellation members are now protected with a mutex for
truely safe access.
---
pthread/pt-alloc.c | 3 +
pthread/pt-cancel.c | 27 +++++-
pthread/pt-internal.h | 8 ++
pthread/pt-join.c | 2 +
pthread/pt-setcancelstate.c | 2 +
pthread/pt-setcanceltype.c | 2 +
pthread/pt-testcancel.c | 7 +-
sysdeps/generic/pt-cond-brdcast.c | 8 +-
sysdeps/generic/pt-cond-signal.c | 23 ++---
sysdeps/generic/pt-cond-timedwait.c | 147 ++++++++++++++++++++++---------
sysdeps/generic/pt-mutex-timedlock.c | 47 ++++++----
sysdeps/generic/pt-rwlock-timedrdlock.c | 49 ++++++-----
sysdeps/generic/pt-rwlock-timedwrlock.c | 50 ++++++-----
sysdeps/generic/pt-rwlock-unlock.c | 13 ++-
sysdeps/generic/sem-timedwait.c | 51 ++++++-----
sysdeps/mach/hurd/pt-docancel.c | 2 +
sysdeps/mach/pt-thread-alloc.c | 4 +
sysdeps/mach/pt-wakeup.c | 4 +-
18 files changed, 290 insertions(+), 159 deletions(-)
diff --git a/pthread/pt-alloc.c b/pthread/pt-alloc.c
index 6af2da9..89fca8a 100644
--- a/pthread/pt-alloc.c
+++ b/pthread/pt-alloc.c
@@ -55,6 +55,9 @@ initialize_pthread (struct __pthread *new, int recycling)
if (err)
return err;
+ new->cancel_lock = (pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER;
+ new->cancel_hook = NULL;
+ new->cancel_hook_arg = NULL;
new->cancel_state = PTHREAD_CANCEL_ENABLE;
new->cancel_type = PTHREAD_CANCEL_DEFERRED;
new->cancel_pending = 0;
diff --git a/pthread/pt-cancel.c b/pthread/pt-cancel.c
index d19c557..96c77f7 100644
--- a/pthread/pt-cancel.c
+++ b/pthread/pt-cancel.c
@@ -31,10 +31,33 @@ pthread_cancel (pthread_t t)
if (! p)
return ESRCH;
+ __pthread_mutex_lock (&p->cancel_lock);
+ if (p->cancel_pending)
+ {
+ __pthread_mutex_unlock (&p->cancel_lock);
+ return 0;
+ }
+
p->cancel_pending = 1;
- if (p->cancel_state == PTHREAD_CANCEL_ENABLE
- && p->cancel_type == PTHREAD_CANCEL_ASYNCHRONOUS)
+
+ if (p->cancel_state != PTHREAD_CANCEL_ENABLE)
+ {
+ __pthread_mutex_unlock (&p->cancel_lock);
+ return 0;
+ }
+
+ if (p->cancel_type == PTHREAD_CANCEL_ASYNCHRONOUS)
+ /* CANCEL_LOCK is unlocked by this call. */
err = __pthread_do_cancel (p);
+ else
+ {
+ if (p->cancel_hook != NULL)
+ /* Thread blocking on a cancellation point. Invoke hook to unblock.
+ See __pthread_cond_timedwait_internal. */
+ p->cancel_hook (p->cancel_hook_arg);
+
+ __pthread_mutex_unlock (&p->cancel_lock);
+ }
return err;
}
diff --git a/pthread/pt-internal.h b/pthread/pt-internal.h
index 291baf5..1b31eea 100644
--- a/pthread/pt-internal.h
+++ b/pthread/pt-internal.h
@@ -73,6 +73,11 @@ struct __pthread
pthread_t thread;
/* Cancellation. */
+ pthread_mutex_t cancel_lock; /* Protect cancel_xxx members. */
+ void (*cancel_hook)(void *); /* Called to unblock a thread blocking
+ in a cancellation point (namely,
+ __pthread_cond_timedwait_internal). */
+ void *cancel_hook_arg;
int cancel_state;
int cancel_type;
int cancel_pending;
@@ -107,6 +112,8 @@ struct __pthread
tcbhead_t *tcb;
#endif /* ENABLE_TLS */
+ /* Queue links. Since PREVP is used to determine if a thread has been
+ awaken, it must be protected by the queue lock. */
struct __pthread *next, **prevp;
};
@@ -128,6 +135,7 @@ static inline void
__pthread_dequeue (struct __pthread *thread)
{
assert (thread);
+ assert (thread->prevp);
if (thread->next)
thread->next->prevp = thread->prevp;
diff --git a/pthread/pt-join.c b/pthread/pt-join.c
index 153058b..8bd2c6c 100644
--- a/pthread/pt-join.c
+++ b/pthread/pt-join.c
@@ -40,6 +40,8 @@ pthread_join (pthread_t thread, void **status)
pthread_cleanup_push ((void (*)(void *)) __pthread_mutex_unlock,
&pthread->state_lock);
+ /* Rely on pthread_cond_wait being a cancellation point to make
+ pthread_join one too. */
while (pthread->state == PTHREAD_JOINABLE)
pthread_cond_wait (&pthread->state_cond, &pthread->state_lock);
diff --git a/pthread/pt-setcancelstate.c b/pthread/pt-setcancelstate.c
index 38550ee..7b60015 100644
--- a/pthread/pt-setcancelstate.c
+++ b/pthread/pt-setcancelstate.c
@@ -35,9 +35,11 @@ __pthread_setcancelstate (int state, int *oldstate)
break;
}
+ __pthread_mutex_lock (&p->cancel_lock);
if (oldstate)
*oldstate = p->cancel_state;
p->cancel_state = state;
+ __pthread_mutex_unlock (&p->cancel_lock);
return 0;
}
diff --git a/pthread/pt-setcanceltype.c b/pthread/pt-setcanceltype.c
index 7226a3a..3cfbe9c 100644
--- a/pthread/pt-setcanceltype.c
+++ b/pthread/pt-setcanceltype.c
@@ -35,9 +35,11 @@ __pthread_setcanceltype (int type, int *oldtype)
break;
}
+ __pthread_mutex_lock (&p->cancel_lock);
if (oldtype)
*oldtype = p->cancel_type;
p->cancel_type = type;
+ __pthread_mutex_unlock (&p->cancel_lock);
return 0;
}
diff --git a/pthread/pt-testcancel.c b/pthread/pt-testcancel.c
index 01f1ac9..3ba07b6 100644
--- a/pthread/pt-testcancel.c
+++ b/pthread/pt-testcancel.c
@@ -25,7 +25,12 @@ void
pthread_testcancel (void)
{
struct __pthread *p = _pthread_self ();
+ int cancelled;
- if (p->cancel_state == PTHREAD_CANCEL_ENABLE && p->cancel_pending)
+ __pthread_mutex_lock (&p->cancel_lock);
+ cancelled = (p->cancel_state == PTHREAD_CANCEL_ENABLE) && p->cancel_pending;
+ __pthread_mutex_unlock (&p->cancel_lock);
+
+ if (cancelled)
pthread_exit (PTHREAD_CANCELED);
}
diff --git a/sysdeps/generic/pt-cond-brdcast.c
b/sysdeps/generic/pt-cond-brdcast.c
index 999cc2d..ad44f83 100644
--- a/sysdeps/generic/pt-cond-brdcast.c
+++ b/sysdeps/generic/pt-cond-brdcast.c
@@ -28,16 +28,12 @@ __pthread_cond_broadcast (pthread_cond_t *cond)
struct __pthread *wakeup;
__pthread_spin_lock (&cond->__lock);
+ __pthread_dequeuing_iterate (cond->__queue, wakeup)
+ __pthread_wakeup (wakeup);
- wakeup = cond->__queue;
cond->__queue = NULL;
__pthread_spin_unlock (&cond->__lock);
- /* We can safely walk the list of waiting threads without holding
- the lock since it is now decoupled from the condition. */
- __pthread_dequeuing_iterate (wakeup, wakeup)
- __pthread_wakeup (wakeup);
-
return 0;
}
diff --git a/sysdeps/generic/pt-cond-signal.c b/sysdeps/generic/pt-cond-signal.c
index d7c91e6..4b5450c 100644
--- a/sysdeps/generic/pt-cond-signal.c
+++ b/sysdeps/generic/pt-cond-signal.c
@@ -21,8 +21,10 @@
#include <pt-internal.h>
-static int
-cond_signal (struct __pthread_cond *cond, int *unblocked)
+/* Unblock at least one of the threads that are blocked on condition
+ variable COND. */
+int
+__pthread_cond_signal (pthread_cond_t *cond)
{
struct __pthread *wakeup;
@@ -33,24 +35,9 @@ cond_signal (struct __pthread_cond *cond, int *unblocked)
__pthread_spin_unlock (&cond->__lock);
if (wakeup)
- {
- /* We found a thread waiting for the condition to be signalled.
- Wake it up! */
- __pthread_wakeup (wakeup);
- *unblocked = 1;
- }
+ __pthread_wakeup (wakeup);
return 0;
}
-/* Unblock at least one of the threads that are blocked on condition
- variable COND. */
-int
-__pthread_cond_signal (pthread_cond_t *cond)
-{
- int unblocked = 0;
-
- return cond_signal (cond, &unblocked);
-}
-
strong_alias (__pthread_cond_signal, pthread_cond_signal);
diff --git a/sysdeps/generic/pt-cond-timedwait.c
b/sysdeps/generic/pt-cond-timedwait.c
index 56eb1ec..978b0f4 100644
--- a/sysdeps/generic/pt-cond-timedwait.c
+++ b/sysdeps/generic/pt-cond-timedwait.c
@@ -35,6 +35,32 @@ __pthread_cond_timedwait (pthread_cond_t *cond,
strong_alias (__pthread_cond_timedwait, pthread_cond_timedwait);
+struct cancel_ctx
+ {
+ struct __pthread *wakeup;
+ pthread_cond_t *cond;
+ };
+
+static void
+cancel_hook (void *arg)
+{
+ struct cancel_ctx *ctx = arg;
+ struct __pthread *wakeup = ctx->wakeup;
+ pthread_cond_t *cond = ctx->cond;
+ int unblock;
+
+ __pthread_spin_lock (&cond->__lock);
+ /* The thread only needs to be awaken if it's blocking or about to block.
+ If it was already unblocked, it's not queued any more. */
+ unblock = wakeup->prevp != NULL;
+ if (unblock)
+ __pthread_dequeue (wakeup);
+ __pthread_spin_unlock (&cond->__lock);
+
+ if (unblock)
+ __pthread_wakeup (wakeup);
+}
+
/* Block on condition variable COND until ABSTIME. As a GNU
extension, if ABSTIME is NULL, then wait forever. MUTEX should be
held by the calling thread. On return, MUTEX will be held by the
@@ -45,67 +71,108 @@ __pthread_cond_timedwait_internal (pthread_cond_t *cond,
const struct timespec *abstime)
{
error_t err;
- int canceltype;
+ int cancelled, oldtype, drain;
clockid_t clock_id = __pthread_default_condattr.clock;
- void cleanup (void *arg)
+ if (abstime && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+ return EINVAL;
+
+ struct __pthread *self = _pthread_self ();
+ struct cancel_ctx ctx;
+ ctx.wakeup= self;
+ ctx.cond = cond;
+
+ /* Test for a pending cancellation request, switch to deferred mode for
+ safer resource handling, and prepare the hook to call in case we're
+ cancelled while blocking. Once CANCEL_LOCK is released, the cancellation
+ hook can be called by another thread at any time. Whatever happens,
+ this function must exit with MUTEX locked.
+
+ This function contains inline implementations of pthread_testcancel and
+ pthread_setcanceltype to reduce locking overhead. */
+ __pthread_mutex_lock (&self->cancel_lock);
+ cancelled = (self->cancel_state == PTHREAD_CANCEL_ENABLE)
+ && self->cancel_pending;
+
+ if (! cancelled)
{
- struct __pthread *self = _pthread_self ();
+ self->cancel_hook = cancel_hook;
+ self->cancel_hook_arg = &ctx;
+ oldtype = self->cancel_type;
+
+ if (oldtype != PTHREAD_CANCEL_DEFERRED)
+ self->cancel_type = PTHREAD_CANCEL_DEFERRED;
+ /* Add ourselves to the list of waiters. This is done while setting
+ the cancellation hook to simplify the cancellation procedure, i.e.
+ if the thread is queued, it can be cancelled, otherwise it is
+ already unblocked, progressing on the return path. */
__pthread_spin_lock (&cond->__lock);
- if (self->prevp)
- __pthread_dequeue (self);
+ __pthread_enqueue (&cond->__queue, self);
+ if (cond->__attr)
+ clock_id = cond->__attr->clock;
__pthread_spin_unlock (&cond->__lock);
-
- pthread_setcanceltype (canceltype, &canceltype);
- __pthread_mutex_lock (mutex);
}
+ __pthread_mutex_unlock (&self->cancel_lock);
- if (abstime && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
- return EINVAL;
-
- struct __pthread *self = _pthread_self ();
-
- /* Add ourselves to the list of waiters. */
- __pthread_spin_lock (&cond->__lock);
- __pthread_enqueue (&cond->__queue, self);
- if (cond->__attr)
- clock_id = cond->__attr->clock;
- __pthread_spin_unlock (&cond->__lock);
+ if (cancelled)
+ pthread_exit (PTHREAD_CANCELED);
+ /* Release MUTEX before blocking. */
__pthread_mutex_unlock (mutex);
- /* Enter async cancelation mode. If cancelation is disabled, then
- this does not change anything which is exactly what we want. */
- pthread_cleanup_push (cleanup, 0);
- pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, &canceltype);
-
+ /* Block the thread. */
if (abstime)
+ err = __pthread_timedblock (self, abstime, clock_id);
+ else
{
- err = __pthread_timedblock (self, abstime, clock_id);
- if (err)
- /* We timed out. We may need to disconnect ourself from the
- waiter queue.
-
- FIXME: What do we do if we get a wakeup message before we
- disconnect ourself? It may remain until the next time we
- block. */
+ err = 0;
+ __pthread_block (self);
+ }
+
+ __pthread_spin_lock (&cond->__lock);
+ if (! self->prevp)
+ {
+ /* Another thread removed us from the list of waiters, which means a
+ wakeup message has been sent. It was either consumed while we were
+ blocking, or queued after we timed out and before we acquired the
+ condition lock, in which case the message queue must be drained. */
+ if (! err)
+ drain = 0;
+ else
{
assert (err == ETIMEDOUT);
-
- __pthread_spin_lock (&mutex->__lock);
- if (self->prevp)
- __pthread_dequeue (self);
- __pthread_spin_unlock (&mutex->__lock);
+ drain = 1;
}
}
else
{
- err = 0;
- __pthread_block (self);
+ /* We're still in the list of waiters. Noone attempted to wake us up,
+ i.e. we timed out. */
+ assert (err == ETIMEDOUT);
+ __pthread_dequeue (self);
+ drain = 0;
}
+ __pthread_spin_unlock (&cond->__lock);
+
+ if (drain)
+ __pthread_block (self);
+
+ /* We're almost done. Remove the unblock hook, restore the previous
+ cancellation type, and check for a pending cancellation request. */
+ __pthread_mutex_lock (&self->cancel_lock);
+ self->cancel_hook = NULL;
+ self->cancel_hook_arg = NULL;
+ self->cancel_type = oldtype;
+ cancelled = (self->cancel_state == PTHREAD_CANCEL_ENABLE)
+ && self->cancel_pending;
+ __pthread_mutex_unlock (&self->cancel_lock);
+
+ /* Reacquire MUTEX before returning/cancelling. */
+ __pthread_mutex_lock (mutex);
- pthread_cleanup_pop (1);
+ if (cancelled)
+ pthread_exit (PTHREAD_CANCELED);
return err;
}
diff --git a/sysdeps/generic/pt-mutex-timedlock.c
b/sysdeps/generic/pt-mutex-timedlock.c
index 48bffaf..43e0eda 100644
--- a/sysdeps/generic/pt-mutex-timedlock.c
+++ b/sysdeps/generic/pt-mutex-timedlock.c
@@ -30,6 +30,8 @@ int
__pthread_mutex_timedlock_internal (struct __pthread_mutex *mutex,
const struct timespec *abstime)
{
+ error_t err;
+ int drain;
struct __pthread *self;
const struct __pthread_mutexattr *attr = mutex->attr;
@@ -127,30 +129,37 @@ __pthread_mutex_timedlock_internal (struct
__pthread_mutex *mutex,
/* Block the thread. */
if (abstime)
+ err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
+ else
{
- error_t err;
-
- err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
- if (err)
- /* We timed out. We may need to disconnect ourself from the
- waiter queue.
+ err = 0;
+ __pthread_block (self);
+ }
- FIXME: What do we do if we get a wakeup message before we
- disconnect ourself? It may remain until the next time we
- block. */
- {
- assert (err == ETIMEDOUT);
+ __pthread_spin_lock (&mutex->__lock);
+ if (! self->prevp)
+ /* Another thread removed us from the queue, which means a wakeup message
+ has been sent. It was either consumed while we were blocking, or
+ queued after we timed out and before we acquired the mutex lock, in
+ which case the message queue must be drained. */
+ drain = err ? 1 : 0;
+ else
+ {
+ /* We're still in the queue. Noone attempted to wake us up, i.e. we
+ timed out. */
+ __pthread_dequeue (self);
+ drain = 0;
+ }
+ __pthread_spin_unlock (&mutex->__lock);
- __pthread_spin_lock (&mutex->__lock);
- if (self->prevp)
- __pthread_dequeue (self);
- __pthread_spin_unlock (&mutex->__lock);
+ if (drain)
+ __pthread_block (self);
- return err;
- }
+ if (err)
+ {
+ assert (err == ETIMEDOUT);
+ return err;
}
- else
- __pthread_block (self);
#if !defined(ALWAYS_TRACK_MUTEX_OWNER)
if (attr && attr->mutex_type != PTHREAD_MUTEX_NORMAL)
diff --git a/sysdeps/generic/pt-rwlock-timedrdlock.c
b/sysdeps/generic/pt-rwlock-timedrdlock.c
index a110213..a81ca71 100644
--- a/sysdeps/generic/pt-rwlock-timedrdlock.c
+++ b/sysdeps/generic/pt-rwlock-timedrdlock.c
@@ -29,6 +29,8 @@ int
__pthread_rwlock_timedrdlock_internal (struct __pthread_rwlock *rwlock,
const struct timespec *abstime)
{
+ error_t err;
+ int drain;
struct __pthread *self;
__pthread_spin_lock (&rwlock->__lock);
@@ -70,32 +72,37 @@ __pthread_rwlock_timedrdlock_internal (struct
__pthread_rwlock *rwlock,
/* Block the thread. */
if (abstime)
+ err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
+ else
{
- error_t err;
-
- err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
- if (err)
- /* We timed out. We may need to disconnect ourself from the
- waiter queue.
-
- FIXME: What do we do if we get a wakeup message before we
- disconnect ourself? It may remain until the next time we
- block. */
- {
- assert (err == ETIMEDOUT);
-
- __pthread_spin_lock (&rwlock->__lock);
- if (self->prevp)
- /* Disconnect ourself. */
- __pthread_dequeue (self);
- __pthread_spin_unlock (&rwlock->__lock);
-
- return err;
- }
+ err = 0;
+ __pthread_block (self);
}
+
+ __pthread_spin_lock (&rwlock->__lock);
+ if (! self->prevp)
+ /* Another thread removed us from the queue, which means a wakeup message
+ has been sent. It was either consumed while we were blocking, or
+ queued after we timed out and before we acquired the rwlock lock, in
+ which case the message queue must be drained. */
+ drain = err ? 1 : 0;
else
+ {
+ /* We're still in the queue. Noone attempted to wake us up, i.e. we
+ timed out. */
+ __pthread_dequeue (self);
+ drain = 0;
+ }
+ __pthread_spin_unlock (&rwlock->__lock);
+
+ if (drain)
__pthread_block (self);
+ if (err)
+ {
+ assert (err == ETIMEDOUT);
+ return err;
+ }
/* The reader count has already been increment by whoever woke us
up. */
diff --git a/sysdeps/generic/pt-rwlock-timedwrlock.c
b/sysdeps/generic/pt-rwlock-timedwrlock.c
index a5cc579..e47e936 100644
--- a/sysdeps/generic/pt-rwlock-timedwrlock.c
+++ b/sysdeps/generic/pt-rwlock-timedwrlock.c
@@ -29,6 +29,8 @@ int
__pthread_rwlock_timedwrlock_internal (struct __pthread_rwlock *rwlock,
const struct timespec *abstime)
{
+ error_t err;
+ int drain;
struct __pthread *self;
__pthread_spin_lock (&rwlock->__lock);
@@ -56,32 +58,38 @@ __pthread_rwlock_timedwrlock_internal (struct
__pthread_rwlock *rwlock,
/* Block the thread. */
if (abstime)
+ err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
+ else
{
- error_t err;
-
- err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
- if (err)
- /* We timed out. We may need to disconnect ourself from the
- waiter queue.
-
- FIXME: What do we do if we get a wakeup message before we
- disconnect ourself? It may remain until the next time we
- block. */
- {
- assert (err == ETIMEDOUT);
-
- __pthread_spin_lock (&rwlock->__lock);
- if (self->prevp)
- /* Disconnect ourself. */
- __pthread_dequeue (self);
- __pthread_spin_unlock (&rwlock->__lock);
-
- return err;
- }
+ err = 0;
+ __pthread_block (self);
}
+
+ __pthread_spin_lock (&rwlock->__lock);
+ if (! self->prevp)
+ /* Another thread removed us from the queue, which means a wakeup message
+ has been sent. It was either consumed while we were blocking, or
+ queued after we timed out and before we acquired the rwlock lock, in
+ which case the message queue must be drained. */
+ drain = err ? 1 : 0;
else
+ {
+ /* We're still in the queue. Noone attempted to wake us up, i.e. we
+ timed out. */
+ __pthread_dequeue (self);
+ drain = 0;
+ }
+ __pthread_spin_unlock (&rwlock->__lock);
+
+ if (drain)
__pthread_block (self);
+ if (err)
+ {
+ assert (err == ETIMEDOUT);
+ return err;
+ }
+
assert (rwlock->readers == 0);
return 0;
diff --git a/sysdeps/generic/pt-rwlock-unlock.c
b/sysdeps/generic/pt-rwlock-unlock.c
index fb23a0b..212cca5 100644
--- a/sysdeps/generic/pt-rwlock-unlock.c
+++ b/sysdeps/generic/pt-rwlock-unlock.c
@@ -65,19 +65,16 @@ pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
if (rwlock->readerqueue)
{
- __pthread_queue_iterate (rwlock->readerqueue, wakeup)
- rwlock->readers ++;
+ __pthread_dequeuing_iterate (rwlock->readerqueue, wakeup)
+ {
+ rwlock->readers ++;
+ __pthread_wakeup (wakeup);
+ }
- wakeup = rwlock->readerqueue;
rwlock->readerqueue = 0;
__pthread_spin_unlock (&rwlock->__lock);
- /* We can safely walk the list of waiting threads without holding
- the lock since it is now decoupled from the rwlock. */
- __pthread_dequeuing_iterate (wakeup, wakeup)
- __pthread_wakeup (wakeup);
-
return 0;
}
diff --git a/sysdeps/generic/sem-timedwait.c b/sysdeps/generic/sem-timedwait.c
index 94e6dee..7ab1583 100644
--- a/sysdeps/generic/sem-timedwait.c
+++ b/sysdeps/generic/sem-timedwait.c
@@ -27,6 +27,8 @@ int
__sem_timedwait_internal (sem_t *restrict sem,
const struct timespec *restrict timeout)
{
+ error_t err;
+ int drain;
struct __pthread *self;
__pthread_spin_lock (&sem->__lock);
@@ -52,32 +54,39 @@ __sem_timedwait_internal (sem_t *restrict sem,
/* Block the thread. */
if (timeout)
+ err = __pthread_timedblock (self, timeout, CLOCK_REALTIME);
+ else
{
- error_t err;
-
- err = __pthread_timedblock (self, timeout, CLOCK_REALTIME);
- if (err)
- /* We timed out. We may need to disconnect ourself from the
- waiter queue.
-
- FIXME: What do we do if we get a wakeup message before we
- disconnect ourself? It may remain until the next time we
- block. */
- {
- assert (err == ETIMEDOUT);
-
- __pthread_spin_lock (&sem->__lock);
- if (self->prevp)
- __pthread_dequeue (self);
- __pthread_spin_unlock (&sem->__lock);
-
- errno = err;
- return -1;
- }
+ err = 0;
+ __pthread_block (self);
}
+
+ __pthread_spin_lock (&sem->__lock);
+ if (! self->prevp)
+ /* Another thread removed us from the queue, which means a wakeup message
+ has been sent. It was either consumed while we were blocking, or
+ queued after we timed out and before we acquired the semaphore lock, in
+ which case the message queue must be drained. */
+ drain = err ? 1 : 0;
else
+ {
+ /* We're still in the queue. Noone attempted to wake us up, i.e. we
+ timed out. */
+ __pthread_dequeue (self);
+ drain = 0;
+ }
+ __pthread_spin_unlock (&sem->__lock);
+
+ if (drain)
__pthread_block (self);
+ if (err)
+ {
+ assert (err == ETIMEDOUT);
+ errno = err;
+ return -1;
+ }
+
return 0;
}
diff --git a/sysdeps/mach/hurd/pt-docancel.c b/sysdeps/mach/hurd/pt-docancel.c
index 105c6fd..b3a5507 100644
--- a/sysdeps/mach/hurd/pt-docancel.c
+++ b/sysdeps/mach/hurd/pt-docancel.c
@@ -36,6 +36,8 @@ __pthread_do_cancel (struct __pthread *p)
assert (p->cancel_pending == 1);
assert (p->cancel_state == PTHREAD_CANCEL_ENABLE);
+ __pthread_mutex_unlock (&p->cancel_lock);
+
ktid = __mach_thread_self ();
me = p->kernel_thread == ktid;
__mach_port_deallocate (__mach_task_self (), ktid);
diff --git a/sysdeps/mach/pt-thread-alloc.c b/sysdeps/mach/pt-thread-alloc.c
index 3d7c046..794f63e 100644
--- a/sysdeps/mach/pt-thread-alloc.c
+++ b/sysdeps/mach/pt-thread-alloc.c
@@ -55,6 +55,10 @@ create_wakeupmsg (struct __pthread *thread)
return EAGAIN;
}
+ /* No need to queue more than one wakeup message on this port. */
+ mach_port_set_qlimit (__mach_task_self (),
+ thread->wakeupmsg.msgh_remote_port, 1);
+
return 0;
}
diff --git a/sysdeps/mach/pt-wakeup.c b/sysdeps/mach/pt-wakeup.c
index 4920d10..95fdbf9 100644
--- a/sysdeps/mach/pt-wakeup.c
+++ b/sysdeps/mach/pt-wakeup.c
@@ -31,8 +31,8 @@ __pthread_wakeup (struct __pthread *thread)
{
error_t err;
- err = __mach_msg (&thread->wakeupmsg, MACH_SEND_MSG,
+ err = __mach_msg (&thread->wakeupmsg, MACH_SEND_MSG | MACH_SEND_TIMEOUT,
sizeof (thread->wakeupmsg), 0, MACH_PORT_NULL,
- MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+ 0 , MACH_PORT_NULL);
assert_perror (err);
}
--
1.7.10.4
- [PATCH] [RFC] Fix pthread timeout handling and cancellation issues,
Richard Braun <=