From 7e246715b4dc4cf1abacf1cdbaa3ceef3773bd77 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 19 Dec 2022 05:57:04 -0800 Subject: [PATCH] Only use futex_wait_busy on the main browser thread When I ported this code from JS back in #15742 I mistakenly changed the semantics here. This change restores the previous behaviour of using atomic.wait except in on the main browser thread. The `futex_wait_busy` can clearly only be used a single main thread and is not appropriate for using in cases where there are other threads in the system that also don't support `atomic.wait` (such as IIUC audio worklets). To support that case we would need a different busy wait function that didn't depend on some global singleton like _emscripten_main_thread_futex. --- system/lib/pthread/emscripten_futex_wait.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/system/lib/pthread/emscripten_futex_wait.c b/system/lib/pthread/emscripten_futex_wait.c index 6225cfb0d87a9..2862412299655 100644 --- a/system/lib/pthread/emscripten_futex_wait.c +++ b/system/lib/pthread/emscripten_futex_wait.c @@ -17,7 +17,9 @@ extern void* _emscripten_main_thread_futex; int _emscripten_thread_supports_atomics_wait(void); -static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) { +static int futex_wait_main_browser_thread(volatile void* addr, + uint32_t val, + double timeout) { // Atomics.wait is not available in the main browser thread, so simulate it // via busy spinning. double now = emscripten_get_now(); @@ -119,13 +121,18 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) int ret; emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX); - // For threads that cannot block (i.e. the main browser thread) we can't use - // __builtin_wasm_memory_atomic_wait32 so we call out the JS function that - // will busy wait. + // For the main browser thread we can't use + // __builtin_wasm_memory_atomic_wait32 so we have busy wait instead. if (!_emscripten_thread_supports_atomics_wait()) { - ret = futex_wait_busy(addr, val, max_wait_ms); - emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING); - return ret; + if (emscripten_is_main_browser_thread()) { + ret = futex_wait_main_browser_thread(addr, val, max_wait_ms); + emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING); + return ret; + } else { + // TODO: handle non-main threads that also don't support `atomic.wait`. + // For example AudioWorklet. + assert(0); + } } // -1 (or any negative number) means wait indefinitely.