Skip to content

Fix Asyncify.handleAsync conflict with PROXY_SYNC_ASYNC#26513

Open
thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
thiblahute:ppoll_proxy
Open

Fix Asyncify.handleAsync conflict with PROXY_SYNC_ASYNC#26513
thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
thiblahute:ppoll_proxy

Conversation

@thiblahute
Copy link
Copy Markdown
Contributor

@thiblahute thiblahute commented Mar 21, 2026

When in a proxied context, skip the Asyncify unwind and call
startAsync() directly, letting the proxy mechanism handle the
async return.

This already affects __syscall_poll and would also affect the new
__syscall_ppoll.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Did you see I landed support for ppoll and pselect yesterday in #26482. Hopefully this change is not needed after that?

@thiblahute
Copy link
Copy Markdown
Contributor Author

thiblahute commented Mar 21, 2026

Thanks! Yes, I just saw #26482 landed ppoll support - I've rebased and dropped the ppoll commit. The remaining change here fixes a pre-existing bug in Asyncify.handleAsync when used with PROXY_SYNC_ASYNC: the test confirms it reproduces with the upstream ppoll implementation too (without the fix, you get rtn.then is not a function because handleAsync does an Asyncify unwind instead of returning a Promise in the proxied context).

@thiblahute thiblahute changed the title Implement ppoll() in JS and fix Asyncify/PROXY_SYNC_ASYNC conflict Fix Asyncify.handleAsync conflict with PROXY_SYNC_ASYNC Mar 21, 2026
@thiblahute thiblahute force-pushed the ppoll_proxy branch 6 times, most recently from 5089fe8 to 4c3d00b Compare March 23, 2026 15:22
@thiblahute thiblahute force-pushed the ppoll_proxy branch 3 times, most recently from c4ceddb to e414ec8 Compare March 23, 2026 17:35
@requires_pthreads
def test_poll_blocking_asyncify_pthread(self):
# Only testing ASYNCIFY=1: JSPI's handleAsync is a plain async function
# and doesn't have this bug. Also, with_asyncify_and_jspi can't be
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should still test both versions of ASYNCIFY here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was failing because of the limitation, getting

FAIL [0.000s]: test_poll_blocking_asyncify_pthread_jspi (test_core.core0)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/project/test/common.py", line 377, in resulting_test
    return func(self, *args)
  File "/root/project/test/test_core.py", line 257, in metafunc
    return func(self, *args, **kwargs)
  File "/root/project/test/test_core.py", line 9657, in test_poll_blocking_asyncify_pthread
    self.require_pthreads()
  File "/root/project/test/common.py", line 467, in require_pthreads
    self.fail('no JS engine found capable of running pthreads')
AssertionError: no JS engine found capable of running pthreads

@thiblahute thiblahute force-pushed the ppoll_proxy branch 2 times, most recently from eaaa24a to ce1fe4f Compare March 30, 2026 21:28
When a JS library function has both __proxy:'sync' and __async:'auto',
the compiler generates an Asyncify.handleAsync wrapper.  When called
from the PROXY_SYNC_ASYNC path on the main thread, handleAsync
triggers an Asyncify unwind instead of returning a Promise, causing
"rtn.then is not a function" in the proxy infrastructure.

Fix by generating a PThread.currentProxiedOperationCallerThread check
in handleAsyncFunction (jsifier.mjs): when in a proxied context, call
the inner function directly and skip the Asyncify unwind, letting the
proxy mechanism handle the async return.
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.

2 participants