Revert 4.0.11 threads normal-mutex debug implementation

AUTHORS

@@ -602,4 +602,5 @@ 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>
 * Han Jiang <jhcarl0814@gmail.com>

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

@@ -2,12 +2,9 @@
 
 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,9 +1,5 @@
 #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))
@@ -61,12 +57,9 @@ 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;
@@ -89,11 +82,6 @@ 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,12 +53,6 @@ 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;
@@ -77,11 +71,8 @@ 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);
 }
 

test/codesize/test_codesize_minimal_pthreads.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7367,
   "a.out.js.gz": 3603,
-  "a.out.nodebug.wasm": 19003,
-  "a.out.nodebug.wasm.gz": 8786,
-  "total": 26370,
-  "total_gz": 12389,
+  "a.out.nodebug.wasm": 18991,
+  "a.out.nodebug.wasm.gz": 8771,
+  "total": 26358,
+  "total_gz": 12374,
   "sent": [
     "a (memory)",
     "b (exit)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7769,
   "a.out.js.gz": 3809,
-  "a.out.nodebug.wasm": 19004,
-  "a.out.nodebug.wasm.gz": 8787,
-  "total": 26773,
-  "total_gz": 12596,
+  "a.out.nodebug.wasm": 18992,
+  "a.out.nodebug.wasm.gz": 8772,
+  "total": 26761,
+  "total_gz": 12581,
   "sent": [
     "a (memory)",
     "b (exit)",

test/other/test_wasm_worker_pthread_mutex_debug_allocator_regression.c

@@ -0,0 +1,44 @@
+// Regression test for https://github.com/emscripten-core/emscripten/issues/26619
+
+#include <stdatomic.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <emscripten.h>
+#include <emscripten/console.h>
+#include <emscripten/wasm_worker.h>
+
+static _Atomic int running_threads = 0;
+
+static void malloc_loop(void) {
+  running_threads += 1;
+  while (running_threads != 2) {}
+
+  for (int i = 0; i < 1000000; ++i) {
+    free(malloc(1));
+  }
+}
+
+static void worker_callback(void) {
+  emscripten_outf("worker_callback");
+  malloc_loop();
+}
+
+static void main_callback(void* arg) {
+  (void)arg;
+  emscripten_outf("main_callback");
+  malloc_loop();
+  emscripten_outf("done");
+  emscripten_terminate_all_wasm_workers();
+  emscripten_force_exit(0);
+}
+
+int main(void) {
+  emscripten_outf("main");
+  emscripten_wasm_worker_post_function_v(
+    emscripten_malloc_wasm_worker(1024 * 1024),
+    worker_callback
+  );
+  emscripten_async_call(main_callback, NULL, 0);
+  return 0;
+}

test/test_other.py

@@ -11723,6 +11723,10 @@ def test_pthread_reuse(self):
   def test_pthread_hello(self, args):
     self.do_other_test('test_pthread_hello.c', args)
 
+  # Preserved for future reinstatement of the debug deadlock check from #24607.
+  # This rollback intentionally removes that behavior and returns debug builds to
+  # the pre-4.0.11 fast-path behavior for PTHREAD_MUTEX_NORMAL.
+  @disabled('disabled by revert-pthread-mutex-debug-deadlock-detection rollback')
   @crossplatform
   @requires_pthreads
   def test_pthread_mutex_deadlock(self):
@@ -13559,6 +13563,13 @@ def test_wasm_worker_preprocessor_flags(self):
   def test_wasm_worker_pthread_api_usage(self):
     self.assert_fail([EMCC, test_file('wasm_worker/wasm_worker_pthread_api_usage.c'), '-sWASM_WORKERS'], 'undefined symbol: pthread_mutex_lock')
 
+  @also_with_minimal_runtime
+  def test_wasm_worker_pthread_mutex_debug_allocator_regression(self):
+    # Regression test for https://github.com/emscripten-core/emscripten/issues/26619
+    self.do_runf(test_file('other/test_wasm_worker_pthread_mutex_debug_allocator_regression.c'),
+                 'done\n',
+                 cflags=['-pthread', '-sWASM_WORKERS', '-sEXIT_RUNTIME'])
+
   @also_with_minimal_runtime
   def test_wasm_worker_cxx_init(self):
     self.do_run_in_out_file_test('wasm_worker/wasm_worker_cxx_init.cpp', cflags=['-sWASM_WORKERS'])