fix: memory leak in language model removal notifications by SimonSiefke · Pull Request #340577 · microsoft/vscode · GitHub
Skip to content

fix: memory leak in language model removal notifications - #340577

Merged
Dmitriy Vasyura (dmitrivMS) merged 9 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-languageModelRemovalNotification
Oct 12, 2026
Merged

Dmitriy Vasyura (dmitrivMS) merged 9 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-languageModelRemovalNotification

Conversation

@SimonSiefke

Copy link
Copy Markdown
Contributor

Details

Disposing a language model provider removes its models from LanguageModelsService without firing the model-change event. Consumers keep their previous model catalogs until another change occurs.

Change

Fire the vendor's model-change event after removing the provider, model groups and listener.

Before

When registering, discovering and disposing a 37-model provider over 37 cycles, CodexContinuationService keeps 37 model entries after disposal. The leaked objects are plain data records and do not appear in named-function-count3; the complete process-memory chart below provides context for the same scenario.

before

After

The 37 stale consumer entries are gone: exact model records stay at zero. Other heap growth remains; the process-memory totals below are context rather than an estimate of savings from these small objects.

after

Test Video

test-video.webm

AI disclosure: Model: GPT-6.1-Sol. Worktime: 16 min

Copilot AI balanced review requested due to automatic review settings October 8, 2026 20:41

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

In-flight model resolution can restore stale models after provider disposal.

1 open finding
What changed in this PR

Updates language-model provider disposal so consumers refresh after models are removed.

Changes:

  • Emits a model-change event during provider disposal.
  • Adds disposal notification and idempotency tests.
File Description
src/​vs/​workbench/​contrib/​chat/​common/​languageModels.ts Notifies consumers after provider removal.
src/​vs/​workbench/​contrib/​chat/​test/​common/​languageModels.test.ts Tests provider-disposal behavior.

🧠 Review effort: Balanced


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

Comment thread src/vs/workbench/contrib/chat/common/languageModels.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

Provider-owned descriptor disposal can trigger duplicate model-change notifications.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/vs/workbench/contrib/chat/common/languageModels.ts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please consider the comment

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

Provider identity cannot distinguish separate registrations that reuse the same provider instance.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/vs/workbench/contrib/chat/common/languageModels.ts
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit 8854342 into microsoft:main Oct 12, 2026
75 of 77 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.143.0 milestone Oct 12, 2026
@SimonSiefke
Simon Siefke (SimonSiefke) deleted the fix/memory-leak-languageModelRemovalNotification branch October 13, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants