Avoid decoration shutdown listeners for detached terminals - #332476
Merged
Dmitriy Vasyura (dmitrivMS) merged 3 commits intoAug 25, 2026
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dmitriy Vasyura (dmitrivMS)
enabled auto-merge (squash)
August 25, 2026 02:46
Copilot started reviewing on behalf of
Dmitriy Vasyura (dmitrivMS)
August 25, 2026 02:46
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Moves terminal decoration shutdown cleanup to XtermTerminal, preventing redundant lifecycle listeners for detached terminals.
Changes:
- Removes lifecycle coupling from
DecorationAddon. - Registers shutdown cleanup only for regular terminals.
- Adds regression coverage for listener counts.
Show a summary per file
| File | Description |
|---|---|
decorationAddon.ts |
Removes lifecycle dependency and listener. |
xtermTerminal.ts |
Owns conditional shutdown cleanup. |
xtermTerminal.test.ts |
Verifies regular and detached listener counts. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
Anthony Kim (anthonykim1)
approved these changes
Aug 25, 2026
Dmitriy Vasyura (dmitrivMS)
deleted the
dev/dmitriv/detached-terminal-decoration-listener-leak
branch
August 25, 2026 23:21
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.
Summary
DecorationAddonXtermTerminal, which already knows whether it is detached, own decoration shutdown cleanupXtermTerminalRoot cause
Every
DecorationAddonregistered a listener on the singletonlifecycleService.onWillShutdownemitter. Agent chat output creates many detached terminals, so tool-heavy sessions eventually crossed the listener leak threshold.Detached terminals already own and dispose their
XtermTerminal; xterm disposal unloads the addon andDecorationAddon.dispose()clears its decorations. Their global shutdown subscription was therefore redundant.Shutdown policy now belongs to
XtermTerminal, which already ownsoptions.detached. Regular terminals preserve the existing shutdown behavior by calling the addon's publicclearDecorations()API from a host-owned listener.DecorationAddonno longer knows about workbench lifecycle or detached-terminal mode.Validation
npm run transpile-clientDecorationAddonandXtermTerminalsuites: 25 passedXtermTerminalinstances add zero shutdown listenersgit diff --checkRelated issues
Fixes #311557
Related to #311225
Related to #294050
The related crash reports contain this listener signature among several independent renderer and extension-host retention paths, so they are intentionally non-closing references.