Fix Asyncify.handleAsync conflict with PROXY_SYNC_ASYNC#26513
Fix Asyncify.handleAsync conflict with PROXY_SYNC_ASYNC#26513thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
Conversation
|
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 |
thiblahute
changed the title
thiblahute
force-pushed
the
ppoll_proxy
branch
6 times, most recently
from
5089fe8 to
4c3d00b
Compare
thiblahute
force-pushed
the
ppoll_proxy
branch
3 times, most recently
from
c4ceddb to
e414ec8
Compare
test/test_core.py
Outdated
| @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 |
There was a problem hiding this comment.
I think we should still test both versions of ASYNCIFY here.
There was a problem hiding this comment.
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
ce1fe4f to
752c3e2
Compare
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.
| # require_pthreads can fail when require_jspi selects d8 (which doesn't | ||
| # support pthreads). Convert to skip since the test needs both. | ||
| if not any(engine_is_node(e) or engine_is_bun(e) for e in self.js_engines): | ||
| self.skipTest('no JS engine capable of running pthreads') |
There was a problem hiding this comment.
I don't love having these extra 4 lines here. Especially with the automatic skip thing (we try not to auto-skip stuff in general, but force folks to opt into skipping this kind of thing)
It seems like there is some kind of bad interaction here between require_pthreads and with_asyncify_and_jspi? Doers it occur whichever way around you put the decorators>
2 participants
When in a proxied context, skip the Asyncify unwind and call
startAsync()directly, letting the proxy mechanism handle theasync return.
This already affects
__syscall_polland would also affect the new__syscall_ppoll.