Repository navigation
fix(chromium): dispose worker sessions when their frame session is disposed - #42282
Merged
Merged
Conversation
Messages addressed to a session that no longer exists in the browser are answered with a -32001 error that has no sessionId, so it was routed to the root session, did not match any callback there and got dropped. The pending command was never resolved, e.g. context.unrouteAll() hung forever. Route such errors by the message id and reject the command as closed. Also dispose worker sessions when their frame session is disposed, otherwise a worker of a detached oopif is never removed and we keep talking to it. Fixes: microsoft#42278
- hoist the message id check into the caller, so the helpers take a number - look up the session without allocating an array - drop the redundant worker session key copy in FrameSession.dispose - assert that an oopif is actually created in the new tests
Member
|
Routing replies to different session seems weird - these callbacks are unexpected there |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Disposing worker sessions in FrameSession.dispose is enough for the reported hang: Target.detachedFromTarget for the oopif arrives before the pending command, and disposing the worker session rejects it. Bring the connection side back when there is a scenario that needs it.
This comment has been minimized.
This comment has been minimized.
Member
Author
Updated. Working with just child worker session removal along with oopif closure. |
Pavel Feldman (pavelfeldman)
approved these changes
Aug 17, 2026
Contributor
Test results for "MCP"1 failed 8100 passed, 1311 skipped Merge workflow run. |
Contributor
Test results for "tests 1"21 flaky51130 passed, 1226 skipped Merge workflow run. |
Contributor
|
Hi, I'm the Playwright bot and I took a look at the CI failures here. 🟢 CI is clear — the one failure is a pre-existing flakeThe only real failure is DetailsLatest reports: "tests 1" is green (21 flaky, 0 failed) and "MCP" has a single failure. The earlier Pre-existing flake / infra
Triaged by the Playwright bot - agent run |
Yury Semikhatsky (yury-s)
merged commit Aug 17, 2026
cf107fc
into
microsoft:main
44 of 45 checks passed
12 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
2 participants
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.
Summary
Target.detachedFromTargetis delivered on the already dead iframe session and is lost, soFrameSession.dispose()left the worker session registered in the network manager and inpage.workers().context.unrouteAll()then sentNetwork.setCacheDisabledto a session the browser had forgotten. The error reply for an unknown session carries nosessionId, so it was routed to the root session, did not match any callback there and got dropped, and the command hung forever.Fixes #42278