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
thiblahute
force-pushed
the
ppoll_proxy
branch
4 times, most recently
from
752c3e2 to
8fe0d22
Compare
| # 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>
There was a problem hiding this comment.
Yes, I tried both ways and I ended up with a "conflicting engine" on the CI as it was trying to use d8 which doesn't support pthread
There was a problem hiding this comment.
OK, this seems like something we should fix, but it doesn't have to be for this PR
There was a problem hiding this comment.
OK, what do you want me to do get to get that one in a shape you are happy with?
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.
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.