From 611209ffddb5d263785b93c051727ee8bef84b9c Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Wed, 11 Nov 2026 16:38:43 -0800 Subject: [PATCH] fix: prevent formatter registration after context disposal Guard late restart failures, missing-interpreter shutdown, and per-project initialization against restoring the formatter placeholder after disposal. Preserve fallback recovery for live contexts and cover all paths with deferred regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 85ef72cc-9e26-4bc4-b1c3-55256d9bef91 --- typescript/src/activation.ts | 19 ++- typescript/tests/activation.test.ts | 175 +++++++++++++++++++++++++++- 2 files changed, 186 insertions(+), 8 deletions(-) diff --git a/typescript/src/activation.ts b/typescript/src/activation.ts index bd522ab..1091c79 100644 --- a/typescript/src/activation.ts +++ b/typescript/src/activation.ts @@ -123,10 +123,12 @@ export interface ToolExtensionContext { initialize(subscriptions: vscode.Disposable[]): Promise; /** - * Dispose all internal resources (timers, state listeners). + * Dispose all internal resources (timers, state listeners, formatter placeholder). * * Call from the extension's `deactivate()` to prevent orphaned * server processes from debounce timers firing after shutdown. + * In-flight restarts cannot restore the formatter placeholder, but + * disposal does not wait for them to finish. */ dispose(): void; } @@ -174,6 +176,9 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens } if (usePerProjectEnvironments(toolConfig) && extensionSubscriptions && !pythonInitialized) { await initializePython(extensionSubscriptions, false); + if (disposed) { + return; + } } if (isRestarting) { if (restartTimer) { @@ -256,7 +261,9 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens // Re-register the placeholder so the extension stays // visible in the formatter picker while there is no // interpreter (and therefore no running LSP formatter). - nullFormatter?.register(); + if (!disposed) { + nullFormatter?.register(); + } updateStatus( vscode.l10n.t('Please select a Python interpreter.'), @@ -350,10 +357,10 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens } } catch (ex) { traceError(`Server restart failed: ${ex}`); - // Ensure placeholder stays visible after a failure — but only - // when there is no healthy running client (same guard as the - // top of runServer to avoid the duplicate-formatter symptom). - if (nullFormatter && (!ctx.lsClient || ctx.lsClient.state !== State.Running)) { + // Ensure placeholder stays visible after a failure on a live + // context without a healthy running client (same state guard + // as the top of runServer to avoid duplicate formatters). + if (!disposed && nullFormatter && (!ctx.lsClient || ctx.lsClient.state !== State.Running)) { nullFormatter.register(); } } finally { diff --git a/typescript/tests/activation.test.ts b/typescript/tests/activation.test.ts index 76fdb9c..8525aa0 100644 --- a/typescript/tests/activation.test.ts +++ b/typescript/tests/activation.test.ts @@ -16,6 +16,7 @@ import { ToolExtensionContext, } from '../src/activation'; import * as configWatcher from '../src/configWatcher'; +import * as loggingModule from '../src/logging'; import * as settingsModule from '../src/settings'; import * as serverModule from '../src/server'; import * as statusModule from '../src/status'; @@ -30,10 +31,12 @@ import * as vscodeapi from '../src/vscodeapi'; function deferred() { let resolve!: (value: T) => void; - const promise = new Promise((res) => { + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { resolve = res; + reject = rej; }); - return { promise, resolve }; + return { promise, resolve, reject }; } function makeToolConfig(overrides?: Partial): ToolConfig { @@ -995,6 +998,174 @@ suite('createToolContext – NullFormatter lifecycle', () => { assert.isTrue(providerDispose.calledOnce, 'placeholder disposed on ctx.dispose()'); }); + test('does not restore a formatter placeholder when an in-flight restart rejects after disposal', async () => { + const entered = deferred(); + const finished = deferred(); + const error = new Error('Debugger path lookup failed'); + const traceError = sandbox.spy(loggingModule, 'traceError'); + const restartStub = sandbox.stub(serverModule, 'restartServer').callsFake(() => { + entered.resolve(); + return finished.promise; + }); + const liveProviders = new Set(); + registerFormattingProviderStub.callsFake(() => { + const provider = { + dispose: sandbox.stub().callsFake(() => { + liveProviders.delete(provider); + }), + }; + liveProviders.add(provider); + return provider; + }); + const ctx = createToolContext(makeFormatterOptions()); + const running = ctx.runServer(); + + try { + await entered.promise; + sinon.assert.calledOnce(restartStub); + sinon.assert.calledOnce(registerFormattingProviderStub); + assert.strictEqual(liveProviders.size, 1, 'the activation placeholder is live'); + + ctx.dispose(); + sinon.assert.calledOnce(registerFormattingProviderStub.firstCall.returnValue.dispose); + assert.strictEqual(liveProviders.size, 0, 'dispose removes the existing placeholder'); + + finished.reject(error); + await running; + + sinon.assert.calledWithExactly(traceError, `Server restart failed: ${error}`); + assert.deepEqual( + { registrations: registerFormattingProviderStub.callCount, liveProviders: liveProviders.size }, + { registrations: 1, liveProviders: 0 }, + 'a rejected restart must not resurrect the disposed formatter context', + ); + } finally { + ctx.dispose(); + finished.reject(error); + await running; + } + }); + + test('restores formatter fallback when an in-flight restart rejects on a live context', async () => { + const mockClient = makeMockClient(State.Running); + const entered = deferred(); + const finished = deferred(); + const error = new Error('Debugger path lookup failed'); + const traceError = sandbox.spy(loggingModule, 'traceError'); + const restartStub = sandbox.stub(serverModule, 'restartServer'); + restartStub.onFirstCall().resolves({ client: asLC(mockClient), disposables: [] }); + restartStub.onSecondCall().callsFake(() => { + entered.resolve(); + return finished.promise; + }); + const ctx = createToolContext(makeFormatterOptions()); + let running: Promise | undefined; + + try { + await ctx.runServer(); + sinon.assert.calledOnce(providerDispose); + + running = ctx.runServer(); + await entered.promise; + mockClient.simulateStateChange(State.Stopped); + sinon.assert.calledOnce(registerFormattingProviderStub); + + finished.reject(error); + await running; + + sinon.assert.calledWithExactly(traceError, `Server restart failed: ${error}`); + sinon.assert.calledTwice(registerFormattingProviderStub); + sinon.assert.calledOnce(providerDispose); + } finally { + ctx.dispose(); + finished.reject(error); + await running; + mockClient.dispose(); + } + }); + + test('does not restore a formatter placeholder when a missing-interpreter stop finishes after disposal', async () => { + const mockClient = makeMockClient(State.Running); + sandbox.stub(serverModule, 'restartServer').resolves({ client: asLC(mockClient), disposables: [] }); + const entered = deferred(); + const finished = deferred(); + const stop = sandbox.stub(mockClient, 'stop').callsFake(async () => { + entered.resolve(); + await finished.promise; + mockClient.simulateStateChange(State.Stopped); + }); + const ctx = createToolContext(makeFormatterOptions()); + let running: Promise | undefined; + + try { + await ctx.runServer(); + sinon.assert.calledOnce(providerDispose); + (settingsModule.getWorkspaceSettings as sinon.SinonStub).resolves({ + cwd: '/workspace', + workspace: 'file:///workspace', + args: [], + path: [], + interpreter: [], + importStrategy: 'useBundled', + showNotifications: 'off', + }); + + running = ctx.runServer(); + await entered.promise; + sinon.assert.calledOnce(stop); + ctx.dispose(); + finished.resolve(); + await running; + + sinon.assert.calledOnce(registerFormattingProviderStub); + sinon.assert.calledOnce(providerDispose); + } finally { + ctx.dispose(); + finished.resolve(); + await running; + mockClient.dispose(); + } + }); + + test('does not restore a formatter placeholder when per-project initialization finishes after disposal', async () => { + let enabled = false; + sandbox.stub(vscodeapi, 'getConfiguration').returns({ + get: (key: string, defaultValue?: unknown) => + key === 'usePerProjectEnvironments' ? enabled : defaultValue, + } as unknown as vscode.WorkspaceConfiguration); + const restartStub = sandbox.stub(serverModule, 'restartServer').resolves({ client: undefined, disposables: [] }); + const entered = deferred(); + const finished = deferred(); + const options = makeFormatterOptions(); + options.toolConfig = { ...options.toolConfig, supportsPerProjectEnvironments: true }; + (options.pythonProvider.initializePython as sinon.SinonStub).callsFake(() => { + entered.resolve(); + return finished.promise; + }); + const ctx = createToolContext(options); + let running: Promise | undefined; + + try { + await ctx.initialize([]); + enabled = true; + running = ctx.runServer(); + await entered.promise; + ctx.dispose(); + sinon.assert.calledOnce(providerDispose); + + finished.resolve(); + await running; + + sinon.assert.calledOnce(registerFormattingProviderStub); + sinon.assert.calledOnce(providerDispose); + sinon.assert.calledOnce(restartStub); + } finally { + ctx.dispose(); + finished.resolve(); + await running; + } + }); + test('holds formatter fallback until replacement completes and ignores stale client events', async () => { const firstClient = makeMockClient(State.Stopped); const secondClient = makeMockClient(State.Stopped);