fix(lsp): harden local provider lifecycle
This commit is contained in:
@@ -41,7 +41,7 @@ export class LspConnection {
|
||||
private readonly decoder: MessageDecoder
|
||||
private readonly pending = new Map<number, Pending>()
|
||||
private nextId = 1
|
||||
private stderr = ''
|
||||
private stderr = Buffer.alloc(0)
|
||||
private closeReason: Error | undefined
|
||||
/** Set once the process has fully exited; the instance awaits it during teardown. */
|
||||
readonly closed: Promise<void>
|
||||
@@ -90,7 +90,7 @@ export class LspConnection {
|
||||
|
||||
/** The retained stderr tail, for diagnostics on a failed server. */
|
||||
get stderrTail(): string {
|
||||
return this.stderr
|
||||
return this.stderr.toString('utf8')
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -199,7 +199,17 @@ export class LspConnection {
|
||||
private onStderr(chunk: Buffer): void {
|
||||
// Retain the TAIL, not the prefix: a language server's fatal diagnostic usually appears just
|
||||
// before it exits, so the final bounded segment is the useful one.
|
||||
this.stderr = (this.stderr + chunk.toString('utf8')).slice(-this.spec.maxStderrBytes)
|
||||
const cap = this.spec.maxStderrBytes
|
||||
if (chunk.length >= cap) {
|
||||
// Copy the bounded suffix so retaining it does not pin an arbitrarily large incoming buffer.
|
||||
this.stderr = Buffer.from(chunk.subarray(chunk.length - cap))
|
||||
return
|
||||
}
|
||||
const retainedBytes = Math.min(this.stderr.length, cap - chunk.length)
|
||||
this.stderr = Buffer.concat([
|
||||
this.stderr.subarray(this.stderr.length - retainedBytes),
|
||||
chunk,
|
||||
], retainedBytes + chunk.length)
|
||||
}
|
||||
|
||||
private dispatch(message: unknown): void {
|
||||
@@ -246,7 +256,7 @@ export class LspConnection {
|
||||
|
||||
/** The exit-close error message, appending the retained stderr tail when the server wrote any. */
|
||||
private exitMessage(): string {
|
||||
const tail = this.stderr.trim()
|
||||
const tail = this.stderrTail.trim()
|
||||
return tail === '' ? 'language server exited' : `language server exited; stderr: ${tail}`
|
||||
}
|
||||
|
||||
|
||||
@@ -64,6 +64,9 @@ export class MessageDecoder {
|
||||
}
|
||||
return { ready: false }
|
||||
}
|
||||
if (separator > MAX_HEADER_BYTES) {
|
||||
throw new Error(`LSP header exceeded ${MAX_HEADER_BYTES} bytes`)
|
||||
}
|
||||
const headerText = this.buffer.toString('ascii', 0, separator)
|
||||
const contentLength = parseContentLength(headerText)
|
||||
if (contentLength > this.maxMessageBytes) {
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
* @module @deepseek-ai/dsh-lsp-local
|
||||
*/
|
||||
|
||||
import { accessSync, constants } from 'node:fs'
|
||||
import { accessSync, constants, statSync } from 'node:fs'
|
||||
import { delimiter, isAbsolute, join } from 'node:path'
|
||||
import type { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
@@ -178,19 +178,23 @@ class LocalLspProvider implements LspProvider {
|
||||
// were canonicalizing/reading, so creating a server now would leave it unowned by teardown.
|
||||
/* v8 ignore next -- guards a dispose landing during the canonicalize/read await; not a reproducible unit race. */
|
||||
if (this.isDisposed()) throw new Error('lsp-local provider is disposed')
|
||||
const instance = await this.instanceFor(workspace)
|
||||
// Re-check cancellation too: an abort during the canonicalize/read awaits must not go on to spawn
|
||||
// (or pool) a server solely for an operation the caller already gave up on.
|
||||
if (signal?.aborted) throw abortError(signal)
|
||||
let instance = await this.instanceFor(workspace)
|
||||
// A pooled server that exited while idle resolves to a dead instance: evict it and create a fresh
|
||||
// one before dispatch, so this query does not have to fail on a closed connection first. One retry
|
||||
// suffices — the replacement was just constructed and has not been used.
|
||||
if (instance.dead) {
|
||||
await this.evictIfCurrent(workspace, instance)
|
||||
instance = await this.instanceFor(workspace)
|
||||
}
|
||||
try {
|
||||
return await instance.query(request, source, signal)
|
||||
} finally {
|
||||
// A crashed/closed process must not be reused: drop its slot so the next query starts fresh,
|
||||
// but only if the slot still holds THIS instance (a concurrent replacement must survive).
|
||||
if (instance.dead) {
|
||||
const slot = this.instances.get(workspace)
|
||||
/* v8 ignore next -- the slot-undefined arm needs a concurrent eviction of the same slot; defensive. */
|
||||
if (slot !== undefined && (await settledInstance(slot)) === instance) {
|
||||
this.instances.delete(workspace)
|
||||
}
|
||||
}
|
||||
if (instance.dead) await this.evictIfCurrent(workspace, instance)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -208,6 +212,15 @@ class LocalLspProvider implements LspProvider {
|
||||
return created
|
||||
}
|
||||
|
||||
/** Drop the slot for `workspace` iff it still resolves to `instance` (a concurrent replacement survives). */
|
||||
private async evictIfCurrent(workspace: string, instance: LspInstance): Promise<void> {
|
||||
const slot = this.instances.get(workspace)
|
||||
/* v8 ignore next -- the slot-undefined/mismatch arm needs a concurrent eviction of the same slot; defensive. */
|
||||
if (slot !== undefined && (await settledInstance(slot)) === instance) {
|
||||
this.instances.delete(workspace)
|
||||
}
|
||||
}
|
||||
|
||||
private createInstance(workspace: string): LspInstance {
|
||||
const spec: InstanceSpec = {
|
||||
command: this.executable,
|
||||
@@ -265,7 +278,7 @@ function buildChildEnv(extra: Record<string, string>): Record<string, string> {
|
||||
function resolveExecutable(command: string, childEnv: Record<string, string>): string {
|
||||
if (isAbsolute(command)) {
|
||||
// Verify an absolute command too, so an unavailable one fails at load, not on the first query.
|
||||
if (!isExecutableSync(command)) {
|
||||
if (!isExecutableFileSync(command)) {
|
||||
throw new Error(`lsp-local: command "${command}" is not an executable file`)
|
||||
}
|
||||
return command
|
||||
@@ -275,14 +288,15 @@ function resolveExecutable(command: string, childEnv: Record<string, string>): s
|
||||
for (const dir of pathValue.split(delimiter)) {
|
||||
if (dir === '') continue
|
||||
const candidate = join(dir, command)
|
||||
if (isExecutableSync(candidate)) return candidate
|
||||
if (isExecutableFileSync(candidate)) return candidate
|
||||
}
|
||||
throw new Error(`lsp-local: command "${command}" was not found on PATH`)
|
||||
}
|
||||
|
||||
/** Synchronous executable check used only at load-time resolution. */
|
||||
function isExecutableSync(path: string): boolean {
|
||||
/** Synchronous regular-file and executable check used only at load-time resolution. */
|
||||
function isExecutableFileSync(path: string): boolean {
|
||||
try {
|
||||
if (!statSync(path).isFile()) return false
|
||||
accessSync(path, constants.X_OK)
|
||||
return true
|
||||
} catch {
|
||||
|
||||
@@ -169,7 +169,7 @@ export class LspInstance {
|
||||
*/
|
||||
private abortable<T>(work: Promise<T>, signal: AbortSignal | undefined): Promise<T> {
|
||||
if (signal === undefined) return work
|
||||
/* v8 ignore next -- runQuery checks signal.aborted before each abortable() call, so it is not already aborted here; defensive. */
|
||||
/* v8 ignore next -- callers either pre-check the signal or pass a freshly armed teardown deadline. */
|
||||
if (signal.aborted) return Promise.reject(abortError(signal))
|
||||
return new Promise<T>((resolve, reject) => {
|
||||
const onAbort = (): void => { reject(abortError(signal)) }
|
||||
@@ -232,7 +232,10 @@ export class LspInstance {
|
||||
if (operation === 'hover') {
|
||||
return { kind: 'hover', hover: normalizeHover(payload) }
|
||||
}
|
||||
return { kind: 'locations', locations: normalizeLocations(payload) }
|
||||
// `spec.cwd` is the canonical workspace realpath (the provider canonicalizes before spawning),
|
||||
// and every `file:` location URI is relative to it — so it is the root a caller must relativize
|
||||
// display paths against, not the request's possibly-symlinked workspaceRoot.
|
||||
return { kind: 'locations', locations: normalizeLocations(payload), resolvedWorkspaceRoot: this.spec.cwd }
|
||||
}
|
||||
|
||||
private answerServerRequest(method: string, params: unknown): Promise<unknown> {
|
||||
@@ -271,24 +274,18 @@ export class LspInstance {
|
||||
try {
|
||||
using shutdownDeadline = deadline(undefined, this.spec.shutdownTimeoutMs, 'LSP_SHUTDOWN')
|
||||
await this.gracefulShutdown(shutdownDeadline.signal)
|
||||
return
|
||||
} catch {
|
||||
// Graceful shutdown failed or timed out: fall through to signal escalation.
|
||||
}
|
||||
await this.forceTerminate()
|
||||
}
|
||||
|
||||
/** Best-effort LSP `shutdown` request then `exit` notification, bounded by `signal`. */
|
||||
/** Best-effort LSP `shutdown`/`exit`, including process close, bounded by `signal`. */
|
||||
private async gracefulShutdown(signal: AbortSignal): Promise<void> {
|
||||
const shutdown = this.connection.request('shutdown', null)
|
||||
await Promise.race([
|
||||
shutdown,
|
||||
new Promise<never>((_, reject) => {
|
||||
/* v8 ignore next -- the shutdown deadline signal is freshly armed and not yet aborted here; defensive. */
|
||||
if (signal.aborted) { reject(abortError(signal)); return }
|
||||
signal.addEventListener('abort', () => { reject(abortError(signal)) }, { once: true })
|
||||
}),
|
||||
])
|
||||
await this.abortable(this.connection.request('shutdown', null), signal)
|
||||
this.connection.notify('exit', null)
|
||||
await this.abortable(this.connection.closed, signal)
|
||||
}
|
||||
|
||||
/** SIGTERM, wait `killGraceMs` for close, then SIGKILL; await full process close either way. */
|
||||
|
||||
Reference in New Issue
Block a user