Repository navigation
Fix SuspendError in dlopen with -pthread -sJSPI - #28056
ShuangLiu1992 wants to merge 1 commit into
Conversation
…sing Under JSPI every `__async: 'auto'` import is wrapped in `WebAssembly.Suspending`, and `Asyncify.handleAsync` is an `async` function, so such an import always suspends, even when its work is synchronous. Two dynamic-linking paths reach one from a context that is not promising and trap with "SuspendError: trying to suspend without WebAssembly.promising": * A thread started after a library was loaded catches up from its entry point (`invokeEntryPoint` -> `_emscripten_dlsync_self` -> `_dlopen_js`). `_emscripten_dlsync_self` only runs on background threads, which can always load synchronously, so it now calls a new non-async `_dlopen_js_sync`. * `emscripten_dlopen`'s success path, and `dlsym` called from its callback, sync the other threads (`load_library_done`/`__dlsym` -> `_emscripten_dlsync_threads`). On the main thread that is a synchronous loop over the threads, so it now calls a new non-async `_emscripten_dlsync_threads_sync`. Off the main thread the call still proxies to `_emscripten_dlsync_threads`, whose async branch is unchanged. Tests: a `pthread_jspi` variant of `browser.test_dlopen_async` and a `jspi` variant of `browser.test_pthread_dlopen`, which now also starts a thread after the dlopen. Both fail with the SuspendError without this change.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces synchronous versions of _dlopen_js and _emscripten_dlsync_threads (named _dlopen_js_sync and _emscripten_dlsync_threads_sync respectively) to prevent WebAssembly JSPI from trapping with suspension errors when executing in non-promising contexts, such as thread startup or dlopen callbacks. However, a critical typo was identified in src/lib/libpthread.js where _emscripten_dlsync_threads_sync is called with an extra leading underscore, which will lead to a runtime ReferenceError.
| if (callingThread) { | ||
| return dlsyncThreadsAsync(); | ||
| } | ||
| __emscripten_dlsync_threads_sync(); |
There was a problem hiding this comment.
There is a typo here: __emscripten_dlsync_threads_sync() has two leading underscores, but the dependency and the function are defined with a single leading underscore (_emscripten_dlsync_threads_sync). This will cause a ReferenceError at runtime. It should be changed to _emscripten_dlsync_threads_sync().
_emscripten_dlsync_threads_sync();There was a problem hiding this comment.
This is intentional: a JS library symbol _foo is emitted as the JS function __foo (one extra leading underscore), the same convention the existing __emscripten_proxy_dlsync(pthread_ptr) call below relies on for '_emscripten_proxy_dlsync'. The generated glue defines var __emscripten_dlsync_threads_sync = () => {...}, and browser.test_dlopen_async_pthread_jspi exercises this call. A single underscore would be the ReferenceError.
With
-pthread -sJSPI, two dlopen paths call an__asyncimport from a non-promising context and trap withSuspendError: trying to suspend without WebAssembly.promising. Under JSPI such imports always suspend, even when the work is synchronous.invokeEntryPoint→_emscripten_dlsync_self→_dlopen_js. Background threads can always load synchronously, so this now calls a new non-async_dlopen_js_sync.emscripten_dlopensuccess callback (anddlsyminside it):load_library_done→_emscripten_dlsync_threads. On the main thread this is a plain loop, so it now calls a new non-async_emscripten_dlsync_threads_sync. Other threads still proxy to_emscripten_dlsync_threadsas before.Similar to #27633 (OPFS under
-pthread -sJSPI).Tests: new
browser.test_dlopen_async_pthread_jspiandbrowser.test_pthread_dlopen_jspi(the test now also starts a thread after the dlopen). Both fail without this change. Tested locally in Chrome on top of 6.0.9; not yet built onmain.🤖 Generated with Claude Code