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.

3 files changed, 53 insertions, 48 deletions

diff --git a/lib/libpthread/thread/thr_find_thread.c b/lib/libpthread/thread/thr_find_thread.c
index 861c4cb..e6e9294 100644
--- a/lib/libpthread/thread/thr_find_thread.c
+++ b/lib/libpthread/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/libpthread/thread/thr_join.c b/lib/libpthread/thread/thr_join.c
index e1a1f3d..968c24c 100644
--- a/lib/libpthread/thread/thr_join.c
+++ b/lib/libpthread/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/libpthread/thread/thr_private.h b/lib/libpthread/thread/thr_private.h
index 3bfb7c0..6528c13 100644
--- a/lib/libpthread/thread/thr_private.h
+++ b/lib/libpthread/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 *);