Fix listener leak in FocusSessionActionViewItem#311759
Merged
roblourens merged 5 commits intomainfrom Apr 21, 2026
Merged
Conversation
Listeners added to debug sessions (onDidChangeName/onDidEndAdapter) in the constructor were not tied to the view item's lifetime. Since debug sessions outlive the action view item (recreated on toolbar updates), each recreation piled more listeners onto the same sessions, eventually triggering the 'potential listener LEAK detected' warning. Route all per-session listeners through a DisposableStore owned by the view item so they are released when the view item is disposed, while still self-cleaning when the session ends. Fixes #308534 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use sessionListenersStore.delete() instead of sessionListeners.dispose() so that ended sessions don't accumulate disposed stores in the parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Changed (1)
|
| Before | After |
|---|---|
 |
 |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a listener leak in FocusSessionActionViewItem by ensuring per-debug-session event listeners are properly scoped to the action view item’s lifetime, preventing repeated toolbar/title-area recreations from piling listeners onto long-lived debug sessions.
Changes:
- Introduces a
DisposableStoreowned byFocusSessionActionViewItemto track per-session listeners. - Ensures per-session listener stores are disposed both when the view item is disposed and when a session’s adapter ends.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/debug/browser/debugActionViewItems.ts | Routes onDidChangeName / onDidEndAdapter listeners through a view-item-owned DisposableStore to prevent listener accumulation across view item recreations. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
dmitrivMS
approved these changes
Apr 21, 2026
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 #308534 (Written by Copilot)
Listeners added to debug sessions (
onDidChangeName/onDidEndAdapter) inFocusSessionActionViewItem's constructor were not tied to the view item's lifetime. Since debug sessions outlive the action view item (it gets recreated on toolbar / title-area updates), each recreation piled more listeners onto the same long-lived sessions, eventually tripping the "potential listener LEAK detected, popular" warning from the REPL title toolbar.Fix: route all per-session listeners through a
DisposableStoreowned by the view item, so they're released when the view item is disposed — while still self-cleaning when the session ends (and removing themselves from the parent store so disposed children don't accumulate).(Recreated from #311745 / #311758, which had stuck CI jobs.)