fix(rpc): kill measurement gateway trees on windows

This commit is contained in:
Vincent Koc
2026-06-20 13:10:59 +02:00
parent 8ee638236a
commit f719f0cf77
2 changed files with 144 additions and 37 deletions
+66 -25
View File
@@ -1,6 +1,6 @@
// Measures gateway RPC round-trip time by launching an isolated local gateway
// and writing qa-lab-compatible summary artifacts.
import { spawn } from "node:child_process";
import { spawn, spawnSync } from "node:child_process";
import { randomUUID } from "node:crypto";
import { existsSync } from "node:fs";
import fs from "node:fs/promises";
@@ -264,6 +264,10 @@ function defaultKillProcess(pid, signal) {
return process.kill(pid, signal);
}
function defaultRunTaskkill(command, args, options) {
return spawnSync(command, args, options);
}
async function defaultOpen(filePath, flags) {
return await fs.open(filePath, flags);
}
@@ -277,10 +281,15 @@ function resolveOpenClawLaunchArgs(repoRoot, sourceEntryExists = existsSync) {
}
/**
* Signals the gateway process group on POSIX so spawned children are cleaned up.
* Signals the gateway process tree so spawned package-manager children are cleaned up.
*/
export function signalGatewayProcess(child, signal, killProcess = defaultKillProcess) {
if (process.platform !== "win32" && typeof child.pid === "number") {
export function signalGatewayProcess(
child,
signal,
killProcess = defaultKillProcess,
{ platform = process.platform, runTaskkill = defaultRunTaskkill } = {},
) {
if (platform !== "win32" && typeof child.pid === "number") {
try {
killProcess(-child.pid, signal);
return true;
@@ -291,6 +300,16 @@ export function signalGatewayProcess(child, signal, killProcess = defaultKillPro
throw error;
}
}
if (platform === "win32" && typeof child.pid === "number") {
const args = ["/PID", String(child.pid), "/T"];
if (signal === "SIGKILL") {
args.push("/F");
}
const result = runTaskkill("taskkill", args, { stdio: "ignore" });
if (!result?.error && result?.status === 0) {
return true;
}
}
try {
return child.kill(signal);
} catch (error) {
@@ -304,8 +323,12 @@ export function signalGatewayProcess(child, signal, killProcess = defaultKillPro
/**
* Checks process-group liveness without treating an already-exited child as an error.
*/
export function isGatewayProcessAlive(child, killProcess = defaultKillProcess) {
if (process.platform !== "win32" && typeof child.pid === "number") {
export function isGatewayProcessAlive(
child,
killProcess = defaultKillProcess,
{ platform = process.platform } = {},
) {
if (platform !== "win32" && typeof child.pid === "number") {
try {
killProcess(-child.pid, 0);
return true;
@@ -319,17 +342,17 @@ export function isGatewayProcessAlive(child, killProcess = defaultKillProcess) {
return child.exitCode === null && child.signalCode === null;
}
function signalGatewayProcessForParentExit(child, signal, killProcess) {
function signalGatewayProcessForParentExit(child, signal, killProcess, signalOptions) {
try {
signalGatewayProcess(child, signal, killProcess);
signalGatewayProcess(child, signal, killProcess, signalOptions);
} catch {
// Parent shutdown cleanup is best effort; the original signal should win.
}
}
function gatewayProcessAliveForParentExit(child, killProcess) {
function gatewayProcessAliveForParentExit(child, killProcess, signalOptions) {
try {
return isGatewayProcessAlive(child, killProcess);
return isGatewayProcessAlive(child, killProcess, signalOptions);
} catch {
return true;
}
@@ -340,28 +363,37 @@ function gatewayProcessAliveForParentExit(child, killProcess) {
*/
export function installGatewayParentCleanup(
child,
{ killProcess = defaultKillProcess, processLike = process } = {},
{
killProcess = defaultKillProcess,
platform = process.platform,
processLike = process,
runTaskkill = defaultRunTaskkill,
} = {},
) {
const signalOptions = { platform, runTaskkill };
const signalHandlers = new Map();
let forceKillTimer;
let parentSignalPending = false;
const forceCleanup = (signal) => {
signalGatewayProcessForParentExit(child, signal, killProcess);
if (process.platform !== "win32") {
signalGatewayProcessForParentExit(child, "SIGKILL", killProcess);
signalGatewayProcessForParentExit(child, signal, killProcess, signalOptions);
if (platform !== "win32") {
signalGatewayProcessForParentExit(child, "SIGKILL", killProcess, signalOptions);
}
};
const cleanupAndReraise = (signal) => {
parentSignalPending = true;
signalGatewayProcessForParentExit(child, signal, killProcess);
signalGatewayProcessForParentExit(child, signal, killProcess, signalOptions);
const finish = () => {
forceKillTimer = undefined;
signalGatewayProcessForParentExit(child, "SIGKILL", killProcess);
signalGatewayProcessForParentExit(child, "SIGKILL", killProcess, signalOptions);
processLike.kill?.(processLike.pid, signal);
};
// Signal handlers can give the detached gateway group one normal teardown
// grace window before re-raising; process exit cleanup cannot wait.
if (process.platform === "win32" || !gatewayProcessAliveForParentExit(child, killProcess)) {
if (
platform === "win32" ||
!gatewayProcessAliveForParentExit(child, killProcess, signalOptions)
) {
processLike.kill?.(processLike.pid, signal);
return;
}
@@ -393,31 +425,40 @@ export function installGatewayParentCleanup(
return removeHandlers;
}
async function waitForGatewayExit(child, timeoutMs, killProcess = defaultKillProcess) {
async function waitForGatewayExit(
child,
timeoutMs,
killProcess = defaultKillProcess,
signalOptions = {},
) {
const deadline = Date.now() + timeoutMs;
while (Date.now() <= deadline) {
if (!isGatewayProcessAlive(child, killProcess)) {
if (!isGatewayProcessAlive(child, killProcess, signalOptions)) {
return true;
}
await sleep(Math.min(25, Math.max(0, deadline - Date.now())));
}
return !isGatewayProcessAlive(child, killProcess);
return !isGatewayProcessAlive(child, killProcess, signalOptions);
}
/**
* Stops the gateway with SIGTERM first and SIGKILL after the grace window.
*/
export async function stopGateway(child, options = {}) {
if (!isGatewayProcessAlive(child, options.killProcess)) {
const signalOptions = {
platform: options.platform ?? process.platform,
runTaskkill: options.runTaskkill ?? defaultRunTaskkill,
};
if (!isGatewayProcessAlive(child, options.killProcess, signalOptions)) {
return;
}
const killGraceMs = Math.max(0, options.killGraceMs ?? 1_500);
const forceKillGraceMs = Math.max(0, options.forceKillGraceMs ?? GATEWAY_FORCE_KILL_GRACE_MS);
signalGatewayProcess(child, "SIGTERM", options.killProcess);
const exited = await waitForGatewayExit(child, killGraceMs, options.killProcess);
signalGatewayProcess(child, "SIGTERM", options.killProcess, signalOptions);
const exited = await waitForGatewayExit(child, killGraceMs, options.killProcess, signalOptions);
if (!exited) {
signalGatewayProcess(child, "SIGKILL", options.killProcess);
await waitForGatewayExit(child, forceKillGraceMs, options.killProcess);
signalGatewayProcess(child, "SIGKILL", options.killProcess, signalOptions);
await waitForGatewayExit(child, forceKillGraceMs, options.killProcess, signalOptions);
}
}
+78 -12
View File
@@ -319,17 +319,54 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
signalCode: null,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
expect(signalGatewayProcess(child, "SIGTERM", kill)).toBe(true);
expect(signalGatewayProcess(child, "SIGTERM", kill, { runTaskkill })).toBe(true);
if (process.platform === "win32") {
expect(child.kill).toHaveBeenCalledWith("SIGTERM");
expect(runTaskkill).toHaveBeenCalledWith("taskkill", ["/PID", "12345", "/T"], {
stdio: "ignore",
});
expect(child.kill).not.toHaveBeenCalled();
} else {
expect(kill).toHaveBeenCalledWith(-12345, "SIGTERM");
expect(child.kill).not.toHaveBeenCalled();
}
});
it("signals Windows gateway process trees with taskkill", () => {
const child = Object.assign(new EventEmitter(), {
exitCode: null,
kill: vi.fn(),
pid: 12345,
signalCode: null,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
expect(
signalGatewayProcess(child, "SIGTERM", kill, {
platform: "win32",
runTaskkill,
}),
).toBe(true);
expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "12345", "/T"], {
stdio: "ignore",
});
expect(
signalGatewayProcess(child, "SIGKILL", kill, {
platform: "win32",
runTaskkill,
}),
).toBe(true);
expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "12345", "/T", "/F"], {
stdio: "ignore",
});
expect(kill).not.toHaveBeenCalled();
expect(child.kill).not.toHaveBeenCalled();
});
it("treats missing gateway process groups as already exited", () => {
const child = Object.assign(new EventEmitter(), {
exitCode: null,
@@ -340,8 +377,9 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
const kill = vi.fn(() => {
throw Object.assign(new Error("no such process"), { code: "ESRCH" });
});
const runTaskkill = vi.fn(() => ({ error: new Error("not found"), status: 1 }));
expect(signalGatewayProcess(child, "SIGTERM", kill)).toBe(false);
expect(signalGatewayProcess(child, "SIGTERM", kill, { runTaskkill })).toBe(false);
});
it("checks process group liveness instead of only the pnpm wrapper", () => {
@@ -369,12 +407,18 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
signalCode: null,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
await stopGateway(child, { killGraceMs: 1, killProcess: kill });
await stopGateway(child, { killGraceMs: 1, killProcess: kill, runTaskkill });
if (process.platform === "win32") {
expect(child.kill).toHaveBeenNthCalledWith(1, "SIGTERM");
expect(child.kill).toHaveBeenNthCalledWith(2, "SIGKILL");
expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "12346", "/T"], {
stdio: "ignore",
});
expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "12346", "/T", "/F"], {
stdio: "ignore",
});
expect(child.kill).not.toHaveBeenCalled();
} else {
expect(kill).toHaveBeenNthCalledWith(1, -12346, 0);
expect(kill).toHaveBeenNthCalledWith(2, -12346, "SIGTERM");
@@ -405,12 +449,23 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
}
return true;
});
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
await stopGateway(child, { forceKillGraceMs: 50, killGraceMs: 1, killProcess: kill });
await stopGateway(child, {
forceKillGraceMs: 50,
killGraceMs: 1,
killProcess: kill,
runTaskkill,
});
if (process.platform === "win32") {
expect(child.kill).toHaveBeenNthCalledWith(1, "SIGTERM");
expect(child.kill).toHaveBeenNthCalledWith(2, "SIGKILL");
expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "12350", "/T"], {
stdio: "ignore",
});
expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "12350", "/T", "/F"], {
stdio: "ignore",
});
expect(child.kill).not.toHaveBeenCalled();
} else {
expect(kill).toHaveBeenCalledWith(-12350, "SIGKILL");
expect(postKillLivenessChecks).toBe(2);
@@ -426,8 +481,9 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
signalCode: null,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
await stopGateway(child, { killGraceMs: 1, killProcess: kill });
await stopGateway(child, { killGraceMs: 1, killProcess: kill, runTaskkill });
if (process.platform === "win32") {
expect(child.kill).not.toHaveBeenCalled();
@@ -452,16 +508,21 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
pid: 98765,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
try {
const removeCleanup = installGatewayParentCleanup(child, {
killProcess: kill,
processLike,
runTaskkill,
});
processLike.emit("SIGTERM");
if (process.platform === "win32") {
expect(child.kill).toHaveBeenCalledWith("SIGTERM");
expect(runTaskkill).toHaveBeenCalledWith("taskkill", ["/PID", "12348", "/T"], {
stdio: "ignore",
});
expect(child.kill).not.toHaveBeenCalled();
expect(processLike.kill).toHaveBeenCalledWith(98765, "SIGTERM");
} else {
expect(kill).toHaveBeenNthCalledWith(1, -12348, "SIGTERM");
@@ -494,15 +555,20 @@ describe("scripts/measure-rpc-rtt.mjs", () => {
pid: 98766,
});
const kill = vi.fn(() => true);
const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));
const removeCleanup = installGatewayParentCleanup(child, {
killProcess: kill,
processLike,
runTaskkill,
});
processLike.emit("exit");
if (process.platform === "win32") {
expect(child.kill).toHaveBeenCalledWith("SIGTERM");
expect(runTaskkill).toHaveBeenCalledWith("taskkill", ["/PID", "12349", "/T"], {
stdio: "ignore",
});
expect(child.kill).not.toHaveBeenCalled();
} else {
expect(kill).toHaveBeenNthCalledWith(1, -12349, "SIGTERM");
expect(kill).toHaveBeenNthCalledWith(2, -12349, "SIGKILL");