Make libthr async-signal-safe without costly signal masking. The guidlines I

followed are: Only 3 functions (pthread_cancel, pthread_setcancelstate,
pthread_setcanceltype) are required to be async-signal-safe by POSIX. None of
the rest of the pthread api is required to be async-signal-safe. This means
that only the three mentioned functions are safe to use from inside
signal handlers.
However, there are certain system/libc calls that are
cancellation points that a caller may call from within a signal handler,
and since they are cancellation points calls have to be made into libthr
to test for cancellation and exit the thread if necessary. So, the
cancellation test and thread exit code paths must be async-signal-safe
as well. A summary of the changes follows:

o Almost all of the code paths that masked signals, as well as locking the
  pthread structure now lock only the pthread structure.
o Signals are masked (and left that way) as soon as a thread enters
  pthread_exit().
o The active and dead threads locks now explicitly require that signals
  are masked.
o Access to the isdead field of the pthread structure is protected by both
  the active and dead list locks for writing. Either one is sufficient for
  reading.
o The thread state and type fields have been combined into one three-state
  switch to make it easier to read without requiring a lock. It doesn't need
  a lock for writing (and therefore for reading either) because only the
  current thread can write to it and it is an integer value.
o The thread state field of the pthread structure has been eliminated. It
  was an unnecessary field that mostly duplicated the flags field, but
  required additional locking that would make a lot more code paths require
  signal masking. Any truly unique values (such as PS_DEAD) have been
  reborn as separate members of the pthread structure.
o Since the mutex and condvar pthread functions are not async-signal-safe
  there is no need to muck about with the wait queues when handling
  a signal ...
o ... which also removes the need for wrapping signal handlers and sigaction(2).
o The condvar and mutex async-cancellation code had to be revised as a result
  of some of these changes, which resulted in semi-unrelated changes which
  would have been difficult to work on as a separate commit, so they are
  included as well.

The only part of the changes I am worried about is related to locking for
the pthread joining fields. But, I will take a closer look at them once this
mega-patch is committed.

1 files changed, 65 insertions, 157 deletions

diff --git a/lib/libthr/thread/thr_cancel.c b/lib/libthr/thread/thr_cancel.c
index a539de7..5880e1f 100644
--- a/lib/libthr/thread/thr_cancel.c
+++ b/lib/libthr/thread/thr_cancel.c
@@ -17,219 +17,127 @@ __weak_reference(_pthread_setcancelstate, pthread_setcancelstate);
 __weak_reference(_pthread_setcanceltype, pthread_setcanceltype);
 __weak_reference(_pthread_testcancel, pthread_testcancel);
 
