Repository navigation
Pass sync to __async: 'auto' library functions - #28022
guybedford wants to merge 1 commit into
Conversation
An 'auto' body whose last parameter, following those of its __sig, is named `sync` receives there whether it must complete synchronously on this call, as a returned Promise could not be waited for: never under ASYNCIFY/JSPI, nor on behalf of a sync-proxied pthread caller, which awaits it; otherwise it is. __syscall_poll and epoll_pwait, which computed this by hand, now take it.
There was a problem hiding this comment.
Code Review
This pull request introduces support for __async: 'auto' library functions to conditionally execute synchronously or asynchronously using a new sync parameter. This parameter is automatically passed to the JS library function based on whether the calling context can suspend (under ASYNCIFY/JSPI) or wait (proxied pthreads). The review feedback points out a critical issue where a synchronous proxied call (where ctx is falsy) might incorrectly evaluate sync as false because PThread.currentProxiedOperationCallerThread is truthy, leading to an unawaited Promise. The reviewer suggests tracking whether the proxied operation can actually wait via a new property PThread.currentProxiedOperationCanWait and using it to determine the sync parameter.
| PThread.currentProxiedOperationCallerThread = callingThread; | ||
| var rtn = func(...proxiedJSCallArgs); | ||
| PThread.currentProxiedOperationCallerThread = 0; |
There was a problem hiding this comment.
If a synchronous proxied call (which does not support async, i.e., ctx is falsy) calls an __async: 'auto' function, PThread.currentProxiedOperationCallerThread will still be truthy. This causes sync to be evaluated as false, meaning the function will return a Promise. However, since ctx is falsy, the Promise is never awaited, and the pthread receives a Promise object (coerced to a garbage value) instead of the resolved value.
To fix this, we should track whether the current proxied operation can actually wait (i.e., whether ctx is present) and use that to determine the sync parameter.
PThread.currentProxiedOperationCallerThread = callingThread;
PThread.currentProxiedOperationCanWait = !!ctx;
var rtn = func(...proxiedJSCallArgs);
PThread.currentProxiedOperationCallerThread = 0;
PThread.currentProxiedOperationCanWait = false;There was a problem hiding this comment.
All __async pthreads function calls across threads are async, so it is only the same-thread calls (to non-proxied __async functions, or from the main thread) which need to set sync: true, and currentProxiedOperationCallerThread exhaustively handles this without needing a separate ctx flag. Under JSPI and asyncify, sync is always false.
This lets an
__async: 'auto'JS library function know whether its caller can wait for a returned Promise, so one body can block where the stack allows it and fall back to a synchronous result (typically anEAGAIN-style error) where it doesn't.Whether the caller can wait is a per-call property: under
PTHREADSit depends on whether the body is running on behalf of a sync-proxied pthread or directly on the main thread.__syscall_pollandepoll_pwaiteach computed this by hand fromPThread.currentProxiedOperationCallerThread, and the upcoming blocking socket calls need the same. So it is now passed as an argument:__sig, is namedsyncreceives there whether it must complete synchronously:falseunderASYNCIFY/JSPI and on behalf of a sync-proxied pthread caller,trueotherwise. Bodies not naming it are unchanged.syncat the wrong position (e.g. omitting unused trailing__sigparameters) is a compile error, rather than silently receiving another argument.'auto'body returning a Promise withoutASYNCIFY(on a pthread's behalf) now has itsp/jreturn conversion applied to the resolved value under wasm64, and the proxied result is converted from BigInt on the sync-async path too.__syscall_pollandepoll_pwaittakesyncinstead of computing it.ASYNCIFY/JSPI the caller must still be on a stack that can suspend.Tests:
jslib.test_jslib_async_auto_syncwith pointer-returning andvoidfunctions in plain,ASYNCIFY, JSPI andPROXY_TO_PTHREADbuilds, wasm32 and wasm64, andjslib.test_jslib_async_auto_sync_arityfor the misplaced parameter error.Made with AI assistance under my review