mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(gateway): bound watch regression teardown
This commit is contained in:
@@ -20,6 +20,7 @@ const DEFAULTS = {
|
||||
readyTimeoutMs: 20_000,
|
||||
readySettleMs: 500,
|
||||
sigkillGraceMs: 10_000,
|
||||
sigkillExitGraceMs: 2_000,
|
||||
cpuWarnMs: 1_000,
|
||||
cpuFailMs: 8_000,
|
||||
distRuntimeFileGrowthMax: 200,
|
||||
@@ -69,6 +70,9 @@ function parseArgs(argv) {
|
||||
case "--sigkill-grace-ms":
|
||||
options.sigkillGraceMs = Number(readValue());
|
||||
break;
|
||||
case "--sigkill-exit-grace-ms":
|
||||
options.sigkillExitGraceMs = Number(readValue());
|
||||
break;
|
||||
case "--cpu-warn-ms":
|
||||
options.cpuWarnMs = Number(readValue());
|
||||
break;
|
||||
@@ -467,10 +471,6 @@ async function runTimedWatch(options, outputDir) {
|
||||
stderr += String(chunk);
|
||||
});
|
||||
|
||||
const exitPromise = new Promise((resolve) => {
|
||||
child.on("exit", (code, signal) => resolve({ code, signal }));
|
||||
});
|
||||
|
||||
let watchPid = null;
|
||||
for (let attempt = 0; attempt < 50; attempt += 1) {
|
||||
if (fs.existsSync(pidFilePath)) {
|
||||
@@ -491,30 +491,7 @@ async function runTimedWatch(options, outputDir) {
|
||||
await sleep(options.windowMs);
|
||||
const idleCpuEndMs = watchPid ? readProcessTreeCpuMs(watchPid) : null;
|
||||
|
||||
if (watchPid) {
|
||||
try {
|
||||
process.kill(watchPid, "SIGTERM");
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
|
||||
const gracefulExit = await Promise.race([
|
||||
exitPromise,
|
||||
sleep(options.sigkillGraceMs).then(() => null),
|
||||
]);
|
||||
|
||||
if (gracefulExit === null) {
|
||||
if (watchPid) {
|
||||
try {
|
||||
process.kill(watchPid, "SIGKILL");
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const exit = (await exitPromise) ?? { code: null, signal: null };
|
||||
const exit = await stopTimedWatchChild(child, watchPid, options);
|
||||
fs.writeFileSync(stdoutPath, stdout, "utf8");
|
||||
fs.writeFileSync(stderrPath, stderr, "utf8");
|
||||
const timing = fs.existsSync(timeFilePath)
|
||||
@@ -535,6 +512,56 @@ async function runTimedWatch(options, outputDir) {
|
||||
};
|
||||
}
|
||||
|
||||
export async function stopTimedWatchChild(child, watchPid, options, deps = {}) {
|
||||
const killProcess = deps.killProcess ?? ((pid, signal) => process.kill(pid, signal));
|
||||
const currentExit = () =>
|
||||
child.exitCode !== null || child.signalCode !== null
|
||||
? { code: child.exitCode, signal: child.signalCode }
|
||||
: null;
|
||||
const exited = new Promise((resolve) => {
|
||||
child.once("exit", (code, signal) => resolve({ code, signal }));
|
||||
});
|
||||
const waitForExit = async (ms) =>
|
||||
currentExit() ?? (await Promise.race([exited, sleep(ms).then(() => null)]));
|
||||
const signalWatchProcess = (signal) => {
|
||||
if (!watchPid) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
killProcess(watchPid, signal);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
};
|
||||
|
||||
const existingExit = currentExit();
|
||||
if (existingExit) {
|
||||
return existingExit;
|
||||
}
|
||||
|
||||
signalWatchProcess("SIGTERM");
|
||||
const gracefulExit = await waitForExit(options.sigkillGraceMs);
|
||||
if (gracefulExit) {
|
||||
return gracefulExit;
|
||||
}
|
||||
|
||||
signalWatchProcess("SIGKILL");
|
||||
const killedExit = await waitForExit(options.sigkillExitGraceMs ?? DEFAULTS.sigkillExitGraceMs);
|
||||
if (killedExit) {
|
||||
return killedExit;
|
||||
}
|
||||
|
||||
releaseUnsettledWatchChild(child);
|
||||
return { code: null, signal: "SIGKILL" };
|
||||
}
|
||||
|
||||
function releaseUnsettledWatchChild(child) {
|
||||
child.stdin?.destroy?.();
|
||||
child.stdout?.destroy?.();
|
||||
child.stderr?.destroy?.();
|
||||
child.unref?.();
|
||||
}
|
||||
|
||||
function parsePathFile(filePath) {
|
||||
return fs
|
||||
.readFileSync(filePath, "utf8")
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
hasGatewayReadyLog,
|
||||
shouldRefreshBuildStampForRestoredArtifacts,
|
||||
stopTimedWatchChild,
|
||||
writeBuildAndRuntimePostBuildStamps,
|
||||
} from "../../scripts/check-gateway-watch-regression.mjs";
|
||||
import {
|
||||
@@ -57,4 +59,38 @@ describe("check-gateway-watch-regression", () => {
|
||||
fs.rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("bounds teardown when the watch process ignores termination signals", async () => {
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
exitCode: number | null;
|
||||
signalCode: NodeJS.Signals | null;
|
||||
stderr: { destroy: ReturnType<typeof vi.fn> };
|
||||
stdin: { destroy: ReturnType<typeof vi.fn> };
|
||||
stdout: { destroy: ReturnType<typeof vi.fn> };
|
||||
unref: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
child.exitCode = null;
|
||||
child.signalCode = null;
|
||||
child.stderr = { destroy: vi.fn() };
|
||||
child.stdin = { destroy: vi.fn() };
|
||||
child.stdout = { destroy: vi.fn() };
|
||||
child.unref = vi.fn();
|
||||
const killProcess = vi.fn();
|
||||
|
||||
await expect(
|
||||
stopTimedWatchChild(
|
||||
child,
|
||||
1234,
|
||||
{ sigkillExitGraceMs: 1, sigkillGraceMs: 1 },
|
||||
{ killProcess },
|
||||
),
|
||||
).resolves.toEqual({ code: null, signal: "SIGKILL" });
|
||||
|
||||
expect(killProcess).toHaveBeenNthCalledWith(1, 1234, "SIGTERM");
|
||||
expect(killProcess).toHaveBeenNthCalledWith(2, 1234, "SIGKILL");
|
||||
expect(child.stdin.destroy).toHaveBeenCalledOnce();
|
||||
expect(child.stdout.destroy).toHaveBeenCalledOnce();
|
||||
expect(child.stderr.destroy).toHaveBeenCalledOnce();
|
||||
expect(child.unref).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user