Start locking up the active and dead threads lists. The active threads

list is protected by a spinlock_t, but the dead list uses a pthread_mutex
because it is necessary to synchronize other threads with the garbage
collector thread. Lock/Unlock macros are used so it's easier to make
changes to the locks in the future.

The 'dead thread list' lock is intended to replace the gc mutex.
This doesn't have any practical ramifications. It simply makes it
clearer what the purpose of the lock is. The gc will use this lock,
instead of the gc mutex, to synchronize access to the dead list with
other threads.

Modify _pthread_exit() to use these two new locks instead of GIANT_LOCK,
and also to properly lock and protect thread state changes,
especially with respect to a joining thread.

The gc thread was also re-arranged to be more organized and less nested.

_pthread_join() was also modified to use the thread list locks. However,
locking and unlocking here needs special care because a thread could find
itself in a position where it's joining an exiting thread that is
waiting on the dead list lock, which this thread (joiner) holds. If the
joiner doesn't take care to lock *and* unlock in the same order they
(the joiner and the joinee) could deadlock against each other.

Approved by:	re/blanket libthr

1 files changed, 26 insertions, 30 deletions

diff --git a/lib/libthr/thread/thr_exit.c b/lib/libthr/thread/thr_exit.c
index 118876f..8c4a060 100644
--- a/lib/libthr/thread/thr_exit.c
+++ b/lib/libthr/thread/thr_exit.c
@@ -95,6 +95,7 @@ void
 _pthread_exit(void *status)
 {
 	pthread_t pthread;
+	int exitNow = 0;
 
 	/* Check if this thread is already in the process of exiting: */
 	if ((curthread->flags & PTHREAD_EXITING) != 0) {
@@ -121,37 +122,14 @@ _pthread_exit(void *status)
 		_thread_cleanupspecific();
 	}
 
-	/*
-	 * 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");
-
-	/* Add this thread to the list of dead threads. */
-	TAILQ_INSERT_HEAD(&_dead_list, curthread, dle);
-
-	/*
-	 * Signal the garbage collector thread that there is something
-	 * to clean up.
-	 */
-	if (pthread_cond_signal(&_gc_cond) != 0)
-		PANIC("Cannot signal gc cond");
-
-	/*
-	 * Avoid a race condition where a scheduling signal can occur
-	 * causing the garbage collector thread to run.  If this happens,
-	 * the current thread can be cleaned out from under us.
-	 */
-	GIANT_LOCK(curthread);
-
-	/* Unlock the garbage collector mutex. */
-	if (pthread_mutex_unlock(&_gc_mutex) != 0)
-		PANIC("Cannot unlock gc mutex");
+	/* Lock the dead list first to maintain correct lock order */
+	DEAD_LIST_LOCK;
+	_thread_critical_enter(curthread);
 
 	/* Check if there is a thread joining this one: */
 	if (curthread->joiner != NULL) {
 		pthread = curthread->joiner;
+		_SPINLOCK(&pthread->lock);
 		curthread->joiner = NULL;
 
 		/* Make the joining thread runnable: */
@@ -161,6 +139,7 @@ _pthread_exit(void *status)
 		pthread->join_status.ret = curthread->ret;
 		pthread->join_status.error = 0;
 		pthread->join_status.thread = NULL;
+		_SPINUNLOCK(&pthread->lock);
 
 		/* Make this thread collectable by the garbage collector. */
 		PTHREAD_ASSERT(((curthread->attr.flags & PTHREAD_DETACHED) ==
@@ -168,14 +147,31 @@ _pthread_exit(void *status)
 		curthread->attr.flags |= PTHREAD_DETACHED;
 	}
 
-	/* Remove this thread from the thread list: */
+	/*
+	 * Add this thread to the list of dead threads, and
+	 * also remove it from the active threads list.
+	 */
+	THREAD_LIST_LOCK;
+	TAILQ_INSERT_HEAD(&_dead_list, curthread, dle);
 	TAILQ_REMOVE(&_thread_list, curthread, tle);
-
 	PTHREAD_SET_STATE(curthread, PS_DEAD);
-	GIANT_UNLOCK(curthread);
+	_thread_critical_exit(curthread);
+	
+	/*
+	 * Signal the garbage collector thread that there is something
+	 * to clean up.
+	 */
+	if (pthread_cond_signal(&_gc_cond) != 0)
+		PANIC("Cannot signal gc cond");
 
 	/* If we're the last thread, call it quits */
 	if (TAILQ_EMPTY(&_thread_list))
+		exitNow = 1;
+
+	THREAD_LIST_UNLOCK;
+	DEAD_LIST_UNLOCK;
+
+	if (exitNow)
 		exit(0);
 
 	/*