fix: cancel pending single-pane docked-tab reconciles on dispose (fixes #333537)

[VS Code PR Bot (vscodebot-pr)]

Summary

The single-pane docked-tabs reconcile pipeline runs asynchronously on a Sequencer. When SinglePaneDockedTabsCoordinator is disposed (session switch / Existing-session strategy teardown / window close) while a reconcile is still queued or in-flight, that reconcile keeps running and eventually calls IEditorGroup.replaceEditors, which instantiates an editor pane through the coordinator's IInstantiationService. By then the DI tree has been disposed, so createInstance throws InstantiationService has been disposed. Impact: an unhandled error on the Agent Sessions window during teardown/session switching (Mac + Windows, 1.136 insiders).

Fixes #333537

Recommended reviewer: @sandy081

Culprit Commit

Field	Value
Commit	f45fb5350b9
Author	@sandy081
PR	#330573
Message	sessions: stabilize single pane lifecycle behavior
Why	This commit introduced the Sequencer-based asynchronous reconcile pipeline (queueReconcile to _reconcile to _reconcileCore) with generation checkpoints, but dispose() was never overridden to cancel work already queued on the sequencer. A reconcile queued before disposal therefore survives teardown and reaches replaceEditors, which instantiates an editor pane through the disposed instantiation service.

The reconcile pipeline has since been extended (e.g. #332365, "Show Changes view for new sessions", which added the _reconcileForeignChangesEditors to replaceEditors path seen in the stack), but the missing disposal cancellation is the underlying defect.

Code Flow

sequenceDiagram
    participant Trigger as Ambient trigger
    participant Queue as queueReconcile / Sequencer
    participant Reconcile as _reconcileCore
    participant Group as replaceEditors
    participant DI as InstantiationService

    Trigger->>Queue: queueReconcile(target, trigger)
    Note over Queue: reconcile queued on Sequencer
    Note over DI: Root cause: coordinator disposed,<br/>DI tree torn down,<br/>queued reconcile not cancelled
    Queue->>Reconcile: _reconcile(generation) runs after dispose
    Reconcile->>Group: _reconcileForeignChangesEditors then replaceEditors
    Group->>DI: createInstance(editor pane)
    Note over DI: Error thrown:<br/>InstantiationService has been disposed

Loading

Affected Files

File	Role	Evidence
src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts	root cause / fix	L288-L297: queueReconcile queues _reconcile(generation) on this._sequencer; class had no dispose() override to cancel queued work
src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts	crash path	L458-L478: _reconcileForeignChangesEditors calls group.replaceEditors([{ replacement: this._instantiationService.createInstance(...) }])
src/vs/workbench/browser/editor.ts	crash site	L78 (from stack): instantiate to createInstance
src/vs/platform/instantiation/common/instantiationService.ts	throw site	L69 (from stack): _throwIfDisposed throws InstantiationService has been disposed

Repro Steps

This is a teardown/dispose race, so it is timing-dependent:

1. Open the Agent Sessions window with a workspace-backed session that shows managed docked tabs (Changes + Files).
2. Trigger a reconcile (switch sessions, reveal the side pane, or open/close an editor) so a reconcile is queued on the sequencer.
3. Immediately dispose the coordinator before the queued reconcile resolves — e.g. switch away to a different session (Existing to New/QuickChat) or close the window while the async replaceEditors/openEditor step is pending.
4. The surviving reconcile reaches replaceEditors to createInstance on the disposed instantiation service and throws.

To increase likelihood: rapidly switch sessions or close the window right after a session switch, which maximizes the window between "reconcile queued" and "reconcile reaches an editor-open call."

How the Fix Works

Lifecycle pattern: use-after-dispose.
Producer site: SinglePaneDockedTabsCoordinator.dispose() (previously absent) — the disposable owner did not cancel callbacks already queued on this._sequencer.
Fix location: singlePaneDockedTabsCoordinator.ts — new dispose() override plus entry guards in _reconcile / _reconcileCore.

Chosen approach (producer-side; fix where the lifecycle is owned rather than guarding the crash site):

• Added an override dispose() that bumps this._generation and clears this._pending before super.dispose(). Bumping the generation makes any reconcile still queued on the sequencer bail at its generation checkpoints, so it never proceeds to open/replace editors after teardown.
• Added this._store.isDisposed to the early-return guard in _reconcile, and a this._store.isDisposed check immediately before the first async editor operation in _reconcileCore (_reconcileForeignChangesEditors), which is where the reported stack crashes. This covers a reconcile that was already past its entry check when disposal happened.

This follows the data-producer / lifecycle-owner principle: the coordinator owns the sequencer and the DI-backed editor operations, so cancelling its own in-flight work on dispose is the correct place for the fix — not a try/catch at the createInstance crash site, which would swallow the error and hide any other genuine disposal bug from telemetry.

Alternatives considered:

• Wrap createInstance/replaceEditors in try/catch and swallow the disposed error — rejected: it hides the symptom at the crash site instead of stopping the stale async work at its lifecycle owner, and would silence unrelated disposal errors from telemetry.
• Check isDisposed only inside queueReconcile — rejected: it does not stop reconciles already queued/in-flight at the moment of disposal, which is exactly the race in the stack trace.

Recommended Owner

@sandy081 — author of the recent single-pane reconcile/lifecycle commits (#330573 introducing the sequencer pipeline, #332365) and owner of the src/vs/sessions/contrib/layout area.

Generated by errors-fix · opus48 · 451.1 AIC · ⌖ 12 AIC · ⊞ 18.6K · ◷

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Benjamin Christopher Simmonds (@benibenj)

Matched files:

• src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts
• src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts
• src/vs/sessions/contrib/layout/test/browser/layoutControllerTestUtils.ts

[Copilot]

Pull request overview

Prevents stale single-pane docked-tab reconciles from using disposed editor services during Sessions teardown.

Changes:

• Invalidates pending reconciles during disposal.
• Adds disposal guards before reconciliation and editor operations.

---

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

[vs-code-engineering]

Driver cycle recorded

_{errors-fix-driver:cycle head:667473acc768f841356bdc4a35bebd4d58edf0b8}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:317

• These comments overstate the cancellation semantics: generation checkpoints cannot cancel the editor operation currently being awaited (the new test deliberately lets that Changes open complete after disposal); they only prevent subsequent operations. Describe queued work and checkpoint invalidation instead of promising that in-flight/resumed work never touches editors.

		// Cancel any pending/in-flight reconciles queued on the sequencer: bumping the
		// generation makes queued reconciles bail at their entry (and in-flight ones at their
		// next generation checkpoint) so none open editors — which would instantiate an editor
		// pane through the now-disposed instantiation service after teardown.

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:330

• The regression test stalls after _queue has already invoked _reconcile, so it validates the generation checkpoint but never exercises this new queued-task guard. Add a case that blocks one sequencer task, queues a collapse/restore/files-removal task, disposes the controller, then verifies the queued task performs no editor operation; otherwise the non-reconcile disposal fix can regress unnoticed.

	private _queue(task: () => Promise<void>): void {
		void this._sequencer.queue(() => this._store.isDisposed ? Promise.resolve() : task()).catch(onUnexpectedError);

[vs-code-engineering]

Driver cycle recorded

_{errors-fix-driver:cycle head:8f1da41450d4b7264f93aea56a31a5152823d761}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:317

• This four-line inline comment exceeds the codebase's hard one-line limit for comments inside methods. Keep only the non-obvious lifecycle ordering constraint; the implementation and regression test already document the remaining mechanics.

		// Cancel any pending/in-flight reconciles queued on the sequencer: bumping the
		// generation makes queued reconciles bail at their entry (and in-flight ones at their
		// next generation checkpoint) so none open editors — which would instantiate an editor
		// pane through the now-disposed instantiation service after teardown.

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:330

• This guard only drops tasks that have not started; it does not cancel an in-flight reconcile. Concretely, _reconcileForeignChangesEditors can resume from await group.replaceEditors(...) after dispose() and call _closeManagedEditors(...) before control reaches the generation checkpoint in _reconcileCore, so the disposed coordinator can still mutate the editor group. Add a disposal/generation checkpoint between those awaited editor operations (and any other multi-step task) so teardown prevents every subsequent editor call.

	private _queue(task: () => Promise<void>): void {
		void this._sequencer.queue(() => this._store.isDisposed ? Promise.resolve() : task()).catch(onUnexpectedError);

[vs-code-engineering]

Driver cycle recorded

_{errors-fix-driver:cycle head:9ad658743a717c91cbd5792f852fd68b0e16aaf0}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[vs-code-engineering]

Driver cycle recorded

_{errors-fix-driver:cycle head:90a4f974416ad4b644784aef9999736d515f38ef}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:324

• This function JSDoc exceeds the project's 1–2 short-sentence limit and enumerates implementation details. Condense it to the disposal contract and leave the specific call sites/checkpoints to the code.

	/**
	 * Queues coordinator-owned async work on the sequencer with a disposal guard so that a task
	 * still queued when teardown happens never starts and touches editors through the now-disposed
	 * instantiation service. The guard is checked before the task starts; a task already running at
	 * disposal time relies on its own checkpoints (e.g. `_reconcile`'s generation checks) to bail.

src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts:3008

• Function-body comments are limited to one line. Keep the disposal-ordering constraint, but remove the multi-line narration of the following calls and assertion.

		// Dispose the controller (session-switch teardown / window close) while the reconcile is
		// stalled, then let it resume. Bumping the generation on dispose makes it bail, so it must
		// never reach a later editor open — which would instantiate a pane on the disposed DI.

[vs-code-engineering]

Driver cycle recorded

_{errors-fix-driver:cycle head:bc1a8d4d43060f7d035d9d40c24981aca3652274}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:329

• The new regression test does not exercise this disposal guard: it starts _reconcile and blocks inside openChangesEditor, so it passes after release solely because _openChangesTab observes the generation bump. Removing the _store.isDisposed check here would leave that test green, so queued non-reconcile work—the reason all sequencer tasks were routed through this helper—can regress unnoticed. Add a case that blocks the sequencer, queues a collapse/restore/files-tab task, disposes the controller, releases the blocker, and verifies the queued task never starts.

		void this._sequencer.queue(() => this._store.isDisposed ? Promise.resolve() : task()).catch(onUnexpectedError);

[Dmitriy Vasyura (dmitrivMS)]

The lifecycle owner is the right place for this fix, but the current guards do not cover the in-flight rejection in #333537. Two focused changes are needed before this reliably closes the telemetry issue.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts:324

• The active-group cancellation only exists inside _reconcileCore, but this helper now also runs _restoreCollapsedTabs, _collapseNonManagedTabs, and _removeFilesTab. If one of those operations is already awaiting an editor-service call when group.onWillDispose fires, it can continue against the disposed group, and any rejection is still reported because the coordinator itself may remain live (the new group-disposal test demonstrates this lifecycle window). Apply the same group-lifetime cancellation/error filtering to every group-bound sequencer task, not only reconciles.

	private _queue(task: () => Promise<void>): void {
		void this._sequencer.queue(() => this._store.isDisposed ? Promise.resolve() : task()).catch(error => {
			if (!this._store.isDisposed) {
				onUnexpectedError(error);