Instead of using a join queue for each thread, use a single pointer to

keep track of a joiner.  POSIX only supports a single joiner, so this
simplification is acceptable.

At the same time, make sure to mark a joined thread as detached so that
its resources can be freed.

Reviewed by:	deischen
PR:		24345

10 files changed, 106 insertions, 145 deletions

diff --git a/lib/libkse/thread/thr_cancel.c b/lib/libkse/thread/thr_cancel.c
index 8b9132c..2af3db6 100644
--- a/lib/libkse/thread/thr_cancel.c
+++ b/lib/libkse/thread/thr_cancel.c
@@ -63,6 +63,7 @@ _pthread_cancel(pthread_t pthread)
 				PTHREAD_NEW_STATE(pthread,PS_RUNNING);
 				break;
 
+			case PS_JOIN:
 			case PS_SUSPENDED:
 				if (pthread->suspended == SUSP_NO ||
 				    pthread->suspended == SUSP_YES ||
@@ -82,7 +83,6 @@ _pthread_cancel(pthread_t pthread)
 			case PS_FDLR_WAIT:
 			case PS_FDLW_WAIT:
 			case PS_FILE_WAIT:
-			case PS_JOIN:
 				/*
 				 * Threads in these states may be in queues.
 				 * In order to preserve queue integrity, the
@@ -189,6 +189,7 @@ _pthread_testcancel(void)
 		 */
 		curthread->cancelflags &= ~PTHREAD_CANCELLING;
 		_thread_exit_cleanup();
+		pthread_detach((pthread_t)curthread);
 		pthread_exit(PTHREAD_CANCELED);
 		PANIC("cancel");
 	}
@@ -225,6 +226,7 @@ finish_cancellation(void *arg)
 	if ((curthread->cancelflags & PTHREAD_CANCEL_NEEDED) != 0) {
 		curthread->cancelflags &= ~PTHREAD_CANCEL_NEEDED;
 		_thread_exit_cleanup();
+		pthread_detach((pthread_t)curthread);
 		pthread_exit(PTHREAD_CANCELED);
 	}
 }
diff --git a/lib/libkse/thread/thr_create.c b/lib/libkse/thread/thr_create.c
index f1a3473..40f5364 100644
--- a/lib/libkse/thread/thr_create.c
+++ b/lib/libkse/thread/thr_create.c
@@ -235,8 +235,8 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr,
 			new_thread->active_priority = new_thread->base_priority;
 			new_thread->inherited_priority = 0;
 
-			/* Initialise the join queue for the new thread: */
-			TAILQ_INIT(&(new_thread->join_queue));
+			/* Initialize joiner to NULL (no joiner): */
+			new_thread->joiner = NULL;
 
 			/* Initialize the mutex queue: */
 			TAILQ_INIT(&new_thread->mutexq);
