feat(fs): compiled fs/promises real thread-pool backgrounding (#971)#1010
Merged
Conversation
Compiled fs/promises was fake-async: each op ran its sync implementation inline and wrapped the result in Task.FromResult (already-completed), so the I/O ran on the calling thread with no overlap, diverging from the interpreter's real Task.Run. Replace it with genuine backgrounding. A shared $FsAsyncOp closure + FsRunAsync helper now: Ref the $EventLoop, run the sync op on the thread pool (Task.Run -> MethodInfo.Invoke), and Unref on completion — so the loop stays alive until the op (and its callback/await continuation) drains. This mirrors the proven fetch/DNS/timers pattern and the interpreter's refsEventLoopWhileInFlight. All ~21 fs/promises ops now build their args and call FsRunAsync (rm's inline logic moved to FsRmAsyncImpl). Error mapping preserved: $FsAsyncOp.Worker unwraps TargetInvocationException so a sync NodeError faults the Task with the same err.code. Event-loop liveness: the Unref is held for a short grace period (Task.Delay) before being scheduled onto the loop. The facade derives callbacks from the promise via a SEPARATE `.then` continuation registered after ours; dropping the ref immediately let a fire-and-forget callback (fs.readFile(path, cb), no awaiter) race program exit under load. The callback posts within microseconds of I/O completion, so the grace guarantees it drains before the loop goes quiescent. Awaited ops are unaffected (their pending top-level task already keeps the loop alive). Byte-identical interp==compiled (result + err.code + liveness + concurrency), standalone preserved. New dual-mode test; full suite 14482/0 (x2, non-flaky); TS conformance unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #971 — the last open child of epic #968.
Problem
Compiled
fs/promiseswas fake-async: each op ran its sync implementation inline and wrapped the result inTask.FromResult(already-completed). The I/O ran on the calling thread with no overlap, diverging from the interpreter's realTask.Run.Fix
A shared
$FsAsyncOpclosure +FsRunAsynchelper give genuine backgrounding:EventLoop.Ref()→ run the sync op on the thread pool (Task.Run→MethodInfo.Invoke) →Unrefon completion, so the loop stays alive until the op and its callback/await continuation drain. Mirrors the provenfetch/DNS/timers pattern and the interpreter'srefsEventLoopWhileInFlight. All ~21fs/promisesops now build their args and callFsRunAsync(rm's inline logic moved toFsRmAsyncImpl). The oldBegin/EndFsAsyncTryCatchhelpers are gone.Error mapping preserved:
$FsAsyncOp.WorkerunwrapsTargetInvocationExceptionso a syncNodeErrorfaults the Task with the sameerr.code.The subtle part — event-loop liveness
The facade derives callbacks from the promise via a separate
.thencontinuation registered after ours, so dropping the loop ref immediately let a fire-and-forget callback (fs.readFile(path, cb), no awaiter) race program exit under load. The Unref is therefore held a short grace period (Task.Delay) before being scheduled onto the loop; the callback posts within microseconds of I/O completion, so it always drains first. Awaited ops are unaffected (their pending top-level task keeps the loop alive regardless).Verification
New dual-mode test asserts result +
err.codeparity, loop liveness (a real fs op concurrent with a timer both complete), andPromise.allconcurrency. Byte-identical interp == compiled; standalone preserved. Full suite 14482/0 run twice (non-flaky); TS conformance unchanged. Merging this completes epic #968.