Skip to content

Conversation

@brendandahl
Copy link
Collaborator

@brendandahl brendandahl commented Nov 8, 2024

Use a WebAssembly.Promising wrapper on the thread entry points to allow
JSPI to be used from worker threads. The code that invokes the wrapper
also adds async/await to handle this async call to entry point.

Fixes #22354

@brendandahl brendandahl requested review from kripken and sbc100 November 8, 2024 19:36
self.onunhandledrejection = (e) => { throw e.reason || e; };

function handleMessage(e) {
{{{ asyncIf(ASYNCIFY == 2) }}} function handleMessage(e) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like these fixes would apply to JSPI using in the main function of any pthread, not just the PROXY_TO_PTHREAD mode. Is there something specific to PROXY_TO_PTHREAD here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not totally specific to PROXY_TO_PTHREAD, more it showed up as an issue there since the entry point for the main proxy is not the users actual main.

With a user defined entry point there were ways of handling this (see core.test_pthread_join_and_asyncify). I think I can actually change that test now to not require the user to specify that the entry function is JSPI. Hower, this makes me wonder if we should automatically JSPI wrap thread entry functions or require the user to specify that they need JSPI. FWIW, we currently auto JSPI wrap main.

Copy link
Collaborator

@sbc100 sbc100 Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So an alternative solution to this one would be to auto-wrap _main_thread which is used in _emscripten_proxy_main ?

If the clang annotation was working we could use that there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So an alternative solution to this one would be to auto-wrap _main_thread which is used in _emscripten_proxy_main ?

Yeah, we'd need EMSCRIPTEN_KEEPALIVE or add it to EXPORTED_FUNCTIONS and also add it to DEFAULT_ASYNCIFY_EXPORTS.

If the clang annotation was working we could use that there?

Yeah, I still need to fix binaryen to handle the annotation.

Use a WebAssembly.Promising wrapper on the thread entry points to allow
JSPI to be used from worker threads. The code that invokes the wrapper
also adds async/await to handle this async call to entry point.

Fixes emscripten-core#22354
@brendandahl brendandahl force-pushed the jspi-proxy-to-pthread branch from aaf0481 to 7bca7aa Compare November 12, 2024 18:32
@brendandahl brendandahl changed the title [JSPI] Fix PROXY_TO_PTHREAD when used with JSPI. [JSPI] Automatically add promising JSPI wrappers to thread entry points. Nov 12, 2024
@brendandahl
Copy link
Collaborator Author

Per discussion in meeting, we'll auto wrap all thread entry points. I've removed my new test, but updated the original test to cover the case of suspending on them main thread too.

(sorry for the force push, but I wanted to amend to make sure I removed all new code)

@brendandahl
Copy link
Collaborator Author

Wasm64 was broken since it was trying to do WebAssembly.promising(func) where func was a wrapper and not the actual wasm export. I added an option to makeDynCall to add the WebAssembly.promising there instead.

@brendandahl brendandahl merged commit 015b400 into emscripten-core:main Nov 13, 2024
28 checks passed
uysalibov pushed a commit to uysalibov/emscripten that referenced this pull request Nov 15, 2024
…ts. (emscripten-core#22891)

Use a WebAssembly.Promising wrapper on the thread entry points to allow
JSPI to be used from worker threads. The code that invokes the wrapper
also adds async/await to handle this async call to entry point.

Fixes emscripten-core#22354

---------

Co-authored-by: Alon Zakai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSPI and pthreads shows RuntimeError - invalid suspender object for suspend

3 participants