Does this issue occur when all extensions are disabled?: Yes
- VS Code Version: 1.120.0 (main branch, commit 89fc639)
- OS Version: All platforms
Bug
In GettingStartedPage (the Welcome/Walkthrough editor), a configurationService.onDidChangeConfiguration listener for REDUCED_MOTION_KEY is registered inside the onDidProgressStep event callback using this._register(). Each time a walkthrough step is completed, a new permanent configuration listener accumulates in the disposable store and is never cleaned up.
After completing N walkthrough steps across any walkthroughs, N identical onDidChangeConfiguration listeners exist, each toggling the same CSS class on every config change. This grows without bound for the lifetime of the editor.
This is the exact anti-pattern the project's own coding guidelines warn about:
Do NOT register a disposable to the containing class if the object is created within a method that is called repeatedly to avoid leaks. Instead, return an IDisposable from such method and let the caller register it.
Affected code
File: src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts:275
this._register(this.gettingStartedService.onDidProgressStep(step => {
// ... step handling ...
// BUG: registered inside event callback, accumulates on every step completion
this._register(this.configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(REDUCED_MOTION_KEY)) {
this.container.classList.toggle('animatable', this.shouldAnimate());
}
}));
ourStep.done = step.done;
// ...
}));
Steps to Reproduce
- Open the Welcome tab (Getting Started page)
- Complete several walkthrough steps (e.g., toggle "Choose your Theme", "Rich Language Support", etc.)
- Each step completion fires
onDidProgressStep, which registers a new onDidChangeConfiguration listener
- After completing 10 steps, there are 10 identical config listeners in the disposable store
- These listeners remain until the Getting Started editor is closed
- Reopening the Getting Started editor and completing more steps adds more listeners
Impact
- Memory grows linearly with walkthrough step completions
- Each redundant listener runs on every configuration change event, wasting CPU
- Users who explore multiple walkthroughs accumulate dozens of dead listeners
Fix
A PR is available: #314636
The fix moves the onDidChangeConfiguration subscription to the constructor, alongside the other config/event listeners, so it is registered exactly once.
Does this issue occur when all extensions are disabled?: Yes
Bug
In
GettingStartedPage(the Welcome/Walkthrough editor), aconfigurationService.onDidChangeConfigurationlistener forREDUCED_MOTION_KEYis registered inside theonDidProgressStepevent callback usingthis._register(). Each time a walkthrough step is completed, a new permanent configuration listener accumulates in the disposable store and is never cleaned up.After completing N walkthrough steps across any walkthroughs, N identical
onDidChangeConfigurationlisteners exist, each toggling the same CSS class on every config change. This grows without bound for the lifetime of the editor.This is the exact anti-pattern the project's own coding guidelines warn about:
Affected code
File:
src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts:275Steps to Reproduce
onDidProgressStep, which registers a newonDidChangeConfigurationlistenerImpact
Fix
A PR is available: #314636
The fix moves the
onDidChangeConfigurationsubscription to the constructor, alongside the other config/event listeners, so it is registered exactly once.