Release terminal link cache and cancel pending link providers on terminal close#319184
Open
dmitrivMS wants to merge 1 commit into
Open
Release terminal link cache and cancel pending link providers on terminal close#319184dmitrivMS wants to merge 1 commit into
dmitrivMS wants to merge 1 commit into
Conversation
…inal close ExtHostTerminalService kept per-terminal entries in _terminalLinkCache and _terminalLinkCancellationSource forever: populated them but nothing ever removed them when a terminal was closed. Each terminal that ever had links queried leaked its cached links plus a CancellationTokenSource until the extension host exited. Clear both maps in and dispose(true) the in-flight cancellation source so dangling provider promises resolve instead of hanging.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes terminal link-provider state cleanup in the extension host so closed terminals no longer retain cached links or pending cancellation sources.
Changes:
- Clears per-terminal link cache entries when
$acceptTerminalClosedruns. - Cancels and removes any in-flight terminal link provider cancellation source on terminal close.
- Adds a unit test covering cleanup and cancellation behavior.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/api/common/extHostTerminalService.ts |
Releases link cache and cancels pending link-provider work during terminal close handling. |
src/vs/workbench/api/test/common/extHostTerminalService.test.ts |
Adds coverage for cleanup of cached link state and cancellation source disposal. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
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.
Fixes a memory leak in
BaseExtHostTerminalServicewhere the per-terminal_terminalLinkCacheand_terminalLinkCancellationSourcemaps were populated by$provideLinksbut never released when a terminal was closed. Every terminal that ever had link providers queried leaked its cachedICachedLinkEntrymap plus aCancellationTokenSourceuntil the extension host exited.Cleanup is now done in
$acceptTerminalClosed(fires for all terminals, not just extension-owned ptys), and the in-flight cancellation source isdispose(true)-ed so any pending link-provider promises resolve instead of hanging.Related to #310216 — that PR put the cleanup in
_onProcessExit, which only runs for extension-pty terminals, so regular shell terminals would continue to leak.Adds a unit test covering the cleanup behaviour.