From 6e5d711099a1db4f14bf897a9416c705d9fd2d4f Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:59:55 +0800 Subject: [PATCH] fix(lsp): cancel executable setup on disposal --- packages/lsp/lsp-local/src/index.ts | 49 ++++++++++++++----- packages/lsp/lsp-local/tests/provider.spec.ts | 37 +++++++++++++- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index d12e8986fc..0d511588bd 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -117,21 +117,44 @@ export async function apply(ctx: Context, config: Config): Promise { const entries = Object.entries(config.servers) if (entries.length === 0) throw new Error('lsp-local: servers must contain at least one server') + const setupAbort = new AbortController() + const stopSetupCancellation = ctx.on('internal/plugin', (fiber) => { + // An async plugin callback must observe its own disposal before Cordis can + // run effect cleanup, because unload otherwise waits for this callback. + if (fiber === ctx.fiber && fiber.uid === null) { + setupAbort.abort(new Error('lsp-local setup disposed')) + } + }) + // Resolve every server-local setting before registration so a bad later command or bound cannot // publish an earlier provider. Registry-level mapping conflicts are rolled back below. - const providers = await Promise.all(entries.map(async ([providerId, rawConfig]) => { - if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings') - const resolved = rawConfig as ResolvedServerConfig - validateServerConfig(providerId, resolved) - const executable = await ctx.subprocess.resolveExecutable(resolved.command, resolved.env) - return new LocalLspProvider( - providerId, - ctx.fs, - resolved, - executable, - spec => ctx.subprocess.spawn(spec), - ) - })) + const providers = await (async () => { + try { + return await Promise.all(entries.map(async ([providerId, rawConfig]) => { + if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings') + const resolved = rawConfig as ResolvedServerConfig + validateServerConfig(providerId, resolved) + const executable = await ctx.subprocess.resolveExecutable( + resolved.command, + resolved.env, + setupAbort.signal, + ) + setupAbort.signal.throwIfAborted() + return new LocalLspProvider( + providerId, + ctx.fs, + resolved, + executable, + spec => ctx.subprocess.spawn(spec), + ) + })) + } catch (error: unknown) { + setupAbort.abort(error) + throw error + } finally { + stopSetupCancellation() + } + })() ctx.effect(() => { const disposers: Array<() => void> = [] diff --git a/packages/lsp/lsp-local/tests/provider.spec.ts b/packages/lsp/lsp-local/tests/provider.spec.ts index b0209e09b8..8d8a7ad356 100644 --- a/packages/lsp/lsp-local/tests/provider.spec.ts +++ b/packages/lsp/lsp-local/tests/provider.spec.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { chmod, mkdtemp, mkdir, rm, writeFile, realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' import { delimiter, join } from 'node:path' @@ -195,6 +195,41 @@ describe('lsp-local provider resolution', () => { await ctx.fiber.dispose() }) + it('aborts executable resolution when disposed during setup', async () => { + const ctx = new Context() + await ctx.plugin(Lsp) + await ctx.plugin(LocalSubprocessService) + await ctx.plugin(LocalFileSystem, { cwd: process.cwd() }) + const subprocess = ctx.subprocess + const lookupStarted = Promise.withResolvers() + vi.spyOn(subprocess, 'resolveExecutable').mockImplementation(async (_command, _env, signal) => { + if (signal === undefined) throw new Error('missing setup signal') + lookupStarted.resolve(signal) + return await new Promise((_resolve, reject) => { + const onAbort = (): void => { + reject(signal.reason instanceof Error ? signal.reason : new Error(String(signal.reason))) + } + signal.addEventListener('abort', onAbort, { once: true }) + if (signal.aborted) onAbort() + }) + }) + + const loading = ctx.plugin(LspLocal, config('pending', { + command: 'pending-lsp', + extensionToLanguage: { '.ts': 'typescript' }, + })) + const signal = await lookupStarted.promise + const unrelated = await ctx.plugin(() => {}) + await unrelated.dispose() + expect(signal.aborted).toBe(false) + const disposing = loading.dispose() + + await expect(loading).rejects.toThrow('lsp-local setup disposed') + await expect(disposing).resolves.toBeUndefined() + expect(signal.aborted).toBe(true) + await ctx.fiber.dispose() + }) + it('rolls back earlier registrations when a later server conflicts', async () => { const ctx = new Context() await ctx.plugin(Lsp)