fix(scripts): reap child check processes

This commit is contained in:
Vincent Koc
2026-04-23 08:44:22 -07:00
parent 5071c58346
commit ecefe1c39c
4 changed files with 249 additions and 46 deletions

View File

@@ -1,4 +1,3 @@
import { spawn } from "node:child_process";
import { performance } from "node:perf_hooks";
import {
detectChangedLanes,
@@ -8,6 +7,7 @@ import {
} from "./changed-lanes.mjs";
import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs";
import { printTimingSummary } from "./lib/check-timing-summary.mjs";
import { runManagedCommand } from "./lib/managed-child-process.mjs";
import { resolveChangedTestTargetPlan } from "./test-projects.test-support.mjs";
export function createChangedCheckPlan(result, options = {}) {
@@ -216,31 +216,22 @@ async function runNode(command, timings) {
async function runCommand(command, timings) {
const startedAt = performance.now();
console.error(`\n[check:changed] ${command.name}`);
const child = spawn(command.bin, command.args, {
stdio: "inherit",
shell: process.platform === "win32",
});
let status = 1;
try {
status = await runManagedCommand({
bin: command.bin,
args: command.args,
});
} catch (error) {
console.error(error);
}
return await new Promise((resolve) => {
child.once("error", (error) => {
console.error(error);
timings.push({
name: command.name,
durationMs: performance.now() - startedAt,
status: 1,
});
resolve(1);
});
child.once("close", (status) => {
const resolvedStatus = status ?? 1;
timings.push({
name: command.name,
durationMs: performance.now() - startedAt,
status: resolvedStatus,
});
resolve(resolvedStatus);
});
timings.push({
name: command.name,
durationMs: performance.now() - startedAt,
status,
});
return status;
}
function printSummary(timings, options) {

View File

@@ -0,0 +1,117 @@
import { spawn } from "node:child_process";
const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"];
const FORCE_KILL_DELAY_MS = 5_000;
/**
* @param {NodeJS.Signals} signal
* @returns {number}
*/
export function signalExitCode(signal) {
const signalNumber = signalNumberFor(signal);
return signalNumber ? 128 + signalNumber : 1;
}
/**
* @param {import("node:child_process").ChildProcess} child
* @param {NodeJS.Signals} [signal]
*/
export function terminateManagedChild(child, signal = "SIGTERM") {
if (!child.pid) {
return;
}
try {
if (process.platform !== "win32") {
process.kill(-child.pid, signal);
return;
}
} catch (error) {
if (!isMissingProcessError(error)) {
try {
child.kill(signal);
} catch {
// The process may have already exited between the group kill and fallback kill.
}
}
return;
}
child.kill(signal);
}
/**
* @param {{
* bin: string;
* args?: string[];
* cwd?: string;
* env?: NodeJS.ProcessEnv;
* stdio?: import("node:child_process").StdioOptions;
* shell?: boolean;
* }} options
* @returns {Promise<number>}
*/
export async function runManagedCommand({
bin,
args = [],
cwd,
env,
stdio = "inherit",
shell = process.platform === "win32",
}) {
const child = spawn(bin, args, {
cwd,
env,
stdio,
shell,
detached: process.platform !== "win32",
});
let receivedSignal = null;
let forceKillTimer = null;
const forwardSignal = (signal) => {
receivedSignal ??= signal;
terminateManagedChild(child, signal);
forceKillTimer ??= setTimeout(() => {
terminateManagedChild(child, "SIGKILL");
}, FORCE_KILL_DELAY_MS);
};
for (const signal of FORWARDED_SIGNALS) {
process.once(signal, forwardSignal);
}
try {
return await new Promise((resolve, reject) => {
child.once("error", reject);
child.once("close", (status) => {
if (forceKillTimer) {
clearTimeout(forceKillTimer);
}
resolve(receivedSignal ? signalExitCode(receivedSignal) : (status ?? 1));
});
});
} finally {
for (const signal of FORWARDED_SIGNALS) {
process.off(signal, forwardSignal);
}
}
}
function signalNumberFor(signal) {
switch (signal) {
case "SIGHUP":
return 1;
case "SIGINT":
return 2;
case "SIGTERM":
return 15;
default:
return 0;
}
}
function isMissingProcessError(error) {
return Boolean(error && typeof error === "object" && "code" in error && error.code === "ESRCH");
}

View File

@@ -1,10 +1,10 @@
import { spawnSync } from "node:child_process";
import path from "node:path";
import {
acquireLocalHeavyCheckLockSync,
applyLocalOxlintPolicy,
shouldAcquireLocalHeavyCheckLockForOxlint,
} from "./lib/local-heavy-check-runtime.mjs";
import { runManagedCommand } from "./lib/managed-child-process.mjs";
const oxlintPath = path.resolve("node_modules", ".bin", "oxlint");
const PREPARE_EXTENSION_BOUNDARY_ARGS = [
@@ -24,7 +24,7 @@ export function shouldPrepareExtensionPackageBoundaryArtifacts(args) {
return !args.some((arg) => OXLINT_PREPARE_SKIP_FLAGS.has(arg));
}
function prepareExtensionPackageBoundaryArtifacts(env) {
async function prepareExtensionPackageBoundaryArtifacts(env) {
const releaseArtifactsLock = acquireLocalHeavyCheckLockSync({
cwd: process.cwd(),
env,
@@ -33,18 +33,15 @@ function prepareExtensionPackageBoundaryArtifacts(env) {
});
try {
const result = spawnSync(process.execPath, PREPARE_EXTENSION_BOUNDARY_ARGS, {
stdio: "inherit",
const status = await runManagedCommand({
bin: process.execPath,
args: PREPARE_EXTENSION_BOUNDARY_ARGS,
env,
});
if (result.error) {
throw result.error;
}
if ((result.status ?? 1) !== 0) {
if (status !== 0) {
throw new Error(
`prepare-extension-package-boundary-artifacts failed with exit code ${result.status ?? 1}`,
`prepare-extension-package-boundary-artifacts failed with exit code ${status}`,
);
}
} finally {
@@ -52,7 +49,7 @@ function prepareExtensionPackageBoundaryArtifacts(env) {
}
}
export function main(argv = process.argv.slice(2), runtimeEnv = process.env) {
export async function main(argv = process.argv.slice(2), runtimeEnv = process.env) {
const { args: finalArgs, env } = applyLocalOxlintPolicy(argv, runtimeEnv);
const releaseLock =
env.OPENCLAW_OXLINT_SKIP_LOCK === "1"
@@ -73,25 +70,20 @@ export function main(argv = process.argv.slice(2), runtimeEnv = process.env) {
env.OPENCLAW_OXLINT_SKIP_PREPARE !== "1" &&
shouldPrepareExtensionPackageBoundaryArtifacts(finalArgs)
) {
prepareExtensionPackageBoundaryArtifacts(env);
await prepareExtensionPackageBoundaryArtifacts(env);
}
const result = spawnSync(oxlintPath, finalArgs, {
stdio: "inherit",
const status = await runManagedCommand({
bin: oxlintPath,
args: finalArgs,
env,
shell: process.platform === "win32",
});
if (result.error) {
throw result.error;
}
process.exitCode = result.status ?? 1;
process.exitCode = status;
} finally {
releaseLock();
}
}
if (import.meta.main) {
main();
await main();
}

View File

@@ -0,0 +1,103 @@
import { spawn } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { pathToFileURL } from "node:url";
import { describe, expect, it } from "vitest";
import { signalExitCode } from "../../scripts/lib/managed-child-process.mjs";
import { createScriptTestHarness } from "./test-helpers.js";
const { createTempDir } = createScriptTestHarness();
describe("managed-child-process", () => {
it("maps forwarded signals to shell-compatible exit codes", () => {
expect(signalExitCode("SIGHUP")).toBe(129);
expect(signalExitCode("SIGINT")).toBe(130);
expect(signalExitCode("SIGTERM")).toBe(143);
});
it("kills the managed child process group when the runner is terminated", async () => {
const dir = createTempDir("openclaw-managed-child-");
const childPath = path.join(dir, "child.mjs");
const runnerPath = path.join(dir, "runner.mjs");
const childPidPath = path.join(dir, "child.pid");
const helperUrl = pathToFileURL(path.resolve("scripts/lib/managed-child-process.mjs")).href;
fs.writeFileSync(
childPath,
`
import fs from "node:fs";
fs.writeFileSync(process.argv[2], String(process.pid));
for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"]) {
process.on(signal, () => process.exit(0));
}
setInterval(() => {}, 1_000);
`,
"utf8",
);
fs.writeFileSync(
runnerPath,
`
import { runManagedCommand } from ${JSON.stringify(helperUrl)};
process.exitCode = await runManagedCommand({
bin: process.execPath,
args: [${JSON.stringify(childPath)}, ${JSON.stringify(childPidPath)}],
stdio: "ignore",
});
`,
"utf8",
);
const runner = spawn(process.execPath, [runnerPath], {
stdio: "ignore",
});
let childPid = 0;
try {
await waitFor(() => fs.existsSync(childPidPath));
childPid = Number(fs.readFileSync(childPidPath, "utf8"));
expect(Number.isInteger(childPid)).toBe(true);
expect(isProcessAlive(childPid)).toBe(true);
process.kill(runner.pid!, "SIGTERM");
const result = await waitForClose(runner);
expect(result).toEqual({ code: 143, signal: null });
await waitFor(() => !isProcessAlive(childPid));
} finally {
if (runner.pid && isProcessAlive(runner.pid)) {
process.kill(runner.pid, "SIGKILL");
}
if (childPid && isProcessAlive(childPid)) {
process.kill(childPid, "SIGKILL");
}
}
});
});
async function waitFor(condition: () => boolean, timeoutMs = 3_000) {
const startedAt = Date.now();
while (!condition()) {
if (Date.now() - startedAt > timeoutMs) {
throw new Error("timed out waiting for condition");
}
await delay(25);
}
}
async function waitForClose(child: ReturnType<typeof spawn>) {
return await new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => {
child.once("close", (code, signal) => resolve({ code, signal }));
});
}
function isProcessAlive(pid: number) {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}