Fix potential listener leak in document semantic tokens#298512
Merged
Fix potential listener leak in document semantic tokens#298512
Conversation
Hoist provider onDidChange and registry onDidChange subscriptions from each ModelSemanticColoring instance into the singleton DocumentSemanticTokensFeature. Previously, every ModelSemanticColoring subscribed individually to both the global LanguageFeatureRegistry.onDidChange and each provider's onDidChange event, resulting in O(N*M) listeners (N models × M providers). In scenarios like chat editing where many models are created rapidly, these listeners accumulated and triggered leak detection. Now the singleton subscribes once to the registry change and once per provider (via allNoModel()), then fans out notifications to watchers. Each watcher checks provider relevance via _provider.all(model).includes() before acting on the event. Also replaces manual IDisposable[] management with a DisposableStore for proper lifecycle tracking.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a potential event-listener leak in document semantic tokens by centralizing provider/registry change subscriptions in the singleton DocumentSemanticTokensFeature instead of registering them per ModelSemanticColoring instance (reducing O(N×M) listeners in scenarios with many models).
Changes:
- Hoists
LanguageFeatureRegistry.onDidChangesubscription from eachModelSemanticColoringintoDocumentSemanticTokensFeature. - Adds per-provider
onDidChangelisteners once (viaallNoModel()), then fans out notifications to active model watchers. - Replaces manual
IDisposable[]tracking with aDisposableStorefor provider change listeners.
dmitrivMS
approved these changes
Mar 1, 2026
DonJayamanne
pushed a commit
that referenced
this pull request
Mar 2, 2026
* Fix potential listener leak in document semantic tokens Hoist provider onDidChange and registry onDidChange subscriptions from each ModelSemanticColoring instance into the singleton DocumentSemanticTokensFeature. Previously, every ModelSemanticColoring subscribed individually to both the global LanguageFeatureRegistry.onDidChange and each provider's onDidChange event, resulting in O(N*M) listeners (N models × M providers). In scenarios like chat editing where many models are created rapidly, these listeners accumulated and triggered leak detection. Now the singleton subscribes once to the registry change and once per provider (via allNoModel()), then fans out notifications to watchers. Each watcher checks provider relevance via _provider.all(model).includes() before acting on the event. Also replaces manual IDisposable[] management with a DisposableStore for proper lifecycle tracking. * Review feedback
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #226407
Hoist provider onDidChange and registry onDidChange subscriptions from each ModelSemanticColoring instance into the singleton DocumentSemanticTokensFeature.
Previously, every ModelSemanticColoring subscribed individually to both the global LanguageFeatureRegistry.onDidChange and each provider's onDidChange event, resulting in O(N*M) listeners (N models × M providers). In scenarios like chat editing where many models are created rapidly, these listeners accumulated and triggered leak detection.
Now the singleton subscribes once to the registry change and once per provider (via allNoModel()), then fans out notifications to watchers. Each watcher checks provider relevance via _provider.all(model).includes() before acting on the event.
Also replaces manual IDisposable[] management with a DisposableStore for proper lifecycle tracking.