diff --git a/scripts/lib/gateway-bench-child.ts b/scripts/lib/gateway-bench-child.ts index 9e667f143107..97d0287437ee 100644 --- a/scripts/lib/gateway-bench-child.ts +++ b/scripts/lib/gateway-bench-child.ts @@ -3,6 +3,7 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process"; const TEARDOWN_GRACE_MS = 2_000; const TEARDOWN_KILL_GRACE_MS = 1_000; +const EXIT_POLL_MS = 10; export type ChildExit = { exitCode: number | null; @@ -23,43 +24,95 @@ export async function stopChild( child: ChildProcessWithoutNullStreams, options: { killGraceMs?: number; teardownGraceMs?: number } = {}, ): Promise { - const currentExit = (): ChildExit | null => - child.exitCode != null || child.signalCode != null + const teardownGraceMs = options.teardownGraceMs ?? TEARDOWN_GRACE_MS; + const killGraceMs = options.killGraceMs ?? TEARDOWN_KILL_GRACE_MS; + let observedExit: ChildExit | null = null; + const directExit = (): ChildExit | null => + observedExit ?? + (child.exitCode != null || child.signalCode != null ? { exitCode: child.exitCode, signal: child.signalCode } - : null; + : null); + const currentExit = (): ChildExit | null => { + const exit = directExit(); + if (exit == null || isProcessTreeAlive(child)) { + return null; + } + return exit; + }; + const waitForProcessTreeExit = async (ms: number): Promise => { + const deadlineAt = Date.now() + ms; + while (Date.now() < deadlineAt) { + if (!isProcessTreeAlive(child)) { + return true; + } + await delay(Math.min(EXIT_POLL_MS, deadlineAt - Date.now())); + } + return !isProcessTreeAlive(child); + }; + const cleanupExitedProcessTree = async ( + exit: ChildExit, + exitedBeforeTeardown: boolean, + ): Promise => { + if (!isProcessTreeAlive(child)) { + return { ...exit, exitedBeforeTeardown }; + } + const sentTeardownSignal = killProcessTree(child, "SIGTERM"); + if (sentTeardownSignal) { + await waitForProcessTreeExit(teardownGraceMs); + } + if (sentTeardownSignal && isProcessTreeAlive(child)) { + killProcessTree(child, "SIGKILL"); + await waitForProcessTreeExit(killGraceMs); + } + if (!sentTeardownSignal) { + releaseUnsettledChild(child); + } + return { ...exit, exitedBeforeTeardown }; + }; - const existingExit = currentExit(); + const existingExit = directExit(); if (existingExit != null) { - return { ...existingExit, exitedBeforeTeardown: true }; + return await cleanupExitedProcessTree(existingExit, true); } - let observedExit: ChildExit | null = null; const exited = new Promise((resolve) => { child.once("exit", (exitCode, signal) => { observedExit = { exitCode, signal }; resolve(observedExit); }); }); - const waitForExit = async (ms: number): Promise => - await Promise.race([exited, delay(ms).then(() => null)]); + const waitForExit = async (ms: number): Promise => { + const deadlineAt = Date.now() + ms; + while (Date.now() < deadlineAt) { + const waitMs = Math.min(EXIT_POLL_MS, deadlineAt - Date.now()); + if (directExit() == null) { + await Promise.race([exited, delay(waitMs)]); + } else { + await delay(waitMs); + } + const exit = currentExit(); + if (exit != null) { + return exit; + } + } + return currentExit(); + }; await new Promise((resolve) => { setImmediate(resolve); }); - const queuedExit = observedExit ?? currentExit(); + const queuedExit = directExit(); if (queuedExit != null) { - return { ...queuedExit, exitedBeforeTeardown: true }; + return await cleanupExitedProcessTree(queuedExit, true); } - const teardownGraceMs = options.teardownGraceMs ?? TEARDOWN_GRACE_MS; - const killGraceMs = options.killGraceMs ?? TEARDOWN_KILL_GRACE_MS; const sentTeardownSignal = killProcessTree(child, "SIGTERM"); const gracefulExit = await waitForExit(teardownGraceMs); if (gracefulExit != null) { return { ...gracefulExit, exitedBeforeTeardown: !sentTeardownSignal }; } - const postGraceExit = currentExit() ?? observedExit; + const postGraceExit = currentExit(); if (postGraceExit != null) { return { ...postGraceExit, exitedBeforeTeardown: !sentTeardownSignal }; } @@ -70,7 +123,7 @@ export async function stopChild( killProcessTree(child, "SIGKILL"); const killedExit = await waitForExit(killGraceMs); - const finalExit = killedExit ?? currentExit() ?? observedExit; + const finalExit = killedExit ?? currentExit(); if (finalExit != null) { return { ...finalExit, exitedBeforeTeardown: false }; } @@ -86,6 +139,23 @@ function releaseUnsettledChild(child: ChildProcessWithoutNullStreams): void { child.unref(); } +function isProcessTreeAlive(child: ChildProcessWithoutNullStreams): boolean { + if (process.platform === "win32" || child.pid === undefined) { + return false; + } + try { + process.kill(-child.pid, 0); + return true; + } catch (error) { + return isProcessStillExistsError(error); + } +} + +function isProcessStillExistsError(error: unknown): boolean { + const code = (error as { code?: unknown }).code; + return code === "EPERM"; +} + function killProcessTree(child: ChildProcessWithoutNullStreams, signal: NodeJS.Signals): boolean { if (process.platform !== "win32" && child.pid !== undefined) { try { diff --git a/scripts/package-openclaw-for-docker.mjs b/scripts/package-openclaw-for-docker.mjs index 0a6d52c88463..7a83e740c4c8 100644 --- a/scripts/package-openclaw-for-docker.mjs +++ b/scripts/package-openclaw-for-docker.mjs @@ -14,6 +14,8 @@ const DEFAULT_PACKAGE_INVENTORY_TIMEOUT_MS = 5 * 60 * 1000; const DEFAULT_PACKAGE_PACK_TIMEOUT_MS = 5 * 60 * 1000; const DEFAULT_PACKAGE_TARBALL_CHECK_TIMEOUT_MS = 5 * 60 * 1000; const DEFAULT_TIMEOUT_KILL_AFTER_MS = 5_000; +const PROCESS_GROUP_EXIT_POLL_MS = 25; +const POST_FORCE_KILL_WAIT_MS = 1_000; const DEFAULT_CAPTURED_STDOUT_MAX_BYTES = 1024 * 1024; const ACTIVE_CHILD_KILLERS = new Set(); const SIGNAL_EXIT_CODES = { @@ -208,6 +210,18 @@ function run(command, args, cwd, options = {}) { return error?.code === "EPERM"; } }; + const waitForProcessGroupExit = async (timeoutMs) => { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!processGroupAlive()) { + return true; + } + await new Promise((resolvePoll) => { + setTimeout(resolvePoll, PROCESS_GROUP_EXIT_POLL_MS); + }); + } + return !processGroupAlive(); + }; const terminateChild = () => { killChild("SIGTERM"); forceKillTimeout = setTimeout(() => { @@ -228,6 +242,16 @@ function run(command, args, cwd, options = {}) { terminateChild(); }, options.timeoutMs); timeout?.unref?.(); + const finishAfterTeardown = async (error, value = "") => { + if (processGroupAlive()) { + await waitForProcessGroupExit(options.killAfterMs ?? DEFAULT_TIMEOUT_KILL_AFTER_MS); + } + if (processGroupAlive()) { + killChild("SIGKILL"); + await waitForProcessGroupExit(POST_FORCE_KILL_WAIT_MS); + } + finish(error, value); + }; if (options.captureStdout) { child.stdout.on("data", (chunk) => { if (outputLimitExceeded) { @@ -250,11 +274,13 @@ function run(command, args, cwd, options = {}) { child.on("error", (error) => finish(error)); child.on("close", (status, signal) => { if (timedOut) { - finish(new Error(`${command} ${args.join(" ")} timed out after ${options.timeoutMs}ms`)); + void finishAfterTeardown( + new Error(`${command} ${args.join(" ")} timed out after ${options.timeoutMs}ms`), + ); return; } if (outputLimitExceeded) { - finish( + void finishAfterTeardown( new Error( `${command} ${args.join(" ")} exceeded captured stdout limit (${maxCapturedStdoutBytes} bytes)`, ), diff --git a/test/scripts/bench-gateway-child-test-support.ts b/test/scripts/bench-gateway-child-test-support.ts index 625eaa9ce1e4..e85dc71c705f 100644 --- a/test/scripts/bench-gateway-child-test-support.ts +++ b/test/scripts/bench-gateway-child-test-support.ts @@ -100,4 +100,133 @@ export function registerStopChildBehaviorTests(params: { expect(child.stderr.destroy).toHaveBeenCalledOnce(); expect(child.unref).toHaveBeenCalledOnce(); }); + + it.skipIf(process.platform === "win32")( + "preserves pre-teardown wrapper exits while cleaning the process group", + async () => { + const child = new EventEmitter() as EventEmitter & { + exitCode: number | null; + kill: ReturnType; + pid: number; + signalCode: NodeJS.Signals | null; + stderr: { destroy: ReturnType }; + stdin: { destroy: ReturnType }; + stdout: { destroy: ReturnType }; + unref: ReturnType; + }; + child.exitCode = null; + child.kill = vi.fn(() => true); + child.pid = 4444; + child.signalCode = null; + child.stderr = { destroy: vi.fn() }; + child.stdin = { destroy: vi.fn() }; + child.stdout = { destroy: vi.fn() }; + child.unref = vi.fn(); + + let processGroupAlive = true; + const processKill = vi.spyOn(process, "kill").mockImplementation((pid, signal) => { + expect(pid).toBe(-child.pid); + if (signal === "SIGKILL") { + processGroupAlive = false; + return true; + } + if (signal === 0 && !processGroupAlive) { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } + return true; + }); + try { + const stopped = params.stopChild(child as unknown as TChild, { + killGraceMs: 50, + teardownGraceMs: 1, + }); + queueMicrotask(() => { + child.exitCode = 0; + child.emit("exit", 0, null); + }); + await expect( + stopped, + ).resolves.toEqual({ + exitedBeforeTeardown: true, + exitCode: 0, + signal: null, + }); + expect(processKill).toHaveBeenCalledWith(-child.pid, "SIGTERM"); + expect(processKill).toHaveBeenCalledWith(-child.pid, "SIGKILL"); + expect(child.kill).not.toHaveBeenCalled(); + expect(child.stdin.destroy).not.toHaveBeenCalled(); + expect(child.stdout.destroy).not.toHaveBeenCalled(); + expect(child.stderr.destroy).not.toHaveBeenCalled(); + expect(child.unref).not.toHaveBeenCalled(); + } finally { + processKill.mockRestore(); + } + }, + ); + + it.skipIf(process.platform === "win32")( + "waits for the process group after a teardown-triggered wrapper exit", + async () => { + const child = new EventEmitter() as EventEmitter & { + exitCode: number | null; + kill: ReturnType; + pid: number; + signalCode: NodeJS.Signals | null; + stderr: { destroy: ReturnType }; + stdin: { destroy: ReturnType }; + stdout: { destroy: ReturnType }; + unref: ReturnType; + }; + child.exitCode = null; + child.kill = vi.fn(() => true); + child.pid = 4445; + child.signalCode = null; + child.stderr = { destroy: vi.fn() }; + child.stdin = { destroy: vi.fn() }; + child.stdout = { destroy: vi.fn() }; + child.unref = vi.fn(); + + let emittedExit = false; + let processGroupAlive = true; + const processKill = vi.spyOn(process, "kill").mockImplementation((pid, signal) => { + expect(pid).toBe(-child.pid); + if (signal === "SIGTERM" && !emittedExit) { + emittedExit = true; + queueMicrotask(() => { + child.exitCode = 0; + child.emit("exit", 0, null); + }); + } + if (signal === "SIGKILL") { + processGroupAlive = false; + return true; + } + if (signal === 0 && !processGroupAlive) { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } + return true; + }); + try { + await expect( + params.stopChild(child as unknown as TChild, { + killGraceMs: 50, + teardownGraceMs: 1, + }), + ).resolves.toEqual({ + exitedBeforeTeardown: false, + exitCode: 0, + signal: null, + }); + expect(processKill).toHaveBeenCalledWith(-child.pid, "SIGTERM"); + expect(processKill).toHaveBeenCalledWith(-child.pid, "SIGKILL"); + expect(child.kill).not.toHaveBeenCalled(); + expect(child.stdin.destroy).not.toHaveBeenCalled(); + expect(child.stdout.destroy).not.toHaveBeenCalled(); + expect(child.stderr.destroy).not.toHaveBeenCalled(); + expect(child.unref).not.toHaveBeenCalled(); + } finally { + processKill.mockRestore(); + } + }, + ); }