Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {
if (!group || group.contains(e.editor)) {
return;
}
void this._sequencer.queue(() => this._removeFilesTab(this._editorGroupsService.mainPart.activeGroup)).catch(onUnexpectedError);
this._queue(() => this._removeFilesTab(this._editorGroupsService.mainPart.activeGroup));
}));
this._register(this._editorService.onDidCloseEditor(e => {
if (e.editor instanceof EmptyFileEditorInput
Expand Down Expand Up @@ -245,7 +245,7 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {
}

if (visible) {
void this._sequencer.queue(() => this._restoreCollapsedTabs()).catch(onUnexpectedError);
this._queue(() => this._restoreCollapsedTabs());
return;
}

Expand All @@ -254,7 +254,7 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {
return;
}
if (this._layoutService.isVisible(Parts.AUXILIARYBAR_PART)) {
void this._sequencer.queue(() => this._collapseNonManagedTabs()).catch(onUnexpectedError);
this._queue(() => this._collapseNonManagedTabs());
}
}));

Expand Down Expand Up @@ -294,7 +294,7 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {
: trigger;
this._pending = { sessionKey, target, trigger: mergedTrigger };
const generation = ++this._generation;
void this._sequencer.queue(() => this._reconcile(generation)).catch(onUnexpectedError);
this._queue(() => this._reconcile(generation));
}

private _readTarget(reader: IReader | undefined): IManagedTabsTarget {
Expand All @@ -310,8 +310,28 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {

// --- Reconcile --------------------------------------------------------

override dispose(): void {
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
// 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.
this._generation++;
this._pending = undefined;
super.dispose();
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
}

/**
* Queues coordinator-owned async work on the sequencer with a disposal guard so that a task
* still queued (or resumed) after teardown never touches editors through the now-disposed
* instantiation service. Every sequencer task must go through here — the reconcile pipeline
* as well as the collapse/restore/files-tab tasks all open or close editors.
*/
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
Outdated
private _queue(task: () => Promise<void>): void {
void this._sequencer.queue(() => this._store.isDisposed ? Promise.resolve() : task()).catch(onUnexpectedError);
}

private async _reconcile(generation: number): Promise<void> {
if (generation !== this._generation || !this._pending) {
if (this._store.isDisposed || generation !== this._generation || !this._pending) {
return;
}

Expand Down Expand Up @@ -343,6 +363,9 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {
// group is never mistaken for the user closing all tabs (which would close the side pane).
const suppression = this._layoutService.suppressEditorPartAutoVisibility();
try {
if (this._store.isDisposed) {
return;
}
// [1] Replace an outgoing session's Changes tab in place when the incoming
// session also wants Changes; close only additional stale tabs.
await this._reconcileForeignChangesEditors(group, changesResource);
Expand Down Expand Up @@ -536,7 +559,7 @@ export class SinglePaneDockedTabsCoordinator extends Disposable {

private _queueCollapseIfDetailsOnly(): void {
if (!this._layoutService.isVisible(Parts.EDITOR_PART, mainWindow) && this._layoutService.isVisible(Parts.AUXILIARYBAR_PART)) {
void this._sequencer.queue(() => this._collapseNonManagedTabs()).catch(onUnexpectedError);
this._queue(() => this._collapseNonManagedTabs());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,38 @@ suite('LayoutController (desktop)', () => {
assert.deepStrictEqual(publishedWorkspaces, ['c']);
});

test('[managed tabs / dispose] a reconcile stalled mid-open opens no editors once the controller is disposed', async () => {
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
Outdated
const controller = createSinglePaneController({ activateAux: true });
await settle();

// Pause the reconcile at the first Changes open so it stalls before the Files tab opens.
let releaseChangesOpen!: () => void;
const changesOpenGate = new Promise<void>(resolve => { releaseChangesOpen = resolve; });
let gateArmed = true;
harness.onOpenChangesEditor = () => {
if (gateArmed) {
gateArmed = false;
return changesOpenGate;
}
return undefined;
};

// A created session with changes wants both a Changes and a Files tab; its reconcile
// stalls awaiting the gated Changes open.
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
Outdated
harness.activeSessionObs.set(makeSession(URI.parse('session:1'), { isCreated: true, changes: [makeChange('/file.ts')] }), undefined);
await settle();
assert.strictEqual(hasFilesTab(), false, 'reconcile should be stalled before opening the Files tab');

// 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.
controller.dispose();
releaseChangesOpen();
await settle();

assert.strictEqual(hasFilesTab(), false, 'a reconcile resumed after dispose must not open further editors');
});

test('[managed tabs / details-only] always restores both docked inputs while only details are visible', async () => {
createSinglePaneController({
activateAux: true,
Expand Down