+/*
+ * Posix requires this function to be async-cancel-safe, so it
+ * may not aquire any type of resource or call any functions
+ * that might do so.
+ */
 int
 _pthread_cancel(pthread_t pthread)
 {
-	int ret;
-	pthread_t joined;
+	/* Don't continue if cancellation has already been set. */
+	if (atomic_cmpset_int(&pthread->cancellation, (int)CS_NULL,
+	    (int)CS_PENDING) != 1)
+		return (0);
 
 	/*
-	 * When canceling a thread that has joined another thread, this
-	 * routine breaks the normal lock order of locking first the
-	 * joined and then the joiner. Therefore, it is necessary that
-	 * if it can't obtain the second lock, that it release the first
-	 * one and restart from the top.
+	 * Only wakeup threads that are in cancellation points or
+	 * have set async cancel.
+	 * XXX - access to pthread->flags is not safe. We should just
+	 *	 unconditionally wake the thread and make sure that
+	 *	 the the library correctly handles spurious wakeups.
 	 */
-retry:
-	if ((ret = _find_thread(pthread)) != 0)
-		/* The thread is not on the list of active threads */
-		goto out;
-
-	_thread_critical_enter(pthread);
-
-	if (pthread->state == PS_DEAD || pthread->state == PS_DEADLOCK
-	    || (pthread->flags & PTHREAD_EXITING) != 0) {
-		/*
-		 * The thread is in the process of (or has already) exited
-		 * or is deadlocked.
-		 */
-		_thread_critical_exit(pthread);
-		ret = 0;
-		goto out;
-	}
-
-	/*
-	 * The thread is on the active thread list and is not in the process
-	 * of exiting.
-	 */
-
-	if (((pthread->cancelflags & PTHREAD_CANCEL_DISABLE) != 0) ||
-	    (((pthread->cancelflags & PTHREAD_CANCEL_ASYNCHRONOUS) == 0) &&
-	    ((pthread->cancelflags & PTHREAD_AT_CANCEL_POINT) == 0)))
-		/* Just mark it for cancellation: */
-		pthread->cancelflags |= PTHREAD_CANCELLING;
-	else {
-		/*
-		 * Check if we need to kick it back into the
-		 * run queue:
-		 */
-		switch (pthread->state) {
-		case PS_RUNNING:
-			/* No need to resume: */
-			pthread->cancelflags |= PTHREAD_CANCELLING;
-			break;
-
-		case PS_SLEEP_WAIT:
-		case PS_WAIT_WAIT:
-			pthread->cancelflags |= PTHREAD_CANCELLING;
-			PTHREAD_NEW_STATE(pthread, PS_RUNNING);
-			break;
-
-		case PS_JOIN:
-			/*
-			 * Disconnect the thread from the joinee:
-			 */
-			if ((joined = pthread->join_status.thread) != NULL) {
-				UMTX_TRYLOCK(&joined->lock, ret);
-				if (ret == EBUSY) {
-					_thread_critical_exit(pthread);
-					goto retry;
-				}
-				pthread->join_status.thread->joiner = NULL;
-				UMTX_UNLOCK(&joined->lock);
-				joined = pthread->join_status.thread = NULL;
-			}
-			pthread->cancelflags |= PTHREAD_CANCELLING;
-			PTHREAD_NEW_STATE(pthread, PS_RUNNING);
-			break;
-
-		case PS_BARRIER_WAIT:
-		case PS_MUTEX_WAIT:
-		case PS_COND_WAIT:
-			/*
-			 * Threads in these states may be in queues.
-			 * In order to preserve queue integrity, the
-			 * cancelled thread must remove itself from the
-			 * queue.  When the thread resumes, it will
-			 * remove itself from the queue and call the
-			 * cancellation routine.
-			 */
-			pthread->cancelflags |= PTHREAD_CANCELLING;
-			PTHREAD_NEW_STATE(pthread, PS_RUNNING);
-			break;
-
-		case PS_DEAD:
-		case PS_DEADLOCK:
-		case PS_STATE_MAX:
-			/* Ignore - only here to silence -Wall: */
-			break;
-		}
-	}
-
-	/* Unprotect the scheduling queues: */
-	_thread_critical_exit(pthread);
-
-	ret = 0;
-out:
-	return (ret);
+	if ((pthread->cancellationpoint || pthread->cancelmode == M_ASYNC) &&
+	    (pthread->flags & PTHREAD_FLAGS_NOT_RUNNING) != 0)
+		PTHREAD_WAKE(pthread);
+	return (0);
 }
 
+/*
+ * Posix requires this function to be async-cancel-safe, so it
+ * may not aquire any type of resource or call any functions
+ * that might do so.
+ */
 int
 _pthread_setcancelstate(int state, int *oldstate)
 {
-	int ostate, ret;
-
-	ret = 0;
-
-	_thread_critical_enter(curthread);
-
-	ostate = curthread->cancelflags & PTHREAD_CANCEL_DISABLE;
+	int ostate;
 
+	ostate = (curthread->cancelmode == M_OFF) ? PTHREAD_CANCEL_DISABLE :
+	    PTHREAD_CANCEL_ENABLE;
 	switch (state) {
 	case PTHREAD_CANCEL_ENABLE:
-		if (oldstate != NULL)
-			*oldstate = ostate;
-		curthread->cancelflags &= ~PTHREAD_CANCEL_DISABLE;
-		if ((curthread->cancelflags & PTHREAD_CANCEL_ASYNCHRONOUS) == 0)
-			break;
-		testcancel();
+		curthread->cancelmode = curthread->cancelstate;
 		break;
 	case PTHREAD_CANCEL_DISABLE:
-		if (oldstate != NULL)
-			*oldstate = ostate;
-		curthread->cancelflags |= PTHREAD_CANCEL_DISABLE;
+		if (curthread->cancelmode != M_OFF) {
+			curthread->cancelstate = curthread->cancelmode;
+			curthread->cancelmode = M_OFF;
+		}
 		break;
 	default:
-		ret = EINVAL;
+		return (EINVAL);
 	}
-
-	_thread_critical_exit(curthread);
-	return (ret);
+	if (oldstate != NULL)
+		*oldstate = ostate;
+	return (0);
 }
 
