Add pthread mutex deadlock detection for debug builds (#24607)

This would really have helped in at least detecting the deadlock that
was found in #24570 / #24565

Author: sbc100

system/lib/libc/musl/src/thread/pthread_mutex_lock.c

@@ -2,9 +2,12 @@
 
 int __pthread_mutex_lock(pthread_mutex_t *m)
 {
+#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
+	/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
 	if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
 	    && !a_cas(&m->_m_lock, 0, EBUSY))
 		return 0;
+#endif
 
 	return __pthread_mutex_timedlock(m, 0);
 }

system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c

@@ -1,5 +1,9 @@
 #include "pthread_impl.h"
 
+#ifdef __EMSCRIPTEN__
+#include <assert.h>
+#endif
+
 #ifndef __EMSCRIPTEN__
 #define IS32BIT(x) !((x)+0x80000000ULL>>32)
 #define CLAMP(x) (int)(IS32BIT(x) ? (x) : 0x7fffffffU+((0ULL+(x))>>63))
@@ -57,9 +61,12 @@ static int pthread_mutex_timedlock_pi(pthread_mutex_t *restrict m, const struct
 
 int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *restrict at)
 {
+#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
+	/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
 	if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
 	    && !a_cas(&m->_m_lock, 0, EBUSY))
 		return 0;
+#endif
 
 	int type = m->_m_type;
 	int r, t, priv = (type & 128) ^ 128;
@@ -82,6 +89,11 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
 		if ((type&3) == PTHREAD_MUTEX_ERRORCHECK
 		    && own == __pthread_self()->tid)
 			return EDEADLK;
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+		// Extra check for deadlock in debug builds, but only if we would block
+		// forever (at == NULL).
+		assert(at || own != __pthread_self()->tid && "pthread mutex deadlock detected");
+#endif
 
 		a_inc(&m->_m_waiters);
 		t = r | 0x80000000;

system/lib/libc/musl/src/thread/pthread_mutex_trylock.c

@@ -53,6 +53,12 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 	}
 #endif
 
+#if defined(__EMSCRIPTEN__) || !defined(NDEBUG)
+	// We can get here for normal mutexes too, but only in debug builds
+	// (where we track ownership purely for debug purposes).
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) return 0;
+#endif
+
 	next = self->robust_list.head;
 	m->_m_next = next;
 	m->_m_prev = &self->robust_list.head;
@@ -71,8 +77,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 
 int __pthread_mutex_trylock(pthread_mutex_t *m)
 {
+#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
+	/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
 	if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
 		return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
+#endif
 	return __pthread_mutex_trylock_owner(m);
 }
 

system/lib/pthread/library_pthread.c

@@ -138,13 +138,9 @@ weak_alias(dummy_tsd, __pthread_tsd_main);
 // See system/lib/README.md for static constructor ordering.
 __attribute__((constructor(48)))
 void _emscripten_init_main_thread(void) {
-  _emscripten_init_main_thread_js(&__main_pthread);
-
   // The pthread struct has a field that points to itself - this is used as
   // a magic ID to detect whether the pthread_t structure is 'alive'.
   __main_pthread.self = &__main_pthread;
-  __main_pthread.stack = (void*)emscripten_stack_get_base();
-  __main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();
   __main_pthread.detach_state = DT_JOINABLE;
   // pthread struct robust_list head should point to itself.
   __main_pthread.robust_list.head = &__main_pthread.robust_list.head;
@@ -157,6 +153,11 @@ void _emscripten_init_main_thread(void) {
   __main_pthread.next = __main_pthread.prev = &__main_pthread;
   __main_pthread.tsd = (void **)__pthread_tsd_main;
 
+  _emscripten_init_main_thread_js(&__main_pthread);
+
+  __main_pthread.stack = (void*)emscripten_stack_get_base();
+  __main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();
+
   _emscripten_thread_mailbox_init(&__main_pthread);
   _emscripten_thread_mailbox_await(&__main_pthread);
 }

test/other/codesize/test_codesize_minimal_pthreads.size

@@ -1 +1 @@
-19526
+19540

test/other/codesize/test_codesize_minimal_pthreads_memgrowth.size

@@ -1 +1 @@
-19527
+19541

test/other/test_pthread_mutex_deadlock.c

@@ -0,0 +1,20 @@
+#include <assert.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <stdio.h>
+
+pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
+
+int main() {
+  printf("in main\n");
+  int rtn = pthread_mutex_lock(&m);
+  assert(rtn == 0);
+
+  // Attempt to lock a second time.  In debug builds this should
+  // hit an assertion.  In release builds this will deadlock and
+  // never return.
+  pthread_mutex_lock(&m);
+  printf("should never get here\n");
+  assert(false);
+  return 0;
+}

test/test_other.py

@@ -12942,6 +12942,12 @@ def test_pthread_reuse(self):
   def test_pthread_hello(self, args):
     self.do_other_test('test_pthread_hello.c', args)
 
+  @crossplatform
+  @node_pthreads
+  def test_pthread_mutex_deadlock(self):
+    self.do_runf('other/test_pthread_mutex_deadlock.c', 'pthread mutex deadlock detected',
+                 cflags=['-g'], assert_returncode=NON_ZERO)
+
   @node_pthreads
   def test_pthread_relocatable(self):
     self.do_run_in_out_file_test('hello_world.c', cflags=['-sRELOCATABLE'])