fix(lsp): cancel executable setup on disposal
This commit is contained in:
@@ -117,21 +117,44 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
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> = []
|
||||
|
||||
@@ -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<AbortSignal>()
|
||||
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<string>((_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)
|
||||
|
||||
Reference in New Issue
Block a user