Skip to content

terminal: cancel unnecessary telemetry timeout on disposal - #333963

Open
Simon Siefke (SimonSiefke) wants to merge 3 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-terminalTelemetry
Open

terminal: cancel unnecessary telemetry timeout on disposal#333963
Simon Siefke (SimonSiefke) wants to merge 3 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-terminalTelemetry

Conversation

@SimonSiefke

@SimonSiefke Simon Siefke (SimonSiefke) commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #276610

Details

When a terminal is created, terminal telemetry waits for the process to become ready and then allows another 10 seconds for shell integration. If the terminal is closed during that delay, telemetry is sent immediately, but the unused timeout continues until it expires.

The timeout is bounded to 10 seconds, so this is not a permanent memory leak. Repeatedly creating and closing terminals can still leave several unnecessary timeout promises pending at once.

Change

Cancel the shell integration timeout when the per-terminal telemetry listeners are disposed.

Before

When creating and closing a terminal 37 times, pending terminal telemetry promises remain until their 10-second timeouts expire:

terminal-create-before

After

The timeout is canceled when the terminal closes, so the pending promise settles immediately.

Test Video

terminal-create.webm

Copilot AI balanced review requested due to automatic review settings September 2, 2026 10:08
@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/terminalContrib/telemetry/browser/terminalTelemetry.ts
  • src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalTelemetry.test.ts

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.

🟡 Changes recommended

Contribution teardown can trigger an unhandled cancellation rejection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Cancels pending terminal telemetry timeouts when per-terminal listeners are disposed.

Changes:

  • Makes the shell-integration timeout cancellation-aware.
  • Adds a regression test for terminal disposal.
File summaries
File Description
terminalTelemetry.ts Cancels the telemetry delay during disposal.
terminalTelemetry.test.ts Verifies timeout cancellation and telemetry emission.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promise memory leak when creating terminal (frontend)

3 participants