diff --git a/AUTHORS b/AUTHORS index 3baa0068ef86b..4b5ad08f827e5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -602,3 +602,4 @@ 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 diff --git a/src/struct_info_generated.json b/src/struct_info_generated.json index 163a1cf69779c..c5c9d7e1ee7a2 100644 --- a/src/struct_info_generated.json +++ b/src/struct_info_generated.json @@ -1036,7 +1036,7 @@ "p_proto": 8 }, "pthread": { - "__size__": 124, + "__size__": 128, "profilerBlock": 104, "stack": 48, "stack_size": 52, diff --git a/src/struct_info_generated_wasm64.json b/src/struct_info_generated_wasm64.json index 8b857fef5dbcb..18a3290fe7728 100644 --- a/src/struct_info_generated_wasm64.json +++ b/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, diff --git a/system/lib/libc/musl/src/internal/pthread_impl.h b/system/lib/libc/musl/src/internal/pthread_impl.h index b2588344eb1ab..bf5a026867617 100644 --- a/system/lib/libc/musl/src/internal/pthread_impl.h +++ b/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 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..efcc974a988d8 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c +++ b/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); 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..fae449a2212f2 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_trylock.c +++ b/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; diff --git a/system/lib/libc/musl/src/thread/pthread_mutex_unlock.c b/system/lib/libc/musl/src/thread/pthread_mutex_unlock.c index af13bcfa12012..982a2e15996e3 100644 --- a/system/lib/libc/musl/src/thread/pthread_mutex_unlock.c +++ b/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) { diff --git a/test/codesize/test_codesize_minimal_pthreads.json b/test/codesize/test_codesize_minimal_pthreads.json index b5da211f8cb60..d4c529a31dcbe 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": 19009, + "a.out.nodebug.wasm.gz": 8781, + "total": 26372, + "total_gz": 12385, "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..9c2eeda5dcefa 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": 19010, + "a.out.nodebug.wasm.gz": 8782, + "total": 26775, + "total_gz": 12592, "sent": [ "a (memory)", "b (exit)", diff --git a/test/test_browser.py b/test/test_browser.py index bf76a59441722..d1c2538ce0a65 100644 --- a/test/test_browser.py +++ b/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') 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..2c0aaa3adb2c9 --- /dev/null +++ b/test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp @@ -0,0 +1,25 @@ +#include +#include +#include + +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; +} diff --git a/test/wasm_worker/pthread_mutex_debug_reporting_teardown.cpp b/test/wasm_worker/pthread_mutex_debug_reporting_teardown.cpp new file mode 100644 index 0000000000000..e3e293bec3dbd --- /dev/null +++ b/test/wasm_worker/pthread_mutex_debug_reporting_teardown.cpp @@ -0,0 +1,25 @@ +#include +#include +#include + +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; +}