Fix pthreads normal-mutex debug implementation

AUTHORS

@@ -602,3 +602,4 @@ a license to everyone to use it as detailed in LICENSE.)
 * Christian Lloyd <clloyd@teladochealth.com> (copyright owned by Teladoc Health, Inc.)
 * Sean Morris <sean@seanmorr.is>
 * Mitchell Wills <mwills@google.com> (copyright owned by Google, Inc.)
+* Eugene Hopkinson <slowriot@armchair.software>

src/struct_info_generated.json

@@ -1036,7 +1036,7 @@
             "p_proto": 8
         },
         "pthread": {
-            "__size__": 124,
+            "__size__": 128,
             "profilerBlock": 104,
             "stack": 48,
             "stack_size": 52,

src/struct_info_generated_wasm64.json

@@ -1036,7 +1036,7 @@
             "p_proto": 16
         },
         "pthread": {
-            "__size__": 216,
+            "__size__": 224,
             "profilerBlock": 184,
             "stack": 80,
             "stack_size": 88,

system/lib/libc/musl/src/internal/pthread_impl.h

@@ -118,6 +118,9 @@ struct pthread {
 	// See emscripten_futex_wait.c.
 	_Atomic char sleeping;
 #endif
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+	pthread_mutex_t* debug_normal_mutex_list;
+#endif
 };
 
 enum {
@@ -165,6 +168,38 @@ enum {
 #define _b_waiters2 __u.__vi[4]
 #define _b_inst __u.__p[3]
 
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+static inline void __emscripten_debug_normal_mutex_note_locked(pthread_t self, pthread_mutex_t *m)
+{
+	m->_m_next = self->debug_normal_mutex_list;
+	m->_m_prev = 0;
+	self->debug_normal_mutex_list = m;
+}
+
+static inline int __emscripten_debug_normal_mutex_owned(pthread_t self, pthread_mutex_t *m)
+{
+	for (pthread_mutex_t *it = self->debug_normal_mutex_list; it; it = (pthread_mutex_t*)it->_m_next) {
+		if (it == m) {
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static inline void __emscripten_debug_normal_mutex_note_unlocked(pthread_t self, pthread_mutex_t *m)
+{
+	pthread_mutex_t **it = &self->debug_normal_mutex_list;
+	while (*it && *it != m) {
+		it = (pthread_mutex_t**)&(*it)->_m_next;
+	}
+	if (*it == m) {
+		*it = (pthread_mutex_t*)m->_m_next;
+	}
+	m->_m_prev = 0;
+	m->_m_next = 0;
+}
+#endif
+
 #ifndef TP_OFFSET
 #define TP_OFFSET 0
 #endif

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

@@ -86,13 +86,22 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
 		int own = r & 0x3fffffff;
 		if (!own && (!r || (type&4)))
 			continue;
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+		// Extra check for deadlock in debug builds, but only if we would block
+		// forever (at == NULL). For normal mutexes in debug Emscripten builds,
+		// _m_lock preserves the historical 0/EBUSY encoding, so ownership is
+		// tracked separately in a per-thread list of held normal mutexes.
+		assert(at || (type & 15) != PTHREAD_MUTEX_NORMAL ||
+		              !__emscripten_debug_normal_mutex_owned(__pthread_self(), m) &&
+		              "pthread mutex deadlock detected");
+#endif
 		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");
+		assert(at || (type & 15) == PTHREAD_MUTEX_NORMAL ||
+		              own != __pthread_self()->tid &&
+		              "pthread mutex deadlock detected");
 #endif
 
 		a_inc(&m->_m_waiters);

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

@@ -35,7 +35,15 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 		if (m->_m_waiters) tid |= 0x80000000;
 		self->robust_list.pending = &m->_m_next;
 	}
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) {
+		tid = EBUSY;
+	} else {
+		tid |= old & 0x40000000;
+	}
+#else
 	tid |= old & 0x40000000;
+#endif
 
 	if (a_cas(&m->_m_lock, old, tid) != old) {
 		self->robust_list.pending = 0;
@@ -53,10 +61,23 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 	}
 #endif
 
-#if defined(__EMSCRIPTEN__) || !defined(NDEBUG)
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+	// In debug Emscripten builds, keep normal mutexes encoded the same way as
+	// the fast path (0/EBUSY) so internal users such as dlmalloc still see the
+	// historical lock-word semantics, and track ownership separately in a
+	// per-thread list for the deadlock assertion in pthread_mutex_timedlock.
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) {
+		__emscripten_debug_normal_mutex_note_locked(self, m);
+		self->robust_list.pending = 0;
+		return 0;
+	}
+#elif 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;
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) {
+		self->robust_list.pending = 0;
+		return 0;
+	}
 #endif
 
 	next = self->robust_list.head;

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

@@ -31,6 +31,11 @@ int __pthread_mutex_unlock(pthread_mutex_t *m)
 			((char *)next - sizeof(void *)) = prev;
 	}
 #ifdef __EMSCRIPTEN__
+#if !defined(NDEBUG)
+	if (type == PTHREAD_MUTEX_NORMAL) {
+		__emscripten_debug_normal_mutex_note_unlocked(__pthread_self(), m);
+	}
+#endif
 	cont = a_swap(&m->_m_lock, new);
 #else
 	if (type&8) {

test/codesize/test_codesize_minimal_pthreads.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7363,
   "a.out.js.gz": 3604,
-  "a.out.nodebug.wasm": 19003,
-  "a.out.nodebug.wasm.gz": 8786,
-  "total": 26366,
-  "total_gz": 12390,
+  "a.out.nodebug.wasm": 19009,
+  "a.out.nodebug.wasm.gz": 8781,
+  "total": 26372,
+  "total_gz": 12385,
   "sent": [
     "a (memory)",
     "b (exit)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7765,
   "a.out.js.gz": 3810,
-  "a.out.nodebug.wasm": 19004,
-  "a.out.nodebug.wasm.gz": 8787,
-  "total": 26769,
-  "total_gz": 12597,
+  "a.out.nodebug.wasm": 19010,
+  "a.out.nodebug.wasm.gz": 8782,
+  "total": 26775,
+  "total_gz": 12592,
   "sent": [
     "a (memory)",
     "b (exit)",

test/test_browser.py

@@ -5243,6 +5243,16 @@ def test_wasm_worker_proxied_function(self):
     # Test that code does not crash in ASSERTIONS-disabled builds
     self.btest('wasm_worker/proxied_function.c', expected='0', cflags=['--js-library', test_file('wasm_worker/proxied_function.js'), '-sWASM_WORKERS', '-sASSERTIONS=0'])
 
+  def test_wasm_worker_pthread_mutex_debug_allocator_regression(self):
+    self.set_setting('ASSERTIONS')
+    self.btest('wasm_worker/pthread_mutex_debug_allocator_regression.cpp',
+               expected='0', cflags=['-pthread', '-sWASM_WORKERS'])
+
+  def test_wasm_worker_pthread_mutex_debug_reporting_teardown(self):
+    self.set_setting('ASSERTIONS')
+    self.btest('wasm_worker/pthread_mutex_debug_reporting_teardown.cpp',
+               expected='0', cflags=['-pthread', '-sWASM_WORKERS'])
+
   @no_firefox('no 4GB support yet')
   @no_2gb('uses MAXIMUM_MEMORY')
   @no_4gb('uses MAXIMUM_MEMORY')

test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp

@@ -0,0 +1,25 @@
+#include <cstdint>
+#include <emscripten.h>
+#include <emscripten/wasm_worker.h>
+
+static void worker_loop() {
+  for (;;) {
+    delete new std::uint8_t{0};
+  }
+}
+
+static void main_loop() {
+  static unsigned ticks;
+  static bool reported;
+  new std::uint8_t{0};
+  if (!reported && ++ticks == 120) {
+    reported = true;
+    REPORT_RESULT(0);
+  }
+}
+
+int main() {
+  emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024 * 1024), worker_loop);
+  emscripten_set_main_loop(main_loop, 0, false);
+  return 0;
+}

test/wasm_worker/pthread_mutex_debug_reporting_teardown.cpp

@@ -0,0 +1,25 @@
+#include <cstdint>
+#include <emscripten.h>
+#include <emscripten/wasm_worker.h>
+
+static void worker_loop() {
+  for (;;) {
+    delete new std::uint8_t{0};
+  }
+}
+
+static void main_loop() {
+  static unsigned ticks;
+  static bool reported;
+  new std::uint8_t{0};
+  if (!reported && ++ticks == 30) {
+    reported = true;
+    REPORT_RESULT(0);
+  }
+}
+
+int main() {
+  emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024 * 1024), worker_loop);
+  emscripten_set_main_loop(main_loop, 0, false);
+  return 0;
+}