Visitar URL original
terminal: cancel unnecessary telemetry timeout on disposal by SimonSiefke · Pull Request #333963 · microsoft/vscode · GitHub
Skip to content

terminal: cancel unnecessary telemetry timeout on disposal - #333963

Merged
Dmitriy Vasyura (dmitrivMS) merged 6 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-terminalTelemetry
Sep 28, 2026
Merged

terminal: cancel unnecessary telemetry timeout on disposal#333963
Dmitriy Vasyura (dmitrivMS) merged 6 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-terminalTelemetry

Conversation

@SimonSiefke

@SimonSiefke Simon Siefke (SimonSiefke) commented Sep 2, 2026 •
edited
Loading

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

vs-code-engineering Bot commented Sep 2, 2026

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 reviewed Sep 2, 2026
View reviewed changes

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
@dmitrivMS Dmitriy Vasyura (dmitrivMS) mentioned this pull request Sep 27, 2026
Closed
@dmitrivMS

Dmitriy Vasyura (dmitrivMS) commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Build failures:
[21:14:29] src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalTelemetry.test.ts(54,34): error TS2554: Expected 5 arguments, but got 4.

@SimonSiefke

Simon Siefke (SimonSiefke) commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Should be fixed now!

@dmitrivMS Dmitriy Vasyura (dmitrivMS) added freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues terminal General terminal issues that don't fall under another label labels Sep 28, 2026
@dmitrivMS

Dmitriy Vasyura (dmitrivMS) commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Simon Siefke (@SimonSiefke) Thank you!

@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit a054781 into microsoft:main Sep 28, 2026
35 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.141.0 milestone Sep 28, 2026
@vs-code-engineering vs-code-engineering Bot mentioned this pull request Sep 28, 2026
Closed
@SimonSiefke
Simon Siefke (SimonSiefke) deleted the fix/memory-leak-terminalTelemetry branch September 29, 2026 11:15
Abdon Morales (abdonmorales) pushed a commit to abdonmorales/vscode-utcs that referenced this pull request Oct 1, 2026
@SimonSiefke @dmitrivMS @claude
terminal: cancel unnecessary telemetry timeout on disposal (microsoft…
ae8871e 
…#333963)

* fix: cancel terminal telemetry timeout on disposal

* fix: handle terminal telemetry cancellation

* Fix terminal telemetry test configuration service dependency

---------

Co-authored-by: Dmitriy Vasyura <dmitriv@microsoft.com>
(cherry picked from commit a054781)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@aeschli Martin Aeschlimann (aeschli) aeschli approved these changes
@chrmarti Christof Marti (chrmarti) chrmarti approved these changes
@dmitrivMS Dmitriy Vasyura (dmitrivMS) dmitrivMS approved these changes

Assignees

@dmitrivMS Dmitriy Vasyura (dmitrivMS)

Labels

freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues terminal General terminal issues that don't fall under another label

Projects

None yet

Milestone

1.141.0

Development

Successfully merging this pull request may close these issues.

Promise memory leak when creating terminal (frontend)

6 participants

@SimonSiefke @dmitrivMS @aeschli @chrmarti @anthonykim1