Skip to content

fix: memory leak in line data event addon - #332139

Open
Simon Siefke (SimonSiefke) wants to merge 1 commit into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-lineDataEventAddon
Open

fix: memory leak in line data event addon#332139
Simon Siefke (SimonSiefke) wants to merge 1 commit into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-lineDataEventAddon

Conversation

@SimonSiefke

Copy link
Copy Markdown
Contributor

Details

Re-adding the first onLineData listener loaded the same LineDataEventAddon again, so each task rerun kept another pair of line-data callbacks reachable.

Change

Track whether the per-terminal addon has already been loaded and skip subsequent loads when listeners are re-added.

Before

When rerunning the failing task 37 times, both line-data callbacks grow once per run:

before

After

No more repeated line-data callback activation is detected.

after

Test Video

test-video.webm

Copilot AI balanced review requested due to automatic review settings August 22, 2026 20:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Prevents repeated loading of the terminal line-data addon, fixing callback retention across task reruns.

Changes:

  • Tracks whether the addon has been loaded.
  • Adds regression coverage for listener re-registration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
terminalInstance.ts Guards addon loading.
terminalInstance.test.ts Tests repeated listener registration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

test('should only load the line data event addon once when listeners are re-added', async () => {
const instance = await createTerminalInstance();
const lineDataEventAddon = instance['_lineDataEventAddon']!;
const originalActivate = lineDataEventAddon.activate.bind(lineDataEventAddon);
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Anthony Kim (@anthonykim1)

Matched files:

  • src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
  • src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts

@wenma531

Copy link
Copy Markdown

data callback activation is detected

@wenma531

Copy link
Copy Markdown

Re-adding the first onLineData listener loaded the same LineDataEv

@wenma531

Copy link
Copy Markdown

terminal/test/browser/terminalInstance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants