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 050cb6cd65f24..9b320ac31cddb 100644 --- a/test/codesize/test_codesize_minimal_pthreads.json +++ b/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)", diff --git a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json index 3076a1322f4a5..f9ed1364fbe71 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": 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)", diff --git a/test/other/test_wasm_worker_pthread_mutex_debug_allocator_regression.c b/test/other/test_wasm_worker_pthread_mutex_debug_allocator_regression.c new file mode 100644 index 0000000000000..9d0c67c4da9b8 --- /dev/null +++ b/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 +#include +#include + +#include +#include +#include + +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; +} diff --git a/test/test_other.py b/test/test_other.py index 780c164773f3a..c69bf570fde43 100644 --- a/test/test_other.py +++ b/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'])