From 22f14eaadfe2f54e3d94f9a549dbb55a301222e1 Mon Sep 17 00:00:00 2001 From: jasone Date: Wed, 27 Jun 2001 11:41:15 +0000 Subject: Fix a race condition in pthread_join(). All of the following must occur atomically: 1) Search _thread_list for the thread to join. 2) Search _dead_list for the thread to join. 3) Set the running thread as the joiner. While we're at it, fix a race in the case where multiple threads try to join on the same thread. POSIX says that the behavior of multiple joiners is undefined, but the fix is cheap as a result of the other fix. --- lib/libkse/thread/thr_find_thread.c | 32 ----------------- lib/libkse/thread/thr_join.c | 68 +++++++++++++++++++++++++++++-------- lib/libkse/thread/thr_private.h | 1 - 3 files changed, 53 insertions(+), 48 deletions(-) (limited to 'lib/libkse') diff --git a/lib/libkse/thread/thr_find_thread.c b/lib/libkse/thread/thr_find_thread.c index 861c4cb..e6e9294 100644 --- a/lib/libkse/thread/thr_find_thread.c +++ b/lib/libkse/thread/thr_find_thread.c @@ -64,35 +64,3 @@ _find_thread(pthread_t pthread) /* Return zero if the thread exists: */ return ((pthread1 != NULL) ? 0:ESRCH); } - -/* Find a thread in the linked list of dead threads: */ -int -_find_dead_thread(pthread_t pthread) -{ - pthread_t pthread1; - - /* Check if the caller has specified an invalid thread: */ - if (pthread == NULL || pthread->magic != PTHREAD_MAGIC) - /* Invalid thread: */ - return(EINVAL); - - /* - * Lock the garbage collector mutex to ensure that the garbage - * collector is not using the dead thread list. - */ - if (pthread_mutex_lock(&_gc_mutex) != 0) - PANIC("Cannot lock gc mutex"); - - /* Search for the specified thread: */ - TAILQ_FOREACH(pthread1, &_dead_list, dle) { - if (pthread1 == pthread) - break; - } - - /* Unlock the garbage collector mutex: */ - if (pthread_mutex_unlock(&_gc_mutex) != 0) - PANIC("Cannot lock gc mutex"); - - /* Return zero if the thread exists: */ - return ((pthread1 != NULL) ? 0:ESRCH); -} diff --git a/lib/libkse/thread/thr_join.c b/lib/libkse/thread/thr_join.c index e1a1f3d..968c24c 100644 --- a/lib/libkse/thread/thr_join.c +++ b/lib/libkse/thread/thr_join.c @@ -42,6 +42,7 @@ _pthread_join(pthread_t pthread, void **thread_return) { struct pthread *curthread = _get_curthread(); int ret = 0; + pthread_t thread; _thread_enter_cancellation_point(); @@ -60,24 +61,63 @@ _pthread_join(pthread_t pthread, void **thread_return) } /* - * Find the thread in the list of active threads or in the - * list of dead threads: + * Lock the garbage collector mutex to ensure that the garbage + * collector is not using the dead thread list. */ - if ((_find_thread(pthread) != 0) && (_find_dead_thread(pthread) != 0)) - /* Return an error: */ - ret = ESRCH; + if (pthread_mutex_lock(&_gc_mutex) != 0) + PANIC("Cannot lock gc mutex"); + + /* + * Defer signals to protect the thread list from access + * by the signal handler: + */ + _thread_kern_sig_defer(); + + /* + * Unlock the garbage collector mutex, now that the garbage collector + * can't be run: + */ + if (pthread_mutex_unlock(&_gc_mutex) != 0) + PANIC("Cannot lock gc mutex"); + + /* + * Search for the specified thread in the list of active threads. This + * is done manually here rather than calling _find_thread() because + * the searches in _thread_list and _dead_list (as well as setting up + * join/detach state) have to be done atomically. + */ + TAILQ_FOREACH(thread, &_thread_list, tle) { + if (thread == pthread) + break; + } + if (thread == NULL) { + /* + * Search for the specified thread in the list of dead threads: + */ + TAILQ_FOREACH(thread, &_dead_list, dle) { + if (thread == pthread) + break; + } + } + + /* Check if the thread was not found or has been detached: */ + if (thread == NULL || + ((pthread->attr.flags & PTHREAD_DETACHED) != 0)) { + /* Undefer and handle pending signals, yielding if necessary: */ + _thread_kern_sig_undefer(); - /* Check if this thread has been detached: */ - else if ((pthread->attr.flags & PTHREAD_DETACHED) != 0) /* Return an error: */ ret = ESRCH; - else if (pthread->joiner != NULL) + } else if (pthread->joiner != NULL) { + /* Undefer and handle pending signals, yielding if necessary: */ + _thread_kern_sig_undefer(); + /* Multiple joiners are not supported. */ ret = ENOTSUP; /* Check if the thread is not dead: */ - else if (pthread->state != PS_DEAD) { + } else if (pthread->state != PS_DEAD) { /* Set the running thread to be the joiner: */ pthread->joiner = curthread; @@ -103,13 +143,11 @@ _pthread_join(pthread_t pthread, void **thread_return) *thread_return = pthread->ret; } - /* - * Make the thread collectable by the garbage collector. There - * is a race here with the garbage collector if multiple threads - * try to join the thread, but the behavior of multiple joiners - * is undefined, so don't bother protecting against the race. - */ + /* Make the thread collectable by the garbage collector. */ pthread->attr.flags |= PTHREAD_DETACHED; + + /* Undefer and handle pending signals, yielding if necessary: */ + _thread_kern_sig_undefer(); } _thread_leave_cancellation_point(); diff --git a/lib/libkse/thread/thr_private.h b/lib/libkse/thread/thr_private.h index 3bfb7c0..6528c13 100644 --- a/lib/libkse/thread/thr_private.h +++ b/lib/libkse/thread/thr_private.h @@ -1207,7 +1207,6 @@ char *__ttyname_r_basic(int, char *, size_t); char *ttyname_r(int, char *, size_t); void _cond_wait_backout(pthread_t); void _fd_lock_backout(pthread_t); -int _find_dead_thread(pthread_t); int _find_thread(pthread_t); struct pthread *_get_curthread(void); void _set_curthread(struct pthread *); -- cgit v1.1