diff --git a/smoke/fixtures/server.cjs b/smoke/fixtures/server.cjs new file mode 100755 index 000000000..dbb70eb3a --- /dev/null +++ b/smoke/fixtures/server.cjs @@ -0,0 +1,49 @@ +#!/usr/bin/env node +const { spawn } = require("node:child_process"); +const { writeFileSync } = require("node:fs"); +const { join } = require("node:path"); +const home = process.env.OPENIDEA_SERVER_HOME; +const mode = process.env.SMOKE_SERVER_MODE; +const tool = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + detached: mode !== "hang-inherited", + stdio: "ignore", +}); +const second = + mode === "signal-multiple" + ? spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + detached: true, + stdio: "ignore", + }) + : undefined; +writeFileSync(join(home, "pids.json"), JSON.stringify({ server: process.pid, tool: tool.pid, second: second?.pid })); +let buffer = Buffer.alloc(0); +process.stdin.on("data", (chunk) => { + buffer = Buffer.concat([buffer, chunk]); + for (;;) { + const end = buffer.indexOf("\r\n\r\n"); + if (end < 0) return; + const length = Number(/Content-Length: (\d+)/i.exec(buffer.subarray(0, end).toString())[1]); + if (buffer.length < end + 4 + length) return; + const request = JSON.parse(buffer.subarray(end + 4, end + 4 + length)); + buffer = buffer.subarray(end + 4 + length); + const body = JSON.stringify({ + jsonrpc: "2.0", + id: request.id, + ...(mode === "reject" ? { error: { message: "initialize failed" } } : { result: {} }), + }); + process.stdout.write(`Content-Length: ${Buffer.byteLength(body)}\r\n\r\n${body}`); + } +}); +process.stdin.on("end", () => { + writeFileSync(join(home, "eof"), "received"); + if (mode.startsWith("hang")) return; + if (mode.startsWith("signal")) return process.kill(process.pid, "SIGTERM"); + setTimeout(() => { + tool.once("exit", () => { + writeFileSync(join(home, "shutdown"), "tool reaped"); + process.exit(0); + }); + tool.kill("SIGKILL"); + }, 150); +}); +setInterval(() => {}, 1000); diff --git a/smoke/server.test.ts b/smoke/server.test.ts new file mode 100644 index 000000000..c7344639d --- /dev/null +++ b/smoke/server.test.ts @@ -0,0 +1,152 @@ +// @vitest-environment node +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, existsSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test, onTestFinished, vi } from "vitest"; +import { spawn } from "node:child_process"; +import { once } from "node:events"; +import { Server, repoRoot } from "./server.ts"; + +// The executable fixture uses a POSIX shebang. Windows teardown uses taskkill. +const options = { skip: process.platform === "win32", timeout: 30_000 }; + +async function fixture(mode = "graceful") { + const home = mkdtempSync(join(tmpdir(), "openidea-server-close-")); + const previous = process.env.OPENIDEA_SERVER; + process.env.OPENIDEA_SERVER = join(repoRoot, "smoke", "fixtures", "server.cjs"); + onTestFinished(async () => { + if (previous === undefined) delete process.env.OPENIDEA_SERVER; + else process.env.OPENIDEA_SERVER = previous; + if (existsSync(join(home, "pids.json"))) { + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + for (const pid of Object.values(pids) as number[]) { + for (const target of [pid]) { + try { + process.kill(target, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } + } + // Reap/observe every owned process before removing its home, even on failure. + await vi.waitFor( + () => { + for (const pid of Object.values(pids) as number[]) assertGone(pid); + }, + { timeout: 3000 }, + ); + } + rmSync(home, { recursive: true, force: true }); + }); + return { home, start: () => Server.start(home, { SMOKE_SERVER_MODE: mode }) }; +} + +function assertGone(pid: number) { + assert.throws(() => process.kill(pid, 0), { code: "ESRCH" }); +} + +test("close waits for graceful EOF cleanup of a tool in its own group", options, async () => { + const { home, start } = await fixture(); + const server = await start(); + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + assert.deepEqual(await Promise.all([server.invoke("first", {}), server.invoke("second", {})]), [{}, {}]); + await Promise.all([server.close(), server.close()]); + assert.equal(readFileSync(join(home, "shutdown"), "utf8"), "tool reaped"); + assertGone(pids.server); + assertGone(pids.tool); + await server.close(); +}); + +test("failed initialize still runs graceful cleanup and preserves the error", options, async () => { + const { home, start } = await fixture("reject"); + await assert.rejects(start(), { message: "initialize failed" }); + assert.equal(readFileSync(join(home, "shutdown"), "utf8"), "tool reaped"); +}); + +test("hung EOF shutdown is bounded and kills independent tool groups", options, async () => { + const { home, start } = await fixture("hang"); + const server = await start(); + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + const started = Date.now(); + await server.close(); + assert.ok(Date.now() - started >= 15_000); + assert.ok(Date.now() - started < 25_000); + assert.ok(existsSync(join(home, "eof"))); + assertGone(pids.server); + assertGone(pids.tool); +}); + +test("signal exit after EOF also cleans up independent tool groups", options, async () => { + const { home, start } = await fixture("signal"); + const server = await start(); + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + await server.close(); + assertGone(pids.server); + assertGone(pids.tool); +}); + +test("spawn failure rejects without waiting for an exit event", options, async () => { + const { home } = await fixture(); + process.env.OPENIDEA_SERVER = join(home, "missing-server"); + await assert.rejects(Server.start(home), { code: "ENOENT" }); +}); + +test("missing ps preserves forced cleanup of the server's inherited group", options, async () => { + const { home, start } = await fixture("hang-inherited"); + const server = await start(); + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + const previous = process.env.PATH; + process.env.PATH = home; + try { + await server.close(); + } finally { + if (previous === undefined) delete process.env.PATH; + else process.env.PATH = previous; + } + assertGone(pids.server); + assertGone(pids.tool); +}); + +test("fixture consumes multiple requests including coalesced frames", options, async () => { + const { home } = await fixture(); + const child = spawn(process.env.OPENIDEA_SERVER!, [], { + env: { ...process.env, OPENIDEA_SERVER_HOME: home, SMOKE_SERVER_MODE: "graceful" }, + detached: true, + }); + const exited = once(child, "exit"); + let output = ""; + child.stdout.on("data", (chunk) => { + output += chunk.toString(); + }); + const frame = (id: number) => { + const body = JSON.stringify({ jsonrpc: "2.0", id, method: "initialize" }); + return `Content-Length: ${Buffer.byteLength(body)}\r\n\r\n${body}`; + }; + // A single write contains two frames and part of the third header. + const third = frame(3); + child.stdin.write(frame(1) + frame(2) + third.slice(0, 10)); + await new Promise((resolve) => setTimeout(resolve, 50)); + child.stdin.end(third.slice(10)); + await exited; + const ids = [...output.matchAll(/"id":(\d+)/g)].map((match) => Number(match[1])); + assert.deepEqual(ids, [1, 2, 3]); +}); + +test("group-kill errors are reported after cleaning the remaining owned groups", options, async () => { + const { home, start } = await fixture("signal-multiple"); + const server = await start(); + const pids = JSON.parse(readFileSync(join(home, "pids.json"), "utf8")); + const kill = process.kill.bind(process); + const denied = Object.assign(new Error("group kill denied"), { code: "EPERM" }); + const spy = vi.spyOn(process, "kill").mockImplementation((pid, signal) => { + if (pid === -pids.tool && signal === "SIGKILL") throw denied; + return kill(pid, signal); + }); + try { + await assert.rejects(server.close(), denied); + assertGone(pids.second); + } finally { + spy.mockRestore(); + } +}); diff --git a/smoke/server.ts b/smoke/server.ts index aa867c127..9a7e66ed9 100644 --- a/smoke/server.ts +++ b/smoke/server.ts @@ -8,6 +8,8 @@ import { dirname, join } from "node:path"; const PROTOCOL_VERSION = 2; const WINDOW = "main"; +// Allow the server's ten-second host shutdown plus scheduling and I/O overhead. +const SHUTDOWN_TIMEOUT_MS = 15_000; // Not `new URL(…, import.meta.url)`: the jsdom environment's `URL` isn't Node's. export const repoRoot = join(dirname(import.meta.filename), ".."); @@ -27,24 +29,87 @@ interface Pending { function treeKill(pid: number | undefined): Promise { return new Promise((resolve) => { if (pid === undefined) return resolve(); - execFile("taskkill", ["/pid", String(pid), "/T", "/F"], () => resolve()); + execFile("taskkill", ["/pid", String(pid), "/T", "/F"], { timeout: 5000 }, () => resolve()); }); } -/** Resolves once no process group `pid` exists anymore (signal 0 only checks). */ +/** Poll for group/leader disappearance until timeoutMs elapses. */ async function waitForGroupExit(pid: number, timeoutMs = 5000): Promise { const start = Date.now(); for (;;) { - try { - process.kill(-pid, 0); - } catch { - return; - } - if (Date.now() - start > timeoutMs) return; + const alive = [pid, -pid].some((target) => { + try { + process.kill(target, 0); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + return false; + } + }); + if (!alive || Date.now() - start > timeoutMs) return; await new Promise((r) => setTimeout(r, 50)); } } +interface ProcessInfo { + pid: number; + parent: number; + group: number; + started: string; +} + +function processTable(): Promise { + return new Promise((resolve) => { + execFile("ps", ["-axo", "pid=,ppid=,pgid=,lstart="], { timeout: 1000 }, (error, stdout) => { + if (error) return resolve([]); + const rows: ProcessInfo[] = []; + for (const line of stdout.split("\n")) { + const match = /^\s*(\d+)\s+(\d+)\s+(\d+)\s+(.+)$/.exec(line); + if (match) + rows.push({ pid: Number(match[1]), parent: Number(match[2]), group: Number(match[3]), started: match[4]! }); + } + resolve(rows); + }); + }); +} + +/** Snapshot owned group members before EOF can orphan them. */ +async function ownedProcesses(pid: number): Promise { + const rows = await processTable(); + const descendants = new Set([pid]); + for (;;) { + const size = descendants.size; + for (const row of rows) { + if (descendants.has(row.parent)) descendants.add(row.pid); + } + if (size === descendants.size) break; + } + return rows.filter((row) => descendants.has(row.pid) && descendants.has(row.group)); +} + +/** Revalidate identities so a group id reused during shutdown is never signalled. */ +async function killOwnedGroups(owned: ProcessInfo[], errors: Set): Promise { + const current = await processTable(); + const identities = new Map(owned.map((row) => [row.pid, row])); + const groups = new Set(); + for (const row of current) { + const old = identities.get(row.pid); + if (old?.group === row.group && old.started === row.started) { + groups.add(row.group); + } + } + const killed: number[] = []; + for (const group of groups) { + try { + process.kill(-group, "SIGKILL"); + killed.push(group); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") errors.add(error); + } + } + return killed; +} + export class Server { private nextId = 1; private readonly pending = new Map(); @@ -55,7 +120,11 @@ export class Server { /** `event` notifications: an `IdeEvent` for a window (`null`: every window). */ onEvent: (window: string | null, event: unknown) => void = () => {}; - private constructor(private readonly child: ChildProcessWithoutNullStreams) { + private closing: Promise | undefined; + private readonly child: ChildProcessWithoutNullStreams; + + private constructor(child: ChildProcessWithoutNullStreams) { + this.child = child; child.stdout.on("data", (chunk: Buffer) => this.receive(chunk)); child.stderr.on("data", (chunk: Buffer) => { // The whole log, when asked for (OPENIDEA_SMOKE_LOG=; OPENIDEA_LOG sets its level). @@ -63,6 +132,15 @@ export class Server { this.stderr.push(chunk.toString()); if (this.stderr.length > 200) this.stderr.shift(); }); + child.on("error", (error) => { + for (const p of this.pending.values()) p.reject(error); + this.pending.clear(); + }); + // A process exiting during a request can close its stdin before the exit event. + child.stdin.on("error", (error) => { + for (const p of this.pending.values()) p.reject(error); + this.pending.clear(); + }); child.on("exit", (code) => { for (const p of this.pending.values()) p.reject(new Error(`openidea-server exited (${code}): ${this.log()}`)); this.pending.clear(); @@ -74,13 +152,17 @@ export class Server { const child = spawn(serverBinary(), ["--stdio"], { env: { ...process.env, OPENIDEA_SERVER_HOME: home, ...env }, stdio: ["pipe", "pipe", "pipe"], - // Its own process group: the language servers it starts (jdtls's JVM) stay in - // it, so `close` can reap them together with the server (see below). + // A dedicated group for forced cleanup; LSPs may create groups of their own. detached: true, }); const server = new Server(child); - await server.request("initialize", { version: PROTOCOL_VERSION }); - return server; + try { + await server.request("initialize", { version: PROTOCOL_VERSION }); + return server; + } catch (error) { + await server.close(); + throw error; + } } /** The tail of the server's log, for failure messages. */ @@ -103,36 +185,76 @@ export class Server { } } - /** - * Kills the server and everything it started; resolves once they're all gone. - * The server alone is not enough: a language server's own process (jdtls's JVM) - * outlives it and keeps writing under the test's home directory, so deleting that - * directory afterwards raced its writes (`ENOTEMPTY` on teardown). The server runs - * detached (its own process group), so the group is only it and its children. - */ - async close(): Promise { - if (this.child.exitCode === null && !this.child.killed) { + /** Let EOF shut down and reap tools before falling back to forced cleanup. */ + close(): Promise { + return (this.closing ??= this.shutdown()); + } + + private async shutdown(): Promise { + const pid = this.child.pid; + const owned = process.platform !== "win32" && pid !== undefined ? await ownedProcesses(pid) : []; + const killedGroups = new Set(); + const errors = new Set(); + const running = () => pid !== undefined && this.child.exitCode === null && this.child.signalCode === null; + if (running()) { + let timer: ReturnType | undefined; const exited = new Promise((resolve) => this.child.once("exit", () => resolve())); this.child.stdin.end(); - if (process.platform === "win32") { - // `child.kill()` stops only the server; take the whole tree while it is - // still enumerable, then wait for the exit below. - await treeKill(this.child.pid); - } else { - this.child.kill(); - } - await exited; - } - if (process.platform !== "win32" && this.child.pid !== undefined) { - // Reap what the server started: `SIGKILL` to the group, then wait until it is - // gone (termination is asynchronous; files held open can't be deleted meanwhile). try { - process.kill(-this.child.pid, "SIGKILL"); - } catch { - // Nothing left running. + await Promise.race([ + exited, + new Promise((resolve) => { + timer = setTimeout(resolve, SHUTDOWN_TIMEOUT_MS); + }), + ]); + } finally { + clearTimeout(timer); + } + if (running()) { + if (process.platform === "win32") { + // Enumerate the tree while its root is still alive. + await treeKill(pid); + } else { + if (pid !== undefined) owned.push(...(await ownedProcesses(pid))); + for (const group of await killOwnedGroups(owned, errors)) killedGroups.add(group); + } + // While the handle is running, its dedicated group cannot have been reused. + // Preserve inherited-group cleanup even when ps is unavailable. + if (running()) { + if (process.platform !== "win32" && pid !== undefined && !killedGroups.has(pid)) { + try { + process.kill(-pid, "SIGKILL"); + killedGroups.add(pid); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") errors.add(error); + } + } + try { + this.child.kill("SIGKILL"); + } catch (error) { + errors.add(error); + } + } + try { + await Promise.race([ + exited, + new Promise((resolve) => { + timer = setTimeout(resolve, 5000); + }), + ]); + if (running()) errors.add(new Error(`openidea-server (${pid}) did not exit after forced cleanup`)); + } finally { + clearTimeout(timer); + } } - await waitForGroupExit(this.child.pid); } + if (process.platform !== "win32") { + for (const group of await killOwnedGroups(owned, errors)) killedGroups.add(group); + } + const waits = await Promise.allSettled([...killedGroups].map((group) => waitForGroupExit(group))); + for (const result of waits) if (result.status === "rejected") errors.add(result.reason); + if (errors.size === 1) throw [...errors][0]; + if (errors.size > 1) throw new AggregateError(errors, "Smoke server cleanup failed"); } private request(method: string, params: unknown): Promise {