_pthread_cancel() breaks the normal lock order of first locking the

joined and then the joiner thread. There isn't an easy (sane?) way
to make it use the correct order without introducing races involving
the target thread and finding which (active or dead) list it is on. So,
after locking the canceled thread it will try to lock the joined thread
and if it fails release the first lock and try again from the top.

Introduce a new function, _spintrylock, which is simply a wrapper arround
umtx_trylock(), to help accomplish this.

Approved by: re/blanket libthr

3 files changed, 28 insertions, 4 deletions

diff --git a/lib/libthr/thread/thr_cancel.c b/lib/libthr/thread/thr_cancel.c
index ec2bfb6..7e1bace 100644
--- a/lib/libthr/thread/thr_cancel.c
+++ b/lib/libthr/thread/thr_cancel.c
@@ -21,7 +21,16 @@ int
 _pthread_cancel(pthread_t pthread)
 {
 	int ret;
+	pthread_t joined;
 
+	/*
+	 * 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.
+	 */
+retry:
 	if ((ret = _find_thread(pthread)) != 0)
 		/* The thread is not on the list of active threads */
 		goto out;
@@ -70,10 +79,14 @@ _pthread_cancel(pthread_t pthread)
 			/*
 			 * Disconnect the thread from the joinee:
 			 */
-			if (pthread->join_status.thread != NULL) {
-				pthread->join_status.thread->joiner
-				    = NULL;
-				pthread->join_status.thread = NULL;
+			if ((joined = pthread->join_status.thread) != NULL) {
+				if (_spintrylock(&joined->lock) == EBUSY) {
+					_thread_critical_exit(pthread);
+					goto retry;
+				}
+				pthread->join_status.thread->joiner = NULL;
+				_spinunlock(&joined->lock);
+				joined = pthread->join_status.thread = NULL;
 			}
 			pthread->cancelflags |= PTHREAD_CANCELLING;
 			PTHREAD_NEW_STATE(pthread, PS_RUNNING);
diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h
index 3668a0e..db2bab2 100644
--- a/lib/libthr/thread/thr_private.h
+++ b/lib/libthr/thread/thr_private.h
@@ -737,6 +737,7 @@ int	_pthread_mutexattr_settype(pthread_mutexattr_t *, int);
 int	_pthread_once(pthread_once_t *, void (*) (void));
 pthread_t _pthread_self(void);
 int	_pthread_setspecific(pthread_key_t, const void *);
+int	_spintrylock(spinlock_t *);
 inline void _spinlock_pthread(pthread_t, spinlock_t *);
 inline void _spinunlock_pthread(pthread_t, spinlock_t *);
 void    _thread_exit(char *, int, char *);
diff --git a/lib/libthr/thread/thr_spinlock.c b/lib/libthr/thread/thr_spinlock.c
index ff9b9e0..0f9cb6b 100644
--- a/lib/libthr/thread/thr_spinlock.c
+++ b/lib/libthr/thread/thr_spinlock.c
@@ -69,6 +69,16 @@ _spinlock(spinlock_t *lck)
 	_spinlock_pthread(curthread, lck);
 }
 
+int
+_spintrylock(spinlock_t *lck)
+{
+	int error;
+	error = umtx_trylock((struct umtx *)lck, curthread->thr_id);
+	if (error != 0 && error != EBUSY)
+		abort();
+	return (error);
+}
+
 inline void
 _spinlock_pthread(pthread_t pthread, spinlock_t *lck)
 {