feat(core): let an embedder abandon work a request forgot - #43
Merged
Merged
Conversation
A context that serves several logical requests in sequence keeps one JS context, so a promise a request neither awaits nor hands to a background-work API stays queued on it. It resumes during a later request and observes *that* request's state -- a different tenant's environment, a different caller's identity -- which is not the state it was written against. `begin_task_scope` opens a scope and every host promise created from then on belongs to it. `cancel_task_scope` abandons the unfinished ones: each has its task aborted, so its future is dropped and whatever it held is released, and its promise is left unsettled so no continuation of it ever runs. Leaving them unsettled rather than rejecting them is the point -- a rejection handler is itself a continuation, and it would run in the wrong request. Aborting the task is what makes the release real. The cancellation flag alone is only read the next time a task is polled, and a task parked on an upstream that never answers is never polled again; it would hold its connection until the context itself went away. The flag stays as the fast path for work that is still being polled. Nothing changes for an embedder that opens no scope, which is every one of them today.
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.
Why
A context that serves several logical requests in sequence keeps one JS context, so a promise a request neither awaits nor hands to a background-work API stays queued on it. It resumes during a later request and observes that request's state — a different tenant's environment, a different caller's identity. That is not the state it was written against.
There is no way to stop it today.
shutdown_tasksis all-or-nothing and ends the context.What
Three methods on
JSContext:begin_task_scope()opens a scope and makes it current. Every host promise created from then on belongs to it.enter_task_scope(Option<TaskScope>)switches, returning the previous one, for save-and-restore around a nested call.cancel_task_scope(scope)abandons the scope's unfinished promises.Abandoning means the task is aborted, so the future is dropped and whatever it held — a connection, a buffer, an in-flight request — is released, and the promise is left unsettled so none of its continuations run.
Leaving them unsettled rather than rejecting them is deliberate: a rejection handler is itself a continuation, and it would run in the wrong request.
Aborting, not just flagging
The first cut set a cancellation flag that each task checked before its next poll. That is not enough, and
test_task_scope_releases_what_abandoned_work_holdsfails against it: a task parked on an upstream that never answers is never polled again, so it holds its connection until the context itself goes away. The scope now owns its tasks' abort handles and aborts them. The flag stays as the fast path for work still being polled.Compatibility
Nothing changes for an embedder that opens no scope, which is every one of them today —
test_promises_outside_a_scope_are_untouchedpins that.One sharp edge, documented
Entering a cancelled or unknown scope leaves no scope current rather than failing, so promises created afterwards belong to nothing and cannot be abandoned later.
test_entering_a_dead_scope_leaves_no_scope_currentpins the behaviour because the failure is otherwise silent.Tests
cargo test --features quickjs --test promise— 18 passed. Four are new.