Repository navigation
Let every part of an *Async call observe its cancellation token - #4260
Merged
Merged
Conversation
The token an *Async entry is handed now travels with the engine's async reservation: ReserveAsyncHostOperation installs it and whichever release drops the reservation clears it, so a host callback still finishing under the call keeps observing it. Every script-running entry - EvaluateAsync, ExecuteAsync, InvokeAsync, Modules.ImportAsync, UnwrapIfPromiseAsync and WebApi.InvokeFetchHandlerAsync - shares one RunOnReservationAsync body that refuses an already-cancelled token before anything runs. Work that outlives the call captures the token once beside the registered constraint's (EngineCancellation) and links the raw tokens into a source it owns: fetch, XMLHttpRequest, WebSocket, EventSource, AsyncModuleLoader loads and ModuleLoadCompletion's propagation rule, the blocking drain behind UnwrapIfPromise/Modules.Import, and the WaitForScheduledWork pump. The settlement loop re-checks the token right after its wait, and no longer rewraps cancellations it did not raise. Docs (async guide, TM-10, the sebastienros#4226 migration section) now state what the token reaches and what it does not, including the queued jobs a cancelled call leaves behind and the snapshot restore that clears them. Adds AsyncEntryTokenBenchmark. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Sm5FdCDxoX4BMKdcK2CMY
… *Async call's token Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Sm5FdCDxoX4BMKdcK2CMY
Collaborator
Author
|
Paired measurement of the new
The first number in each cell is the median change; the bracketed pair is its 95% CI. Bold rows have a CI that excludes zero.
If the live-token cost ever matters, the follow-up would be letting a token's registration callback raise a flag that the per-statement path already checks. That makes a token that is never cancelled free. It's a separate change and not needed for correctness. |
9 tasks
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
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.
Follow-up to #4226. A review found that an
*Asynccall's token reached only the interpreter, while the docs said it covered the whole call. This PR makes the code match those docs, then rewrites the docs to say exactly what the code now guarantees.The design: one token, carried by the reservation
ReserveAsyncHostOperation(entryToken: …)installs it inEngine._asyncEntryToken. Whichever release actually drops the reservation clears it:ReleaseAsyncHostOperation, orReleaseHostCallbackAdmissiononce the last callback admitted under the reservation finishes. The token therefore lives exactly as long as the reservation.RunOnReservationAsync. It refuses an already-cancelled token before anything runs, so a new entry cannot forget the check. The entries areEvaluateAsyncandExecuteAsync(both overloads each),InvokeAsync,UnwrapIfPromiseAsync,Modules.ImportAsyncandWebApi.InvokeFetchHandlerAsync.Engine.CaptureCancellation(), which returns an internalEngineCancellation: the entry's token beside a registeredCancellationConstraint's. The work then links the raw tokens into a source it owns and disposes itself, never one somebody else may have disposed. A linked source is allocated only when more than one token can fire.Findings
FetchOperation,XhrOperation,WebSocketOperationandEventSource(captured once per object, so a reconnect after the call still observes it) all use the captured cancellation. So doAsyncModuleLoader(it handsLoadModuleContentsAsyncthe completion's token),ModuleLoadCompletion(it owns and disposes the linked source when it settles),MustPropagateLoaderException(a loader'sOperationCanceledExceptionpropagates once either token has fired),DrainEventLoopUntil, and theWaitForScheduledWorkpump. Rewritten:docs/guide/async.md, TM-10, the #4226 migration section, the XML remarks, and theInvokeFetchHandlerAsyncparagraph infetch-and-networking.md.OperationCanceledExceptioncarrying the entry token; a registered constraint still wins, as it does inCheckAmortizedConstraints.UnwrapIfPromiseAsyncandInvokeFetchHandlerAsyncskip the tokenRunOnReservationAsync, so the fetch handler's own synchronous run is cancellable too.ThrowIfAsyncEntryCancelledreads the field twiceOperationCanceledExceptionkeeps its identity. ThePromiseTimeoutcatch is narrowed to the wait's own token.Engine.Advanced.RestoreGlobalSnapshot, or discarding the engine. Two public tests pin both halves.OperationCanceledExceptioneven for a host that also registeredObserveCancellation. (2)InvokeFetchHandlerAsyncno longer runs the handler for a pre-cancelled token. (3)UnwrapIfPromiseAsyncwith a pre-cancelled token is cancelled even for a settled promise. (4) A loader's cancellation now fails the import instead of rejecting it.#nullable enableinAsyncEntryCancellationTests.cs. The cancellation remarks are now four caller-guidance paragraphs onEvaluateAsync(string, …), inherited with<inheritdoc path="/remarks"/>by the other four entries. TheThrow.OperationCanceledException(string, Exception, CancellationToken)helper was not added, because finding 7 removed the only throw site that needed it.AsyncEntryTokenBenchmarkwith rowsEvaluate(control),EvaluateAsyncWithoutTokenandEvaluateAsyncWithLiveToken, each over3d-cube,access-nbody,bitops-bitwise-andandcrypto-md5. Every op builds its own engine. The class uses only APIs that exist at97e553b28. Filter:--filter "Jint.Benchmark.AsyncEntryTokenBenchmark.*"(12 cases). Not run here.Contract changes
InvokeFetchHandlerAsyncwith an already-cancelled token is now cancelled before the handler runs. It used to run the handler with an abortedrequest.signal. This follows from finding 3. The public test that pinned the old behaviour is replaced (TheAwaitableShapeRunsNoHandlerForAnAlreadyCancelledToken). The polledInvokeFetchHandler(request, token)still runs the handler, and the docs point to it.Tests
Failing first. Every behaviour test below failed against unfixed
upstream/main, with only an internal admission counter added there so the finding-4 test compiles:Jint.Tests:AHostCallbackStillRunningUnderTheReservationObservesTheToken(4) andAHostCancellationRaisedWhileAwaitingKeepsItsOwnIdentity(7).Jint.Tests.PublicInterface:ANestedBlockingUnwrapIsWokenByTheCallsToken,ANestedBlockingUnwrapAfterTheCallWasCancelledDoesNotWait,ANestedBlockingImportIsWokenByTheCallsTokenandANestedParkIsWokenByTheCallsToken(2).UnwrapIfPromiseAsyncStopsALoopingContinuation,TheAwaitableShapesTokenStopsAHandlerThatNeverYieldsandTheAwaitableShapeRunsNoHandlerForAnAlreadyCancelledToken(3).TheCallsTokenReachesTheLoadersFetch,ALoadersCancellationForTheCallsTokenPropagatesandTheCallsTokenReachesAFetchThatOutlivesTheCall(1).ACancelThatWakesTheWaitWinsOverATurnThatWouldSettle(5). It is deterministic: the timer is due on a manual clock before the token is cancelled, so the cancel is the only thing that can wake the wait.Pins.
ACancelledCallLeavesItsQueuedJobsForTheNextPumpandRestoringASnapshotDiscardsWhatACancelledCallQueuedpin the documented behaviour for finding 8.Release runs on the rebased branch:
Jint.TestsJint.TestsJint.TestsJint.Tests.PublicInterfaceJint.Tests.PublicInterfaceJint.Tests.PublicInterfaceThe full solution builds in Release.
Overlap
ConvertResultedits are untouched.Jint/Constraints/AGENTS.mdstill names the I/O operations as readingConstraints.Find<CancellationConstraint>(). That is still true, and the entry token is now captured beside it. It was left alone to stay clear of Check a shared string's copy against LimitMemory before ConvertResult makes it #4256's edits to that file.🤖 Generated with Claude Code
https://claude.ai/code/session_015Sm5FdCDxoX4BMKdcK2CMY