Skip to content

fix: guard against disposed InstantiationService in Settings editor search (fixes #330277) #330281

Description

@vs-code-engineering

Summary

SettingsEditor2.onConfigUpdate is an async method that awaits several long-running operations (experimental toggle data, installed-extension refresh, per-extension gallery manifest fetches, and createTocTreeForExtensionSettings). If the Settings editor is closed/disposed while one of those awaits is in flight, execution resumes and later calls this.instantiationService.createInstance(SettingsTreeModel, ...) on an already-disposed InstantiationService, which throws InstantiationService has been disposed. The throw escapes as an unhandled error (the search path onSearchInputChanged → triggerSearch → onConfigUpdate), producing the telemetry spike.

Fixes #330277
Recommended reviewer: @rzhao271

Culprit Commit

Field Value
Commit not identified — pre-existing
Author n/a
PR n/a
Message n/a
Why The async onConfigUpdate pattern with post-await service access is long-standing; no single commit in the regression window changed the triggering logic. The 1.132.0 spike (15.25x) most plausibly reflects increased traversal of the async extension-toggle path rather than a new defect at the crash site. Reported as pre-existing per the re-bucketing / pre-existing guidance.

Code Flow

sequenceDiagram
    participant User as User / config change
    participant Search as onSearchInputChanged / triggerSearch
    participant Cfg as onConfigUpdate (async)
    participant IS as InstantiationService
    participant Crash as createInstance

    User->>Search: change search / config
    Search->>Cfg: await onConfigUpdate()
    Note over Cfg: ⚠️ awaits gallery manifest fetches<br/>editor disposed meanwhile
    Cfg->>IS: createInstance(SettingsTreeModel, ...)
    Note over IS: store already disposed
    IS->>Crash: 💥 _throwIfDisposed()<br/>"InstantiationService has been disposed"
Loading

Affected Files

File Role Evidence
src/vs/platform/instantiation/common/instantiationService.ts crash site L69 _throwIfDisposed, L119 createInstance (from stack)
src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts root cause (async re-entry after dispose) L1457-L1670: onConfigUpdate awaits getExperimentalExtensionToggleData, refreshInstalledExtensionsList, getManifest, createTocTreeForExtensionSettings, then this.instantiationService.createInstance(SettingsTreeModel, ...) at L1670

Repro Steps

  1. Open the Settings editor with a query that engages the extension-toggle path (extensions with recommended settings, so gallery manifest fetches are triggered).
  2. Type in the search box to start triggerSearch → onConfigUpdate, which begins awaiting manifest fetches (subject to EXTENSION_FETCH_TIMEOUT_MS).
  3. Immediately close the Settings editor (or switch it out) before the awaits resolve.
  4. When the pending awaits resolve, onConfigUpdate resumes and calls createInstance on the now-disposed InstantiationService, throwing. Because timing-dependent, slow networks / many recommended-setting extensions increase the likelihood.

How the Fix Works

Chosen approach (src/vs/workbench/contrib/preferences/browser/settingsEditor2.tsonConfigUpdate): add a disposed-guard immediately after the final await point (createTocTreeForExtensionSettings, L1589) and before any synchronous service access. If this._store.isDisposed is true, the method returns early, so the disposed InstantiationService is never touched. This is a use-after-dispose async race, and the correct place for the guard is at the async re-entry boundary in the owning object (the consumer that resumed after dispose) — not inside InstantiationService (fix at the re-entry site, not the shared crash site, and never by widening/silencing the base service). The existing logService.error telemetry pipeline is untouched, and no try/catch is used to swallow the error.

Alternatives considered: wrapping the createInstance call in try/catch — rejected because it would silence a real lifecycle bug at the crash site and hide the same use-after-dispose from every other consumer, instead of stopping the disposed object from being used.

Recommended Owner

@rzhao271 — settings editor area owner and recent top contributor to settingsEditor2.ts (write access, active within the last 90 days). Culprit author cascade did not apply (pre-existing); selected via file/area ownership.

Generated by errors-fix · opus48 · 553.1 AIC · ⌖ 11.2 AIC · ⊞ 18.6K ·


Note

This was originally intended as a pull request, but PR creation failed. The changes have been pushed to the branch fix/settings-editor-disposed-race-f4f733eb0484a0ce.

Original error: ERR_API: [2026-08-11T15:49:50.625Z] create pull request in microsoft/vscode failed (attempt 1)

Original error: Validation Failed: {"resource":"PullRequest","code":"custom","field":"fork_collab","message":"fork_collab Fork collab can't be granted by someone without permission"} - https://docs.github.com/rest/pulls/pulls#create-a-pull-request
Retryable: false
Suggestion: This error cannot be resolved by retrying. Please check the error details and fix the underlying issue.

To create the pull request manually:

gh pr create --title "fix: guard against disposed InstantiationService in Settings editor search (fixes #330277)" --base main --head vscodebot-pr:fix/settings-editor-disposed-race-f4f733eb0484a0ce --repo microsoft/vscode
Show patch (31 lines)
From 9fa115766689a6d0803779561f83d1eb3ebd0941 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Tue, 11 Aug 2026 15:41:05 +0000
Subject: [PATCH] fix settings editor disposed race

---
 .../contrib/preferences/browser/settingsEditor2.ts         | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
index 604caadb715..caadcabe0bc 100644
--- a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
+++ b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
@@ -1588,6 +1588,13 @@ export class SettingsEditor2 extends EditorPane {
 
 		resolvedSettingsRoot.children!.push(await createTocTreeForExtensionSettings(this.extensionService, extensionSettingsGroups, filter));
 
+		// The editor may have been disposed while awaiting the async work above
+		// (e.g. extension manifest fetches). Bail out before touching services
+		// like the InstantiationService, which throws once disposed.
+		if (this._store.isDisposed) {
+			return;
+		}
+
 		resolvedSettingsRoot.children!.unshift(getCommonlyUsedData(groups));
 
 		if (toggleData && setAdditionalGroups) {
-- 
2.54.0

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions