fix(memory-host): kill qmd process groups

This commit is contained in:
Vincent Koc 2026-06-17 22:33:10 +02:00
parent 2282fcd9ec
commit 20534c57b7
No known key found for this signature in database
2 changed files with 107 additions and 6 deletions

View file

@ -3,7 +3,17 @@ import { EventEmitter } from "node:events";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import {
afterAll,
afterEach,
beforeAll,
beforeEach,
describe,
expect,
it,
vi,
type MockInstance,
} from "vitest";
import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../../../gateway-client/src/timeouts.js";
const spawnMock = vi.hoisted(() => vi.fn());
@ -24,13 +34,15 @@ import {
type QmdBinaryAvailability,
} from "./qmd-process.js";
function createMockChild() {
function createMockChild(params: { pid?: number } = {}) {
const child = new EventEmitter() as EventEmitter & {
pid?: number;
stdout: EventEmitter;
stderr: EventEmitter;
kill: ReturnType<typeof vi.fn>;
closeWith: (code?: number | null, signal?: NodeJS.Signals | null) => void;
};
child.pid = params.pid;
child.stdout = new EventEmitter();
child.stderr = new EventEmitter();
child.kill = vi.fn();
@ -42,7 +54,7 @@ function createMockChild() {
let fixtureRoot = "";
let tempDir = "";
let platformSpy: { mockRestore(): void } | null = null;
let platformSpy: MockInstance<() => NodeJS.Platform> | null = null;
let fixtureId = 0;
const originalPath = process.env.PATH;
const originalPathExt = process.env.PATHEXT;
@ -69,6 +81,7 @@ afterEach(() => {
vi.useRealTimers();
process.env.PATH = originalPath;
process.env.PATHEXT = originalPathExt;
platformSpy?.mockReturnValue("win32");
spawnMock.mockReset();
tempDir = "";
});
@ -218,6 +231,34 @@ describe("checkQmdBinaryAvailability", () => {
expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_SAFE_TIMEOUT_DELAY_MS);
});
it("kills timed-out availability probes by process group on POSIX", async () => {
platformSpy?.mockReturnValue("linux");
const killProcess = vi.spyOn(process, "kill").mockImplementation(() => true);
const child = createMockChild({ pid: 4321 });
spawnMock.mockReturnValueOnce(child);
try {
await expect(
checkQmdBinaryAvailability({
command: "qmd",
env: process.env,
cwd: tempDir,
timeoutMs: 1,
}),
).resolves.toEqual({
available: false,
reason: "binary",
error: "spawn qmd timed out after 1ms",
});
expect(spawnMock.mock.calls[0]?.[2]).toMatchObject({ detached: true });
expect(killProcess).toHaveBeenCalledWith(-4321, "SIGKILL");
expect(child.kill).not.toHaveBeenCalledWith("SIGKILL");
} finally {
killProcess.mockRestore();
}
});
});
describe("runCliCommand", () => {
@ -341,4 +382,33 @@ describe("runCliCommand", () => {
expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_SAFE_TIMEOUT_DELAY_MS);
});
it("kills timed-out cli command process groups on POSIX", async () => {
platformSpy?.mockReturnValue("linux");
const killProcess = vi.spyOn(process, "kill").mockImplementation(() => true);
const child = createMockChild({ pid: 8765 });
spawnMock.mockReturnValueOnce(child);
try {
const pending = runCliCommand({
commandSummary: "qmd query test",
spawnInvocation: { command: "qmd", argv: ["query", "test", "--json"] },
env: process.env,
cwd: tempDir,
maxOutputChars: 10_000,
timeoutMs: 1,
});
const timeoutAssertion = expect(pending).rejects.toThrow(
"qmd query test timed out after 1ms",
);
await timeoutAssertion;
expect(spawnMock.mock.calls[0]?.[2]).toMatchObject({ detached: true });
expect(killProcess).toHaveBeenCalledWith(-8765, "SIGKILL");
expect(child.kill).not.toHaveBeenCalledWith("SIGKILL");
} finally {
killProcess.mockRestore();
}
});
});

View file

@ -11,6 +11,11 @@ export type CliSpawnInvocation = {
windowsHide?: boolean;
};
type QmdChildProcess = {
pid?: number;
kill: (signal?: NodeJS.Signals) => boolean;
};
export type QmdBinaryUnavailableReason = "binary" | "workspace-cwd";
export type QmdBinaryUnavailable = {
@ -92,10 +97,11 @@ export async function checkQmdBinaryAvailability(params: {
shell: spawnInvocation.shell,
windowsHide: spawnInvocation.windowsHide,
stdio: "ignore",
detached: shouldUseQmdProcessGroup(),
});
const timeoutMs = resolveSafeTimeoutDelayMs(params.timeoutMs ?? 2_000, { minMs: 0 });
const timer = setTimeout(() => {
child.kill("SIGKILL");
signalQmdProcessTree(child, "SIGKILL");
finish({
available: false,
reason: "binary",
@ -108,7 +114,7 @@ export async function checkQmdBinaryAvailability(params: {
});
child.once("spawn", () => {
didSpawn = true;
child.kill();
signalQmdProcessTree(child);
finish({ available: true });
});
child.once("close", () => {
@ -162,6 +168,7 @@ export async function runCliCommand(params: {
cwd: params.cwd,
shell: params.spawnInvocation.shell,
windowsHide: params.spawnInvocation.windowsHide,
detached: shouldUseQmdProcessGroup(),
});
let stdout = "";
let stderr = "";
@ -172,7 +179,7 @@ export async function runCliCommand(params: {
params.timeoutMs === undefined ? undefined : resolveSafeTimeoutDelayMs(params.timeoutMs);
const timer = timeoutMs
? setTimeout(() => {
child.kill("SIGKILL");
signalQmdProcessTree(child, "SIGKILL");
reject(new Error(`${params.commandSummary} timed out after ${timeoutMs}ms`));
}, timeoutMs)
: null;
@ -224,6 +231,30 @@ export async function runCliCommand(params: {
});
}
function shouldUseQmdProcessGroup(): boolean {
return process.platform !== "win32";
}
function signalQmdProcessTree(child: QmdChildProcess, signal?: NodeJS.Signals): void {
if (shouldUseQmdProcessGroup() && typeof child.pid === "number") {
try {
if (signal === undefined) {
process.kill(-child.pid);
} else {
process.kill(-child.pid, signal);
}
return;
} catch {
// Fall back to the direct child if the process group already disappeared.
}
}
if (signal === undefined) {
child.kill();
} else {
child.kill(signal);
}
}
class CliCommandError extends Error {
readonly code: number | null;
readonly signal: NodeJS.Signals | null;