diff --git a/lib/libkse/thread/thr_detach.c b/lib/libkse/thread/thr_detach.c
index 0b3cfd7..b0e6fc4 100644
--- a/lib/libkse/thread/thr_detach.c
+++ b/lib/libkse/thread/thr_detach.c
@@ -41,7 +41,6 @@ int
 _pthread_detach(pthread_t pthread)
 {
 	int             rval = 0;
-	pthread_t       next_thread;
 
 	/* Check for invalid calling parameters: */
 	if (pthread == NULL || pthread->magic != PTHREAD_MAGIC)
@@ -59,19 +58,20 @@ _pthread_detach(pthread_t pthread)
 		 */
 		_thread_kern_sig_defer();
 
-		/* Enter a loop to bring all threads off the join queue: */
-		while ((next_thread = TAILQ_FIRST(&pthread->join_queue)) != NULL) {
-			/* Remove the thread from the queue: */
-			TAILQ_REMOVE(&pthread->join_queue, next_thread, sqe);
-			pthread->flags &= ~PTHREAD_FLAGS_IN_JOINQ;
+		/* Check if there is a joiner: */
+		if (pthread->joiner != NULL) {
+			struct pthread	*joiner = pthread->joiner;
 
 			/* Make the thread runnable: */
-			PTHREAD_NEW_STATE(next_thread, PS_RUNNING);
+			PTHREAD_NEW_STATE(joiner, PS_RUNNING);
+
+			/* Set the return value for the woken thread: */
+			joiner->error = ESRCH;
 
 			/*
-			 * Set the return value for the woken thread:
+			 * Disconnect the joiner from the thread being detached:
 			 */
-			next_thread->error = ESRCH;
+			pthread->joiner = NULL;
 		}
 
 		/*
diff --git a/lib/libkse/thread/thr_exit.c b/lib/libkse/thread/thr_exit.c
index a0ebc8c..9376350 100644
--- a/lib/libkse/thread/thr_exit.c
+++ b/lib/libkse/thread/thr_exit.c
@@ -195,29 +195,38 @@ _pthread_exit(void *status)
 
 	/* Unlock the garbage collector mutex: */
 	if (pthread_mutex_unlock(&_gc_mutex) != 0)
-		PANIC("Cannot lock gc mutex");
-
-	/* Check if there are any threads joined to this one: */
-	while ((pthread = TAILQ_FIRST(&(curthread->join_queue))) != NULL) {
-		/* Remove the thread from the queue: */
-		TAILQ_REMOVE(&curthread->join_queue, pthread, sqe);
-		pthread->flags &= ~PTHREAD_FLAGS_IN_JOINQ;
-
-		/*
-		 * Wake the joined thread and let it
-		 * detach this thread:
-		 */
-		PTHREAD_NEW_STATE(pthread, PS_RUNNING);
-
-		/*
-		 * Set the return value for the woken thread:
-		 */
-		if ((curthread->attr.flags & PTHREAD_DETACHED) != 0)
-			pthread->error = ESRCH;
-		else {
-			pthread->ret = curthread->ret;
-			pthread->error = 0;
+		PANIC("Cannot unlock gc mutex");
+
+	/* Check if there is a thread joining this one: */
+	if (curthread->joiner != NULL) {
+		pthread = curthread->joiner;
+		curthread->joiner = NULL;
+
+		switch (pthread->suspended) {
+		case SUSP_JOIN:
+			/*
+			 * The joining thread is suspended.  Change the
+			 * suspension state to make the thread runnable when it
+			 * is resumed:
+			 */
+			pthread->suspended = SUSP_NO;
+			break;
+		case SUSP_NO:
+			/* Make the joining thread runnable: */
+			PTHREAD_NEW_STATE(pthread, PS_RUNNING);
+			break;
+		default:
+			PANIC("Unreachable code reached");
 		}
+
+		/* Set the return value for the joining thread: */
+		pthread->ret = curthread->ret;
+		pthread->error = 0;
+
+		/* Make this thread collectable by the garbage collector. */
+		PTHREAD_ASSERT(((curthread->attr.flags & PTHREAD_DETACHED) ==
+		    0), "Cannot join a detached thread");
+		curthread->attr.flags |= PTHREAD_DETACHED;
 	}
 
 	/* Remove this thread from the thread list: */
diff --git a/lib/libkse/thread/thr_init.c b/lib/libkse/thread/thr_init.c
index 033b78e..b63a111 100644
--- a/lib/libkse/thread/thr_init.c
+++ b/lib/libkse/thread/thr_init.c
@@ -318,8 +318,8 @@ _thread_init(void)
 		/* Set the name of the thread: */
 		_thread_initial->name = strdup("_thread_initial");
 
-		/* Initialise the queue: */
-		TAILQ_INIT(&(_thread_initial->join_queue));
+		/* Initialize joiner to NULL (no joiner): */
+		_thread_initial->joiner = NULL;
 
 		/* Initialize the owned mutex queue and count: */
 		TAILQ_INIT(&(_thread_initial->mutexq));
diff --git a/lib/libkse/thread/thr_join.c b/lib/libkse/thread/thr_join.c
index b9c75ac..e1a1f3d 100644
--- a/lib/libkse/thread/thr_join.c
+++ b/lib/libkse/thread/thr_join.c
@@ -72,98 +72,48 @@ _pthread_join(pthread_t pthread, void **thread_return)
 		/* Return an error: */
 		ret = ESRCH;
 
+	else if (pthread->joiner != NULL)
+		/* Multiple joiners are not supported. */
+		ret = ENOTSUP;
+
 	/* Check if the thread is not dead: */
 	else if (pthread->state != PS_DEAD) {
-		PTHREAD_ASSERT_NOT_IN_SYNCQ(curthread);
+		/* Set the running thread to be the joiner: */
+		pthread->joiner = curthread;
+
+		/* Schedule the next thread: */
+		_thread_kern_sched_state(PS_JOIN, __FILE__, __LINE__);
 
 		/*
-		 * Enter a loop in case this thread is woken prematurely
-		 * in order to invoke a signal handler:
+		 * The thread return value and error are set by the thread we're
+		 * joining to when it exits or detaches:
+		 */
+		ret = curthread->error;
+		if ((ret == 0) && (thread_return != NULL))
+			*thread_return = curthread->ret;
+	} else {
+		/*
+		 * The thread exited (is dead) without being detached, and no
+		 * thread has joined it.
 		 */
-		for (;;) {
-			/* Clear the interrupted flag: */
-			curthread->interrupted = 0;
-
-			/*
-			 * Protect against being context switched out while
-			 * adding this thread to the join queue.
-			 */
-			_thread_kern_sig_defer();
-
-			/* Add the running thread to the join queue: */
-			TAILQ_INSERT_TAIL(&(pthread->join_queue),
-			    curthread, sqe);
-			curthread->flags |= PTHREAD_FLAGS_IN_JOINQ;
-			curthread->data.thread = pthread;
-
-			/* Schedule the next thread: */
-			_thread_kern_sched_state(PS_JOIN, __FILE__, __LINE__);
-
-			if ((curthread->flags & PTHREAD_FLAGS_IN_JOINQ) != 0) {
-				TAILQ_REMOVE(&(pthread->join_queue),
-				    curthread, sqe);
-				curthread->flags &= ~PTHREAD_FLAGS_IN_JOINQ;
-			}
-			curthread->data.thread = NULL;
-
-			_thread_kern_sig_undefer();
-
-			if (curthread->interrupted != 0) {
-				if (curthread->continuation != NULL)
-					curthread->continuation(curthread);
-				/*
-				 * This thread was interrupted, probably to
-				 * invoke a signal handler.  Make sure the
-				 * target thread is still joinable.
-				 */
-				if (((_find_thread(pthread) != 0) &&
-				    (_find_dead_thread(pthread) != 0)) ||
-				    ((pthread->attr.flags &
-				    PTHREAD_DETACHED) != 0)) {
-					/* Return an error: */
-					ret = ESRCH;
-
-					/* We're done; break out of the loop. */
-					break;
-				}
-				else if (pthread->state == PS_DEAD) {
-					/* We're done; break out of the loop. */
-					break;
-				}
-			} else {
-				/*
-				 * The thread return value and error are set
-				 * by the thread we're joining to when it
-				 * exits or detaches:
-				 */
-				ret = curthread->error;
-				if ((ret == 0) && (thread_return != NULL))
-					*thread_return = curthread->ret;
 
-				/* We're done; break out of the loop. */
-				break;
-			}
+		/* Check if the return value is required: */
+		if (thread_return != NULL) {
+			/* Return the thread's return value: */
+			*thread_return = pthread->ret;
 		}
-	/* Check if the return value is required: */
-	} else if (thread_return != NULL)
-		/* Return the thread's return value: */
-		*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.
+		 */
+		pthread->attr.flags |= PTHREAD_DETACHED;
+	}
 
 	_thread_leave_cancellation_point();
 
 	/* Return the completion status: */
 	return (ret);
 }
-
-void
-_join_backout(pthread_t pthread)
-{
-	struct pthread	*curthread = _get_curthread();
-
-	_thread_kern_sig_defer();
-	if ((pthread->flags & PTHREAD_FLAGS_IN_JOINQ) != 0) {
-		TAILQ_REMOVE(&pthread->data.thread->join_queue, pthread, sqe);
-		curthread->flags &= ~PTHREAD_FLAGS_IN_JOINQ;
-	}
-	_thread_kern_sig_undefer();
-}
diff --git a/lib/libkse/thread/thr_private.h b/lib/libkse/thread/thr_private.h
index e212a0b..3bfb7c0 100644
--- a/lib/libkse/thread/thr_private.h
+++ b/lib/libkse/thread/thr_private.h
@@ -403,6 +403,7 @@ struct pthread_attr {
 enum pthread_susp {
 	SUSP_NO,	/* Not suspended. */
 	SUSP_YES,	/* Suspended. */
+	SUSP_JOIN,	/* Suspended, joining. */
 	SUSP_NOWAIT,	/* Suspended, was in a mutex or condition queue. */
 	SUSP_MUTEX_WAIT,/* Suspended, still in a mutex queue. */
 	SUSP_COND_WAIT	/* Suspended, still in a condition queue. */
@@ -573,7 +574,6 @@ union pthread_wait_data {
 	FILE		*fp;
 	struct pthread_poll_data *poll_data;
 	spinlock_t	*spinlock;
-	struct pthread	*thread;
 };
 
 /*
@@ -755,8 +755,8 @@ struct pthread {
 	 */
 	int	error;
 
-	/* Join queue head and link for waiting threads: */
-	TAILQ_HEAD(join_head, pthread)	join_queue;
+	/* Pointer to a thread that is waiting to join (NULL if no joiner). */
+	struct pthread *joiner;
 
 	/*
 	 * The current thread can belong to only one scheduling queue at
@@ -764,23 +764,24 @@ struct pthread {
 	 *
 	 *   o A queue of threads waiting for a mutex
 	 *   o A queue of threads waiting for a condition variable
-	 *   o A queue of threads waiting for another thread to terminate
-	 *     (the join queue above)
 	 *   o A queue of threads waiting for a file descriptor lock
 	 *   o A queue of threads needing work done by the kernel thread
 	 *     (waiting for a spinlock or file I/O)
 	 *
-	 * It is possible for a thread to belong to more than one of the
-	 * above queues if it is handling a signal.  A thread may only
-	 * enter a mutex, condition variable, or join queue when it is
-	 * not being called from a signal handler.  If a thread is a
+	 * A thread can also be joining a thread (the joiner field above).
+	 *
+	 * It must not be possible for a thread to belong to any of the
+	 * above queues while it is handling a signal.  Signal handlers
+	 * may longjmp back to previous stack frames circumventing normal
+	 * control flow.  This could corrupt queue integrity if the thread
+	 * retains membership in the queue.  Therefore, if a thread is a
 	 * member of one of these queues when a signal handler is invoked,
-	 * it must remain in the queue.  For this reason, the links for
-	 * these queues must not be (re)used for other queues.
+	 * it must remove itself from the queue before calling the signal
+	 * handler and reinsert itself after normal return of the handler.
 	 *
 	 * Use pqe for the scheduling queue link (both ready and waiting),
-	 * sqe for synchronization (mutex, condition variable, and join)
-	 * queue links, and qe for all other links.
+	 * sqe for synchronization (mutex and condition variable) queue
+	 * links, and qe for all other links.
 	 */
 	TAILQ_ENTRY(pthread)	pqe;	/* priority queue link */
 	TAILQ_ENTRY(pthread)	sqe;	/* synchronization queue link */
@@ -826,10 +827,9 @@ struct pthread {
 #define PTHREAD_FLAGS_IN_FDQ	0x0040	/* in fd lock queue using qe link */
 #define PTHREAD_FLAGS_IN_CONDQ	0x0080	/* in condition queue using sqe link*/
 #define PTHREAD_FLAGS_IN_MUTEXQ	0x0100	/* in mutex queue using sqe link */
-#define PTHREAD_FLAGS_IN_JOINQ	0x0200	/* in join queue using sqe link */
-#define PTHREAD_FLAGS_TRACE	0x0400	/* for debugging purposes */
+#define PTHREAD_FLAGS_TRACE	0x0200	/* for debugging purposes */
 #define PTHREAD_FLAGS_IN_SYNCQ	\
-    (PTHREAD_FLAGS_IN_CONDQ | PTHREAD_FLAGS_IN_MUTEXQ | PTHREAD_FLAGS_IN_JOINQ)
+    (PTHREAD_FLAGS_IN_CONDQ | PTHREAD_FLAGS_IN_MUTEXQ)
 
 	/*
 	 * Base priority is the user setable and retrievable priority
@@ -1211,7 +1211,6 @@ int     _find_dead_thread(pthread_t);
 int     _find_thread(pthread_t);
 struct pthread *_get_curthread(void);
 void	_set_curthread(struct pthread *);
-void	_join_backout(pthread_t);
 int     _thread_create(pthread_t *,const pthread_attr_t *,void *(*start_routine)(void *),void *,pthread_t);
 int     _thread_fd_lock(int, int, struct timespec *);
 int     _thread_fd_lock_debug(int, int, struct timespec *,char *fname,int lineno);
diff --git a/lib/libkse/thread/thr_resume_np.c b/lib/libkse/thread/thr_resume_np.c
index 5ed408b..9cbcf85 100644
--- a/lib/libkse/thread/thr_resume_np.c
+++ b/lib/libkse/thread/thr_resume_np.c
@@ -67,6 +67,10 @@ _pthread_resume_np(pthread_t thread)
 				/* Set the thread's state back. */
 				PTHREAD_SET_STATE(thread,PS_COND_WAIT);
 				break;
+			case SUSP_JOIN:
+				/* Set the thread's state back. */
+				PTHREAD_SET_STATE(thread,PS_JOIN);
+				break;
 			case SUSP_NOWAIT:
 				/* Allow the thread to run. */
 				PTHREAD_SET_STATE(thread,PS_RUNNING);
diff --git a/lib/libkse/thread/thr_sig.c b/lib/libkse/thread/thr_sig.c
index 9ce53a0..fd5180c 100644
--- a/lib/libkse/thread/thr_sig.c
+++ b/lib/libkse/thread/thr_sig.c
@@ -670,11 +670,8 @@ thread_sig_add(pthread_t pthread, int sig, int has_args)
 	 * States which cannot be interrupted but still require the
 	 * signal handler to run:
 	 */
-	case PS_JOIN:
-		/* Only set the interrupted flag for PS_JOIN: */
-		pthread->interrupted = 1;
-		/* FALLTHROUGH */
 	case PS_COND_WAIT:
+	case PS_JOIN:
 	case PS_MUTEX_WAIT:
 		/*
 		 * Remove the thread from the wait queue.  It will
@@ -955,11 +952,6 @@ _thread_sig_wrapper(void)
 			psf->saved_state.psd_state = PS_RUNNING;
 			break;
 
-		case PS_JOIN:
-			_join_backout(thread);
-			psf->saved_state.psd_state = PS_RUNNING;
-			break;
-
 		case PS_MUTEX_WAIT:
 			_mutex_lock_backout(thread);
 			psf->saved_state.psd_state = PS_RUNNING;
diff --git a/lib/libkse/thread/thr_suspend_np.c b/lib/libkse/thread/thr_suspend_np.c
index 81ee328..0e272ff 100644
--- a/lib/libkse/thread/thr_suspend_np.c
+++ b/lib/libkse/thread/thr_suspend_np.c
@@ -105,10 +105,15 @@ _pthread_suspend_np(pthread_t thread)
 
 			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
 			break;
+		case PS_JOIN:
+			/* Mark the thread as suspended and joining: */
+			thread->suspended = SUSP_JOIN;
+
+			PTHREAD_NEW_STATE(thread, PS_SUSPENDED);
+			break;
 		case PS_FDLR_WAIT:
 		case PS_FDLW_WAIT:
 		case PS_FILE_WAIT:
-		case PS_JOIN:
 			/* Mark the thread as suspended: */
 			thread->suspended = SUSP_YES;