Skip to content

fix: memory leak in terminal find widget#283466

Merged
meganrogge merged 1 commit intomicrosoft:mainfrom
SimonSiefke:fix/memory-leak-terminal-find
Dec 16, 2025
Merged

fix: memory leak in terminal find widget#283466
meganrogge merged 1 commit intomicrosoft:mainfrom
SimonSiefke:fix/memory-leak-terminal-find

Conversation

@SimonSiefke
Copy link
Contributor

Fixes a memory leak in terminal find widget.

When calling find multiple times, it seems to register a selection change event listener each time:

this._register(Event.once(xterm.onDidChangeSelection)(() => xterm.clearActiveSearchDecoration()));

Using a MutableDisposable ensures there can be at most one selection change listener.


Perhaps another solution might be to register the listener in the constructor once. Not entirely sure if that would be the same overall logic, but if possible it would be simpler since the MutableDisposable could be completly removed.

Before

When searching something with terminal find 97 times, the number of functions in TerminalFindWidget._focusPreviousWithEvent seems to grow by 1 each time:

terminal search

After

No more leaks are detected.

Copy link
Contributor

@meganrogge meganrogge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@meganrogge meganrogge enabled auto-merge (squash) December 16, 2025 18:04
@meganrogge meganrogge added this to the December / January 2026 milestone Dec 16, 2025
@meganrogge meganrogge merged commit 53f4de2 into microsoft:main Dec 16, 2025
17 checks passed
@SimonSiefke SimonSiefke deleted the fix/memory-leak-terminal-find branch January 15, 2026 15:06
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants