Skip to content

fix(server): destroy stream in StreamDispatcher._onDispose#40759

Merged
pavelfeldman merged 1 commit intomicrosoft:mainfrom
SebTardif:fix-stream-dispatcher-dispose
May 10, 2026
Merged

fix(server): destroy stream in StreamDispatcher._onDispose#40759
pavelfeldman merged 1 commit intomicrosoft:mainfrom
SebTardif:fix-stream-dispatcher-dispose

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

@SebTardif SebTardif commented May 9, 2026

Summary

  • StreamDispatcher does not override _onDispose(), so when the dispatcher hierarchy is torn down (client disconnect, page close), the underlying Readable stream from fs.createReadStream is never destroyed, leaking a file descriptor per open artifact stream.
  • Every other resource-holding dispatcher overrides _onDispose() for cleanup: TracingDispatcher, PageDispatcher, DebugControllerDispatcher, WebSocketRouteDispatcher.

Origin

StreamDispatcher was introduced in #2733 (2020-06-26). The _onDispose() pattern was established later in #19572 (2022-12-19) by dgozman, which added _onDispose() to page, context, and tracing dispatchers, but missed StreamDispatcher. A subsequent stream fix in #27291 addressed event listener leaks in the same file but not the missing _onDispose().

StreamDispatcher does not override _onDispose(), so when the
dispatcher hierarchy is torn down on client disconnect, the
underlying Readable stream is never destroyed, leaking a file
descriptor per open artifact stream.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Test results for "MCP"

9 failed
❌ [firefox] › mcp/annotate.spec.ts:273 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:297 › should annotate via direct browser_annotate MCP call @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:330 › should annotate when context has no fixed viewport @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:367 › should cancel browser_annotate when the MCP request is aborted @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:398 › should cancel browser_annotate when the MCP client disconnects @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:427 › should switch screencast to -s session on show --annotate @mcp-windows-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:476 › should disengage annotate mode when --annotate client disconnects @mcp-windows-latest-firefox
❌ [firefox] › mcp/cli-devtools.spec.ts:217 › video-start-stop @mcp-windows-latest-firefox
❌ [firefox] › mcp/cli-devtools.spec.ts:231 › video-chapter @mcp-windows-latest-firefox

7048 passed, 1068 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Test results for "tests 1"

5 flaky ⚠️ [installation tests] › screencast.spec.ts:18 › screencast works `@package-installations-windows-latest`
⚠️ [chromium-library] › library/beforeunload.spec.ts:130 › should support dismissing the dialog multiple times `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:275 › screencast › should capture navigation `@chromium-ubuntu-22.04-node24`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:812 › should update state on subsequent run `@windows-latest-node20`

41701 passed, 850 skipped


Merge workflow run.

@pavelfeldman pavelfeldman merged commit 5327da1 into microsoft:main May 10, 2026
44 of 45 checks passed
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