diff options
author | Richard Braun <rbraun@sceen.net> | 2013-02-03 16:06:51 +0100 |
---|---|---|
committer | Richard Braun <rbraun@sceen.net> | 2013-02-04 00:07:13 +0100 |
commit | 5ab516af0779a7fd74ad893c7c67960df6ede065 (patch) | |
tree | 568a297810b4ba52c351c3f79276205b0b87f20e | |
parent | 14eeff65f732d77bc8d93bd5e1979fa6083b66d8 (diff) |
Fix pthread timeout handling and cancellation issues
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 is 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 (initialize_pthread): Initialize the new `cancel_lock',
`cancel_hook' and `cancel_hook_args' fields.
* pthread/pt-cancel.c (pthread_cancel): Rework cancellation handling.
* pthread/pt-internal.h (struct __pthread): Add `cancel_lock', `cancel_hook'
and `cancel_hook_args' fields.
(__pthread_dequeue): Assert thread->prevp isn't NULL.
* pthread/pt-join.c (pthread_join): Describe how the cancellation point is
implemented.
* pthread/pt-setcancelstate.c (__pthread_setcancelstate): Lock the given
thread cancellation lock when switching state.
* pthread/pt-setcanceltype.c (__pthread_setcanceltype): Likewise for
cancellation type.
* pthread/pt-testcancel.c (pthread_testcancel): Likewise for pending
cancellations.
* sysdeps/generic/pt-cond-brdcast.c (__pthread_cond_broadcast): Dequeue
and wake up threads with condition locked.
* sysdeps/generic/pt-cond-signal.c (cond_signal): Remove function, move
implementation to ...
(__pthread_cond_signal): ... this function. Remove unused `unblocked'
variable.
* sysdeps/generic/pt-cond-timedwait.c (struct cancel_ctx): New structure.
(cancel_hook): New static function.
(__pthread_cond_timedwait_internal): Fix cancellation and timeout handling.
* sysdeps/generic/pt-mutex-timedlock.c
(__pthread_mutex_timedlock_internal): Fix timeout handling.
* sysdeps/generic/pt-rwlock-timedrdlock.c
(__pthread_rwlock_timedrdlock_internal): Likewise.
* sysdeps/generic/pt-rwlock-timedwrlock.c
(__pthread_rwlock_timedwrlock_internal): Likewise.
* sysdeps/generic/pt-rwlock-unlock.c (pthread_rwlock_unlock): Dequeue and
wake up threads with rwlock internal lock held.
* sysdeps/generic/sem-timedwait.c (__sem_timedwait_internal): Fix timeout
handling.
* sysdeps/mach/hurd/pt-docancel.c (__pthread_do_cancel): Unlock the given
thread cancellation lock.
* sysdeps/mach/pt-thread-alloc.c (create_wakeupmsg): Limit the message
queue size of the wakeup port to 1.
* sysdeps/mach/pt-wakeup.c (__pthread_wakeup): Call __mach_msg in a
non-blocking way.
-rw-r--r-- | pthread/pt-alloc.c | 3 | ||||
-rw-r--r-- | pthread/pt-cancel.c | 27 | ||||
-rw-r--r-- | pthread/pt-internal.h | 11 | ||||
-rw-r--r-- | pthread/pt-join.c | 2 | ||||
-rw-r--r-- | pthread/pt-setcancelstate.c | 2 | ||||
-rw-r--r-- | pthread/pt-setcanceltype.c | 2 | ||||
-rw-r--r-- | pthread/pt-testcancel.c | 7 | ||||
-rw-r--r-- | sysdeps/generic/pt-cond-brdcast.c | 8 | ||||
-rw-r--r-- | sysdeps/generic/pt-cond-signal.c | 23 | ||||
-rw-r--r-- | sysdeps/generic/pt-cond-timedwait.c | 147 | ||||
-rw-r--r-- | sysdeps/generic/pt-mutex-timedlock.c | 47 | ||||
-rw-r--r-- | sysdeps/generic/pt-rwlock-timedrdlock.c | 49 | ||||
-rw-r--r-- | sysdeps/generic/pt-rwlock-timedwrlock.c | 50 | ||||
-rw-r--r-- | sysdeps/generic/pt-rwlock-unlock.c | 13 | ||||
-rw-r--r-- | sysdeps/generic/sem-timedwait.c | 51 | ||||
-rw-r--r-- | sysdeps/mach/hurd/pt-docancel.c | 2 | ||||
-rw-r--r-- | sysdeps/mach/pt-thread-alloc.c | 4 | ||||
-rw-r--r-- | sysdeps/mach/pt-wakeup.c | 4 |
18 files changed, 292 insertions, 160 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..aeac009 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; @@ -264,7 +272,8 @@ extern error_t __pthread_timedblock (struct __pthread *__restrict thread, extern void __pthread_wakeup (struct __pthread *thread); -/* Perform a cancelation. */ +/* Perform a cancelation. The CANCEL_LOCK member of the given thread must + be locked before calling this function, which must unlock it. */ extern int __pthread_do_cancel (struct __pthread *thread); 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); } |