+/*
+ * Posix requires this function to be async-cancel-safe, so it
+ * may not aquire any type of resource or call any functions that
+ * might do so.
+ */
 int
 _pthread_setcanceltype(int type, int *oldtype)
 {
-	int otype;
+	enum cancel_mode omode;
 
-	_thread_critical_enter(curthread);
-	otype = curthread->cancelflags & PTHREAD_CANCEL_ASYNCHRONOUS;
+	omode = curthread->cancelstate;
 	switch (type) {
 	case PTHREAD_CANCEL_ASYNCHRONOUS:
-		if (oldtype != NULL)
-			*oldtype = otype;
-		curthread->cancelflags |= PTHREAD_CANCEL_ASYNCHRONOUS;
-		testcancel();
+		if (curthread->cancelmode != M_OFF)
+			curthread->cancelmode = M_ASYNC;
+		curthread->cancelstate = M_ASYNC;
 		break;
 	case PTHREAD_CANCEL_DEFERRED:
-		if (oldtype != NULL)
-			*oldtype = otype;
-		curthread->cancelflags &= ~PTHREAD_CANCEL_ASYNCHRONOUS;
+		if (curthread->cancelmode != M_OFF)
+			curthread->cancelmode = M_DEFERRED;
+		curthread->cancelstate = M_DEFERRED;
 		break;
 	default:
 		return (EINVAL);
 	}
-
-	_thread_critical_exit(curthread);
+	if (oldtype != NULL) {
+		if (omode == M_DEFERRED)
+			*oldtype = PTHREAD_CANCEL_DEFERRED;
+		else if (omode == M_ASYNC)
+			*oldtype = PTHREAD_CANCEL_ASYNCHRONOUS;
+	}
 	return (0);
 }
 
 void
 _pthread_testcancel(void)
 {
-	_thread_critical_enter(curthread);
 	testcancel();
-	_thread_critical_exit(curthread);
 }
 
 static void
 testcancel()
 {
-	/*
-	 * This pthread should already be locked by the caller.
-	 */
-
-	if (((curthread->cancelflags & PTHREAD_CANCEL_DISABLE) == 0) &&
-	    ((curthread->cancelflags & PTHREAD_CANCELLING) != 0) &&
-	    ((curthread->flags & PTHREAD_EXITING) == 0)) {
-		/*
-		 * It is possible for this thread to be swapped out
-		 * while performing cancellation; do not allow it
-		 * to be cancelled again.
-		 */
-		curthread->cancelflags &= ~PTHREAD_CANCELLING;
-		_thread_critical_exit(curthread);
-		_thread_exit_cleanup();
-		pthread_exit(PTHREAD_CANCELED);
-		PANIC("cancel");
+	if (curthread->cancelmode != M_OFF) {
+
+		/* Cleanup a canceled thread only once. */
+		if (atomic_cmpset_int(&curthread->cancellation,
+		    (int)CS_PENDING, (int)CS_SET) == 1) {
+			_thread_exit_cleanup();
+			pthread_exit(PTHREAD_CANCELED);
+			PANIC("cancel");
+		}
 	}
 }
 
 void
 _thread_enter_cancellation_point(void)
 {
-	_thread_critical_enter(curthread);
 	testcancel();
-	curthread->cancelflags |= PTHREAD_AT_CANCEL_POINT;
-	_thread_critical_exit(curthread);
+	curthread->cancellationpoint = 1;
 }
 
 void
 _thread_leave_cancellation_point(void)
 {
-	_thread_critical_enter(curthread);
-	curthread->cancelflags &= ~PTHREAD_AT_CANCEL_POINT;
+	curthread->cancellationpoint = 0;
 	testcancel();
-	_thread_critical_exit(curthread);
-
 }