diff --git a/AUTHORS b/AUTHORS index aa0e91eebc7ef..3ee6d0f94bf9f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -602,4 +602,5 @@ a license to everyone to use it as detailed in LICENSE.) * Christian Lloyd (copyright owned by Teladoc Health, Inc.) * Sean Morris * Mitchell Wills (copyright owned by Google, Inc.) +* Eugene Hopkinson * Han Jiang diff --git a/system/lib/libc/musl/src/thread/pthread_mutex_lock.c b/system/lib/libc/musl/src/thread/pthread_mutex_lock.c index 02269d366c858..638d4b8697d84 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_lock.c +++ b/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); } diff --git a/system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c b/system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c index 232c9b321a8e2..8a8952b67e8ae 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c +++ b/system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c @@ -1,9 +1,5 @@ #include "pthread_impl.h" -#ifdef __EMSCRIPTEN__ -#include -#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; diff --git a/system/lib/libc/musl/src/thread/pthread_mutex_trylock.c b/system/lib/libc/musl/src/thread/pthread_mutex_trylock.c index 37dd28fdd5b1a..9c7d8496a8c10 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_trylock.c +++ b/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); } diff --git a/test/codesize/test_codesize_minimal_pthreads.json b/test/codesize/test_codesize_minimal_pthreads.json index b5da211f8cb60..3d31d2572a2f1 100644 --- a/test/codesize/test_codesize_minimal_pthreads.json +++ b/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)", diff --git a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json index e83fe59e4f5dc..1ac5d2f5cc561 100644 --- a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json +++ b/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)", diff --git a/test/test_browser.py b/test/test_browser.py index bf76a59441722..3c651790195a5 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -5243,6 +5243,10 @@ 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.btest_exit('wasm_worker/pthread_mutex_debug_allocator_regression.cpp', + cflags=['-pthread', '-sWASM_WORKERS']) + @no_firefox('no 4GB support yet') @no_2gb('uses MAXIMUM_MEMORY') @no_4gb('uses MAXIMUM_MEMORY') diff --git a/test/test_other.py b/test/test_other.py index 81d892921162c..84ad681475731 100644 --- a/test/test_other.py +++ b/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): diff --git a/test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp b/test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp new file mode 100644 index 0000000000000..5adf64a38dff5 --- /dev/null +++ b/test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp @@ -0,0 +1,23 @@ +#include +#include +#include + +static void worker_loop() { + for (;;) { + delete new std::uint8_t{0}; + } +} + +static void main_loop() { + static unsigned ticks; + new std::uint8_t{0}; + if (++ticks == 120) { + emscripten_force_exit(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; +}