Skip to content

Make ctx.abort() (for stateless workers) non-experimental. - #6930

Merged
kentonv merged 2 commits into
mainfrom
kenton/enable-ctx-abort
Aug 7, 2026
Merged

Make ctx.abort() (for stateless workers) non-experimental.#6930
kentonv merged 2 commits into
mainfrom
kenton/enable-ctx-abort

Conversation

@kentonv

@kentonv kentonv commented Aug 7, 2026

Copy link
Copy Markdown
Member

All of the concerns in the TODO bullet points had already been addressed:

  • We already use TerminateExecution().
  • Abort handling has been significantly refactored (some time ago!) closing most ways that code could keep running after abort. One remaining, minor issue is fixed in the first commit in this PR.
  • The refactoring of abort handling also ensured that the correct exception propagates.
  • All event handler paths were verified as respecting abort.
  • The Durable Object version was un-gated a long time ago.

It's difficult to actually make this happen (required a low-level C++ test), but theoretically possible that an IoContext could become aborted while we waited for the isolate lock, after we'd already checked the abort exception. Re-checking again after locking solves the problem.

(Bug identified, fixed, and tested by Fable 5.)
@kentonv
kentonv requested a review from jasnell August 7, 2026 19:18
@kentonv
kentonv requested review from a team as code owners August 7, 2026 19:18
@ask-bonk

ask-bonk Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I notice the difference in style: line 1348 uses return ex.clone() (which returns a rejected promise via implicit conversion from kj::Exception to kj::PromiseForResult), while line 1370 uses kj::throwFatalException(ex.clone()). The reason is that at line 1348, we're at the top of the function and the return type is kj::PromiseForResult<Func, Worker::Lock&>, so return ex.clone() works to construct a rejected promise. But inside the .then() callback at line 1370, the return type might be non-promise (e.g., void when the function returns void), so return ex.clone() wouldn't work — kj::throwFatalException is the correct approach. This is sound.

Let me also check the pendingEvent usage in the test to make sure it's properly cleaned up:

Looking at the test: pendingEvent is a local variable, so it gets destroyed when the test function exits. The test calls fixture.drainAndDestroy(kj::mv(request)) at the end, which should handle cleanup properly. The pendingEvent is destroyed before drainAndDestroy is called (since pendingEvent is declared before first and second, and C++ destroys in reverse order — but actually, pendingEvent is destroyed after first and second since it was declared before them... let me re-read).

Actually, looking at the test flow:

  1. pendingEvent declared at line 30
  2. first promise declared at line 35
  3. second promise declared at line 39
  4. first.wait() at line 41
  5. second.wait() at line 44
  6. drainAndDestroy at line 47

After both promises are waited on, pendingEvent is still alive. Then drainAndDestroy is called. Then at scope exit, pendingEvent, second, and first are destroyed in reverse order. This should be fine since the promises have already been resolved/rejected.

This all looks correct. The changes are clean and well-tested. The only potential concern would be the types snapshot, but that's handled by CI (it will fail if the snapshot isn't regenerated). The PR description addresses all the original TODO items that were blocking the feature from going GA.

LGTM

github run

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@codspeed-hq

codspeed-hq Bot commented Aug 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing kenton/enable-ctx-abort (322d59b) with main (5374389)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

All of the concerns in the TODO bullet points had already been addressed:
* We already use TerminateExecution().
* Abort handling has been significantly refactored closing most ways that code could keep running after abort. One remaining issue was fixed in the previous commit.
* The refactoring of abort handling also ensured that the correct exception propagates.
* All event handler paths were verified as respecting abort.
* The Durable Object version was un-gated a long time ago.

(Fable 5 reviewed the code to verify all this.)
@kentonv
kentonv force-pushed the kenton/enable-ctx-abort branch from d0ccd7f to 322d59b Compare August 7, 2026 19:55
@kentonv
kentonv requested a review from a team as a code owner August 7, 2026 19:55
@kentonv
kentonv requested a review from petebacondarwin August 7, 2026 19:55
@kentonv
kentonv merged commit a5ce01f into main Aug 7, 2026
22 checks passed
@kentonv
kentonv deleted the kenton/enable-ctx-abort branch August 7, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants