revert 4.0.11 debug deadlock slow patch changes, resolves #26619

Author: slowriot

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>

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": 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": 18991,
+  "a.out.nodebug.wasm.gz": 8771,
+  "total": 26354,
+  "total_gz": 12375,
   "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": 18992,
+  "a.out.nodebug.wasm.gz": 8772,
+  "total": 26757,
+  "total_gz": 12582,
   "sent": [
     "a (memory)",
     "b (exit)",

test/other/test_pthread_mutex_deadlock.c

@@ -10,9 +10,8 @@ int main() {
   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.
+  // Attempt to lock a second time. In debug builds with the #24607 behavior
+  // enabled this should hit an assertion. Without that behavior it deadlocks.
   pthread_mutex_lock(&m);
   printf("should never get here\n");
   assert(false);

test/test_other.py

@@ -11721,6 +11721,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):