From 771a539662678536e1d3d76c80fe5dbf109a42c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Tue, 26 Mar 2024 15:17:58 +0200 Subject: [PATCH 1/3] Fix Wasm Workers calling emscripten_futex_wait() when building with pthreads support enabled. Fixes #21549. --- src/library_wasm_worker.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/library_wasm_worker.js b/src/library_wasm_worker.js index fea65f934971c..16ccc485a6978 100644 --- a/src/library_wasm_worker.js +++ b/src/library_wasm_worker.js @@ -10,6 +10,7 @@ globalThis.captureModuleArg = () => MODULARIZE ? '' : 'self.Module=d;'; globalThis.instantiateModule = () => MODULARIZE ? `${EXPORT_NAME}(d);` : ''; globalThis.instantiateWasm = () => MINIMAL_RUNTIME ? '' : 'd[`instantiateWasm`]=(i,r)=>{var n=new WebAssembly.Instance(d[`wasm`],i);return r(n,d[`wasm`]);};'; + globalThis.workerSupportsFutexWait = () => AUDIO_WORKLET ? "typeof AudioWorkletGlobalScope === 'undefined'" : '1'; null; }}} #endif @@ -95,8 +96,8 @@ addToLibrary({ // Run the C side Worker initialization for stack and TLS. __emscripten_wasm_worker_initialize(m['sb'], m['sz']); #if PTHREADS - // Record that this Wasm Worker supports synchronous blocking in emscripten_futex_wake(). - ___set_thread_state(/*thread_ptr=*/0, /*is_main_thread=*/0, /*is_runtime_thread=*/0, /*supports_wait=*/0); + // Record the pthread configuration, and whether this Wasm Worker supports synchronous blocking in emscripten_futex_wait(). + ___set_thread_state(/*thread_ptr=*/0, /*is_main_thread=*/0, /*is_runtime_thread=*/0, /*supports_wait=*/ {{{ workerSupportsFutexWait() }}}); #endif #if STACK_OVERFLOW_CHECK >= 2 // Fix up stack base. (TLS frame is created at the bottom address end of the stack) From bba7e4d5f538c2c99ac8ae28b9f17a415787dc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Tue, 27 Aug 2024 15:54:09 +0300 Subject: [PATCH 2/3] Fix implementation, add assert for thread safety, and add a test. --- src/library_wasm_worker.js | 7 +++- system/lib/pthread/emscripten_futex_wait.c | 5 ++- test/test_browser.py | 8 +++++ test/wasm_worker/wasm_worker_futex_wait.c | 39 ++++++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 test/wasm_worker/wasm_worker_futex_wait.c diff --git a/src/library_wasm_worker.js b/src/library_wasm_worker.js index 16ccc485a6978..ec50bdb1f8df0 100644 --- a/src/library_wasm_worker.js +++ b/src/library_wasm_worker.js @@ -10,7 +10,6 @@ globalThis.captureModuleArg = () => MODULARIZE ? '' : 'self.Module=d;'; globalThis.instantiateModule = () => MODULARIZE ? `${EXPORT_NAME}(d);` : ''; globalThis.instantiateWasm = () => MINIMAL_RUNTIME ? '' : 'd[`instantiateWasm`]=(i,r)=>{var n=new WebAssembly.Instance(d[`wasm`],i);return r(n,d[`wasm`]);};'; - globalThis.workerSupportsFutexWait = () => AUDIO_WORKLET ? "typeof AudioWorkletGlobalScope === 'undefined'" : '1'; null; }}} #endif @@ -33,6 +32,11 @@ #error "-sPROXY_TO_WORKER is not supported with -sWASM_WORKERS" #endif +{{{ + globalThis.workerSupportsFutexWait = () => AUDIO_WORKLET ? "typeof AudioWorkletGlobalScope === 'undefined'" : '1'; + null; +}}} + #endif // ~WASM_WORKERS @@ -97,6 +101,7 @@ addToLibrary({ __emscripten_wasm_worker_initialize(m['sb'], m['sz']); #if PTHREADS // Record the pthread configuration, and whether this Wasm Worker supports synchronous blocking in emscripten_futex_wait(). + // (regular Wasm Workers do, AudioWorklets don't) ___set_thread_state(/*thread_ptr=*/0, /*is_main_thread=*/0, /*is_runtime_thread=*/0, /*supports_wait=*/ {{{ workerSupportsFutexWait() }}}); #endif #if STACK_OVERFLOW_CHECK >= 2 diff --git a/system/lib/pthread/emscripten_futex_wait.c b/system/lib/pthread/emscripten_futex_wait.c index 30d215e0c89bb..ea408e4024065 100644 --- a/system/lib/pthread/emscripten_futex_wait.c +++ b/system/lib/pthread/emscripten_futex_wait.c @@ -21,7 +21,10 @@ 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. + // via busy spinning. Only the main browser thread is allowed to call into + // this function. It is not thread-safe to be called from any other thread. + assert(emscripten_is_main_browser_thread()); + double now = emscripten_get_now(); double end = now + timeout; diff --git a/test/test_browser.py b/test/test_browser.py index 46719d218e3b2..4b79abbab5093 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -4946,6 +4946,14 @@ def test_wasm_worker_hello_wasm2js(self): def test_wasm_worker_embedded(self): self.btest('wasm_worker/hello_wasm_worker.c', expected='0', args=['-sWASM_WORKERS=2']) + # Tests that it is possible to call emscripten_futex_wait() in Wasm Workers. + @parameterized({ + '': ([],), + 'pthread': (['-pthread'],), + }) + def test_wasm_worker_futex_wait(self, args): + self.btest('wasm_worker/wasm_worker_futex_wait.c', expected='0', args=['-sWASM_WORKERS=1', '-sASSERTIONS'] + args) + # Tests Wasm Worker thread stack setup @also_with_minimal_runtime def test_wasm_worker_thread_stack(self): diff --git a/test/wasm_worker/wasm_worker_futex_wait.c b/test/wasm_worker/wasm_worker_futex_wait.c new file mode 100644 index 0000000000000..480f5e26c46d6 --- /dev/null +++ b/test/wasm_worker/wasm_worker_futex_wait.c @@ -0,0 +1,39 @@ +// Test that emscripten_futex_wait() works in a Wasm Worker. + +#include +#include +#include +#include +#include +#include +#include + +_Atomic uint32_t futex_value = 0; + +void wake_worker_after_delay(void *user_data) +{ + futex_value = 1; + emscripten_futex_wake(&futex_value, INT_MAX); +} + +void wake_worker() +{ + printf("Waking worker thread from futex wait.\n"); + emscripten_set_timeout(wake_worker_after_delay, 500, 0); +} + +void worker_main() +{ + printf("Worker sleeping for futex wait.\n"); + emscripten_wasm_worker_post_function_v(0, wake_worker); + int rc = emscripten_futex_wait(&futex_value, 0, INFINITY); + printf("emscripten_futex_wait returned with code %d.\n", rc); +#ifdef REPORT_RESULT + REPORT_RESULT(rc); +#endif +} + +int main() +{ + emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024), worker_main); +} From b16ee3dbaa6d6b4e1345533f7815f100b6f55660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Tue, 27 Aug 2024 17:02:28 +0300 Subject: [PATCH 3/3] Address review --- test/wasm_worker/wasm_worker_futex_wait.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/wasm_worker/wasm_worker_futex_wait.c b/test/wasm_worker/wasm_worker_futex_wait.c index 480f5e26c46d6..7830fca63b397 100644 --- a/test/wasm_worker/wasm_worker_futex_wait.c +++ b/test/wasm_worker/wasm_worker_futex_wait.c @@ -10,20 +10,17 @@ _Atomic uint32_t futex_value = 0; -void wake_worker_after_delay(void *user_data) -{ +void wake_worker_after_delay(void *user_data) { futex_value = 1; emscripten_futex_wake(&futex_value, INT_MAX); } -void wake_worker() -{ +void wake_worker() { printf("Waking worker thread from futex wait.\n"); emscripten_set_timeout(wake_worker_after_delay, 500, 0); } -void worker_main() -{ +void worker_main() { printf("Worker sleeping for futex wait.\n"); emscripten_wasm_worker_post_function_v(0, wake_worker); int rc = emscripten_futex_wait(&futex_value, 0, INFINITY); @@ -33,7 +30,6 @@ void worker_main() #endif } -int main() -{ +int main() { emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024), worker_main); }