Skip to content

Cancel stateless tasks before request teardown to correctly close their spans - #7137

Open
apeacock1991 wants to merge 2 commits into
mainfrom
apeacock/fix-request-teardown-order
Open

Cancel stateless tasks before request teardown to correctly close their spans#7137
apeacock1991 wants to merge 2 commits into
mainfrom
apeacock/fix-request-teardown-order

Conversation

@apeacock1991

Copy link
Copy Markdown
Contributor

Stateless context tasks can own trace spans that need the current IncomingRequest while they close. Cancel those tasks before recording the request outcome and removing the request.

Preserve abort reasons and undrained-task diagnostics while matching IoContext's task and timer destruction order.

@apeacock1991
apeacock1991 requested review from a team as code owners August 27, 2026 10:15
@apeacock1991
apeacock1991 force-pushed the apeacock/fix-request-teardown-order branch from 0a6bc2f to 063e463 Compare August 27, 2026 10:25
@ask-bonk

ask-bonk Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

@apeacock1991 Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

@apeacock1991
apeacock1991 force-pushed the apeacock/fix-request-teardown-order branch from 063e463 to a53692e Compare August 27, 2026 11:06
@ask-bonk

ask-bonk Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

Comment thread src/workerd/io/incoming-request-test.c++ Outdated
Comment thread src/workerd/io/io-context.c++ Outdated
@apeacock1991
apeacock1991 force-pushed the apeacock/fix-request-teardown-order branch 13 times, most recently from 1ed7fd3 to 92f6a25 Compare September 3, 2026 12:56
Comment thread src/workerd/io/io-context.c++
@apeacock1991
apeacock1991 force-pushed the apeacock/fix-request-teardown-order branch 2 times, most recently from 08b5779 to 34a8d61 Compare September 3, 2026 13:59
Comment thread src/workerd/io/incoming-request-test.c++ Outdated
An IncomingRequest holding the final IoContext reference can still have
context tasks, wait-until tasks, and reentry callbacks with open spans.
Cancel them while the request is current so the spans close before its
outcome is reported.

If cleanup throws, continue canceling the remaining callbacks and tasks,
then rethrow the first exception after request teardown.

Keep actor task and timeout cleanup on their existing paths.
@apeacock1991
apeacock1991 force-pushed the apeacock/fix-request-teardown-order branch from 34a8d61 to 53b4384 Compare September 3, 2026 17:15

@danlapid danlapid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put this behind an autogate and merge

Keep final-owner cancellation disabled unless the existing JSRPC tracing autogate is enabled. Leave shared-context abort cleanup unchanged.\n\nExercise legacy teardown ordering in the default test variant and early cleanup in the all-autogates variant.
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