mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-09 03:22:40 +00:00
fix(crabbox): clean wrapper child trees on parent signal
This commit is contained in:
@@ -2560,23 +2560,23 @@ const childInvocation = spawnInvocation(binary, childArgs, childEnv, process.pla
|
||||
const child = spawn(childInvocation.command, childInvocation.args, {
|
||||
cwd: childCwd,
|
||||
stdio: "inherit",
|
||||
detached: process.platform !== "win32",
|
||||
env: childEnv,
|
||||
windowsVerbatimArguments: childInvocation.windowsVerbatimArguments,
|
||||
});
|
||||
const childKillGraceMs = 5_000;
|
||||
let childForceKillTimer;
|
||||
let childTreeShutdownStarted = false;
|
||||
if (fullCheckout) {
|
||||
try {
|
||||
stopFullCheckoutKeepalive = startFullCheckoutKeepalive(fullCheckout, {
|
||||
intervalMs: fullCheckoutKeepaliveIntervalMsValue,
|
||||
onMissing: () => {
|
||||
if (!child.killed) {
|
||||
child.kill("SIGTERM");
|
||||
}
|
||||
void exitAfterChildTreeTermination(child, "SIGTERM", 1);
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
if (!child.killed) {
|
||||
child.kill("SIGTERM");
|
||||
}
|
||||
signalChildProcessTree(child, "SIGTERM");
|
||||
cleanupOnce();
|
||||
throw error;
|
||||
}
|
||||
@@ -2588,17 +2588,17 @@ const signalExitCodes = new Map([
|
||||
["SIGTERM", 143],
|
||||
]);
|
||||
for (const signal of signalExitCodes.keys()) {
|
||||
process.once(signal, () => {
|
||||
if (!child.killed) {
|
||||
child.kill(signal);
|
||||
}
|
||||
cleanupOnce();
|
||||
process.exit(signalExitCodes.get(signal) ?? 1);
|
||||
process.on(signal, () => {
|
||||
void exitAfterChildTreeTermination(child, signal, signalExitCodes.get(signal) ?? 1);
|
||||
});
|
||||
}
|
||||
process.once("exit", cleanupOnce);
|
||||
|
||||
child.on("exit", (code, signal) => {
|
||||
clearChildForceKillTimer();
|
||||
if (childTreeShutdownStarted) {
|
||||
return;
|
||||
}
|
||||
let fullCheckoutAvailable = true;
|
||||
if (fullCheckout) {
|
||||
fullCheckoutAvailable = assertFullCheckoutAvailableBeforeExit(fullCheckout.dir);
|
||||
@@ -2612,6 +2612,10 @@ child.on("exit", (code, signal) => {
|
||||
});
|
||||
|
||||
child.on("error", (error) => {
|
||||
clearChildForceKillTimer();
|
||||
if (childTreeShutdownStarted) {
|
||||
return;
|
||||
}
|
||||
if (fullCheckout) {
|
||||
assertFullCheckoutAvailableBeforeExit(fullCheckout.dir);
|
||||
}
|
||||
@@ -2619,3 +2623,81 @@ child.on("error", (error) => {
|
||||
console.error(`[crabbox] failed to execute ${displayBinary}: ${error.message}`);
|
||||
process.exit(2);
|
||||
});
|
||||
|
||||
async function exitAfterChildTreeTermination(childProcess, signal, exitCode) {
|
||||
if (childTreeShutdownStarted) {
|
||||
signalChildProcessTree(childProcess, "SIGKILL");
|
||||
return;
|
||||
}
|
||||
childTreeShutdownStarted = true;
|
||||
signalChildProcessTree(childProcess, signal);
|
||||
await waitForChildTreeExit(childProcess, childKillGraceMs);
|
||||
if (childProcessTreeIsAlive(childProcess)) {
|
||||
signalChildProcessTree(childProcess, "SIGKILL");
|
||||
}
|
||||
await waitForChildTreeExit(childProcess, childKillGraceMs);
|
||||
cleanupOnce();
|
||||
process.exit(exitCode);
|
||||
}
|
||||
|
||||
function signalChildProcessTree(childProcess, signal) {
|
||||
if (
|
||||
process.platform === "win32" &&
|
||||
(childProcess.exitCode !== null || childProcess.signalCode !== null)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
if (process.platform !== "win32" && typeof childProcess.pid === "number") {
|
||||
process.kill(-childProcess.pid, signal);
|
||||
} else {
|
||||
childProcess.kill(signal);
|
||||
}
|
||||
} catch (error) {
|
||||
if (error?.code !== "ESRCH") {
|
||||
try {
|
||||
childProcess.kill(signal);
|
||||
} catch {}
|
||||
}
|
||||
}
|
||||
if (signal !== "SIGKILL" && !childForceKillTimer) {
|
||||
childForceKillTimer = setTimeout(() => {
|
||||
childForceKillTimer = undefined;
|
||||
signalChildProcessTree(childProcess, "SIGKILL");
|
||||
}, childKillGraceMs);
|
||||
childForceKillTimer.unref?.();
|
||||
}
|
||||
}
|
||||
|
||||
function clearChildForceKillTimer() {
|
||||
if (childForceKillTimer) {
|
||||
clearTimeout(childForceKillTimer);
|
||||
childForceKillTimer = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function childProcessTreeIsAlive(childProcess) {
|
||||
if (process.platform === "win32" || typeof childProcess.pid !== "number") {
|
||||
return childProcess.exitCode === null && childProcess.signalCode === null;
|
||||
}
|
||||
try {
|
||||
process.kill(-childProcess.pid, 0);
|
||||
return true;
|
||||
} catch (error) {
|
||||
return error?.code === "EPERM";
|
||||
}
|
||||
}
|
||||
|
||||
async function waitForChildTreeExit(childProcess, timeoutMs) {
|
||||
const started = Date.now();
|
||||
while (Date.now() - started < timeoutMs) {
|
||||
if (!childProcessTreeIsAlive(childProcess)) {
|
||||
clearChildForceKillTimer();
|
||||
return true;
|
||||
}
|
||||
await new Promise((done) => {
|
||||
setTimeout(done, 50);
|
||||
});
|
||||
}
|
||||
return !childProcessTreeIsAlive(childProcess);
|
||||
}
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
// Crabbox Wrapper tests cover crabbox wrapper script behavior.
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { spawn, spawnSync } from "node:child_process";
|
||||
import {
|
||||
chmodSync,
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
readFileSync,
|
||||
readdirSync,
|
||||
rmSync,
|
||||
statSync,
|
||||
@@ -11,6 +13,7 @@ import {
|
||||
} from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
@@ -44,6 +47,12 @@ function writeFakeCrabbox(binDir: string, helpText: string): string {
|
||||
const helperPath = path.join(binDir, "fake-crabbox-json.cjs");
|
||||
|
||||
if (process.platform !== "win32") {
|
||||
const signalIgnoringDescendantScript = [
|
||||
"process.on('SIGHUP', () => {});",
|
||||
"process.on('SIGINT', () => {});",
|
||||
"process.on('SIGTERM', () => {});",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join("");
|
||||
const script = [
|
||||
"#!/bin/sh",
|
||||
'if [ "$1" = "--version" ]; then',
|
||||
@@ -116,6 +125,12 @@ function writeFakeCrabbox(binDir: string, helpText: string): string {
|
||||
" fi",
|
||||
' cd "$deleted_cwd" || exit 1',
|
||||
"fi",
|
||||
'if [ -n "${OPENCLAW_FAKE_CRABBOX_DESCENDANT_PID_PATH:-}" ]; then',
|
||||
` ${shellSingleQuote(process.execPath)} --input-type=module --eval ${shellSingleQuote(signalIgnoringDescendantScript)} &`,
|
||||
' printf "%s" "$!" > "$OPENCLAW_FAKE_CRABBOX_DESCENDANT_PID_PATH"',
|
||||
' trap "exit 0" INT TERM HUP',
|
||||
" while :; do sleep 1; done",
|
||||
"fi",
|
||||
'printf "%s\\0" "__OPENCLAW_FAKE_CRABBOX_V1__"',
|
||||
'printf "%s\\0" "$PWD"',
|
||||
'printf "%s\\0" "$#"',
|
||||
@@ -280,47 +295,57 @@ function shellArgListCondition(args: string[]): string {
|
||||
return checks.join(" && ");
|
||||
}
|
||||
|
||||
function runWrapper(
|
||||
helpText: string,
|
||||
args: string[],
|
||||
options: {
|
||||
configJson?: Record<string, unknown>;
|
||||
configStatus?: number;
|
||||
env?: Record<string, string>;
|
||||
extraPathEntries?: string[];
|
||||
gitResponses?: Record<string, { status?: number; stdout?: string; stderr?: string }>;
|
||||
input?: string;
|
||||
} = {},
|
||||
) {
|
||||
const binDir = makeFakeCrabbox(helpText);
|
||||
const gitResponses = { ...defaultGitResponses, ...options.gitResponses };
|
||||
const gitBinDir = makeFakeGit(gitResponses);
|
||||
function runWrapper(helpText: string, args: string[], options: WrapperOptions = {}) {
|
||||
return spawnSync(process.execPath, ["scripts/crabbox-wrapper.mjs", ...args], {
|
||||
cwd: repoRoot,
|
||||
encoding: "utf8",
|
||||
input: options.input,
|
||||
env: {
|
||||
...process.env,
|
||||
PATH: [...(options.extraPathEntries ?? []), binDir, gitBinDir, process.env.PATH ?? ""]
|
||||
.filter(Boolean)
|
||||
.join(path.delimiter),
|
||||
CRABBOX_PROVIDER: "",
|
||||
OPENCLAW_CRABBOX_ALLOW_DIRECT_AWS: "",
|
||||
OPENCLAW_CRABBOX_SYNC_MIN_FREE_BYTES: "0",
|
||||
OPENCLAW_CRABBOX_WRAPPER_IGNORE_REPO_BINARY: "1",
|
||||
...(options.configJson
|
||||
? { OPENCLAW_FAKE_CRABBOX_CONFIG_JSON: JSON.stringify(options.configJson) }
|
||||
: {}),
|
||||
...(options.configStatus
|
||||
? { OPENCLAW_FAKE_CRABBOX_CONFIG_STATUS: String(options.configStatus) }
|
||||
: {}),
|
||||
...options.env,
|
||||
OPENCLAW_FAKE_GIT_RESPONSES: JSON.stringify(gitResponses),
|
||||
},
|
||||
env: wrapperEnv(helpText, options),
|
||||
timeout: 10_000,
|
||||
});
|
||||
}
|
||||
|
||||
type WrapperOptions = {
|
||||
configJson?: Record<string, unknown>;
|
||||
configStatus?: number;
|
||||
env?: Record<string, string>;
|
||||
extraPathEntries?: string[];
|
||||
gitResponses?: Record<string, { status?: number; stdout?: string; stderr?: string }>;
|
||||
input?: string;
|
||||
};
|
||||
|
||||
function spawnWrapper(helpText: string, args: string[], options: WrapperOptions = {}) {
|
||||
return spawn(process.execPath, ["scripts/crabbox-wrapper.mjs", ...args], {
|
||||
cwd: repoRoot,
|
||||
env: wrapperEnv(helpText, options),
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
}
|
||||
|
||||
function wrapperEnv(helpText: string, options: WrapperOptions): NodeJS.ProcessEnv {
|
||||
const binDir = makeFakeCrabbox(helpText);
|
||||
const gitResponses = { ...defaultGitResponses, ...options.gitResponses };
|
||||
const gitBinDir = makeFakeGit(gitResponses);
|
||||
return {
|
||||
...process.env,
|
||||
PATH: [...(options.extraPathEntries ?? []), binDir, gitBinDir, process.env.PATH ?? ""]
|
||||
.filter(Boolean)
|
||||
.join(path.delimiter),
|
||||
CRABBOX_PROVIDER: "",
|
||||
OPENCLAW_CRABBOX_ALLOW_DIRECT_AWS: "",
|
||||
OPENCLAW_CRABBOX_SYNC_MIN_FREE_BYTES: "0",
|
||||
OPENCLAW_CRABBOX_WRAPPER_IGNORE_REPO_BINARY: "1",
|
||||
...(options.configJson
|
||||
? { OPENCLAW_FAKE_CRABBOX_CONFIG_JSON: JSON.stringify(options.configJson) }
|
||||
: {}),
|
||||
...(options.configStatus
|
||||
? { OPENCLAW_FAKE_CRABBOX_CONFIG_STATUS: String(options.configStatus) }
|
||||
: {}),
|
||||
...options.env,
|
||||
OPENCLAW_FAKE_GIT_RESPONSES: JSON.stringify(gitResponses),
|
||||
};
|
||||
}
|
||||
|
||||
function parseFakeCrabboxOutput(result: ReturnType<typeof runWrapper>): {
|
||||
args: string[];
|
||||
cwd: string;
|
||||
@@ -355,6 +380,76 @@ function normalizeShellLineEndings(value: string): string {
|
||||
return value.replace(/\r\n/g, "\n");
|
||||
}
|
||||
|
||||
async function waitForCondition(predicate: () => boolean, timeoutMs = 8_000): Promise<void> {
|
||||
const started = Date.now();
|
||||
while (Date.now() - started < timeoutMs) {
|
||||
if (predicate()) {
|
||||
return;
|
||||
}
|
||||
await delay(50);
|
||||
}
|
||||
throw new Error("timed out waiting for condition");
|
||||
}
|
||||
|
||||
async function waitForProcessExit(
|
||||
child: ReturnType<typeof spawnWrapper>,
|
||||
timeoutMs = 12_000,
|
||||
): Promise<{ status: number | null; signal: NodeJS.Signals | null }> {
|
||||
return await Promise.race([
|
||||
new Promise<{ status: number | null; signal: NodeJS.Signals | null }>((resolve, reject) => {
|
||||
child.once("error", reject);
|
||||
child.once("exit", (status, signal) => resolve({ status, signal }));
|
||||
}),
|
||||
delay(timeoutMs).then(() => {
|
||||
throw new Error("timed out waiting for wrapper process exit");
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
function isProcessAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function runSignalCleanupProof(sendSignals: (pid: number) => Promise<void>): Promise<void> {
|
||||
const root = mkdtempSync(path.join(tmpdir(), "openclaw-crabbox-descendant-"));
|
||||
tempDirs.push(root);
|
||||
const descendantPidPath = path.join(root, "descendant.pid");
|
||||
let descendantPid = 0;
|
||||
const runner = spawnWrapper(
|
||||
"provider: hetzner, aws, local-container, blacksmith-testbox, or cloudflare\n",
|
||||
["run", "--provider", "aws", "--", "echo ok"],
|
||||
{
|
||||
env: {
|
||||
OPENCLAW_FAKE_CRABBOX_DESCENDANT_PID_PATH: descendantPidPath,
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
try {
|
||||
await waitForCondition(() => existsSync(descendantPidPath));
|
||||
descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10);
|
||||
expect(Number.isInteger(descendantPid)).toBe(true);
|
||||
expect(isProcessAlive(descendantPid)).toBe(true);
|
||||
|
||||
const runnerExit = waitForProcessExit(runner);
|
||||
await sendSignals(runner.pid!);
|
||||
await expect(runnerExit).resolves.toEqual({ status: 143, signal: null });
|
||||
await waitForCondition(() => !isProcessAlive(descendantPid));
|
||||
} finally {
|
||||
if (runner.pid && isProcessAlive(runner.pid)) {
|
||||
runner.kill("SIGKILL");
|
||||
}
|
||||
if (descendantPid && isProcessAlive(descendantPid)) {
|
||||
process.kill(descendantPid, "SIGKILL");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function testCrabboxConfigDir(home: string): string {
|
||||
if (process.platform === "darwin") {
|
||||
return path.join(home, "Library", "Application Support", "crabbox");
|
||||
@@ -3041,6 +3136,26 @@ describe.concurrent("scripts/crabbox-wrapper", () => {
|
||||
}
|
||||
});
|
||||
|
||||
(process.platform === "win32" ? it.skip : it)(
|
||||
"terminates Crabbox descendants before parent signal exit",
|
||||
async () => {
|
||||
await runSignalCleanupProof(async (runnerPid) => {
|
||||
process.kill(runnerPid, "SIGTERM");
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
(process.platform === "win32" ? it.skip : it)(
|
||||
"keeps cleanup active after repeated parent signals",
|
||||
async () => {
|
||||
await runSignalCleanupProof(async (runnerPid) => {
|
||||
process.kill(runnerPid, "SIGTERM");
|
||||
await delay(50);
|
||||
process.kill(runnerPid, "SIGTERM");
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
(process.platform === "win32" ? it.skip : it)(
|
||||
"terminates when sparse-sync temporary full checkouts disappear while Crabbox is running",
|
||||
() => {
|
||||
|
||||
Reference in New Issue
Block a user