From 0cd8db97f91a6d340f9267efec55db769b41df65 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 13:30:28 +0200 Subject: [PATCH] fix(bench): kill gateway child trees on windows --- scripts/lib/gateway-bench-child.ts | 58 ++++++++++++++----- .../bench-gateway-child-test-support.ts | 58 +++++++++++++++++-- 2 files changed, 97 insertions(+), 19 deletions(-) diff --git a/scripts/lib/gateway-bench-child.ts b/scripts/lib/gateway-bench-child.ts index 97d0287437e..2255bf94a89 100644 --- a/scripts/lib/gateway-bench-child.ts +++ b/scripts/lib/gateway-bench-child.ts @@ -1,5 +1,5 @@ // Gateway Bench Child script supports OpenClaw repository automation. -import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { spawnSync, type ChildProcessWithoutNullStreams } from "node:child_process"; const TEARDOWN_GRACE_MS = 2_000; const TEARDOWN_KILL_GRACE_MS = 1_000; @@ -14,6 +14,13 @@ export type StopChildResult = ChildExit & { exitedBeforeTeardown: boolean; }; +export type StopChildOptions = { + killGraceMs?: number; + platform?: NodeJS.Platform; + runTaskkill?: typeof spawnSync; + teardownGraceMs?: number; +}; + export function delay(ms: number): Promise { return new Promise((resolve) => { setTimeout(resolve, ms); @@ -22,10 +29,14 @@ export function delay(ms: number): Promise { export async function stopChild( child: ChildProcessWithoutNullStreams, - options: { killGraceMs?: number; teardownGraceMs?: number } = {}, + options: StopChildOptions = {}, ): Promise { const teardownGraceMs = options.teardownGraceMs ?? TEARDOWN_GRACE_MS; const killGraceMs = options.killGraceMs ?? TEARDOWN_KILL_GRACE_MS; + const processTreeOptions = { + platform: options.platform ?? process.platform, + runTaskkill: options.runTaskkill ?? spawnSync, + }; let observedExit: ChildExit | null = null; const directExit = (): ChildExit | null => observedExit ?? @@ -34,7 +45,7 @@ export async function stopChild( : null); const currentExit = (): ChildExit | null => { const exit = directExit(); - if (exit == null || isProcessTreeAlive(child)) { + if (exit == null || isProcessTreeAlive(child, processTreeOptions)) { return null; } return exit; @@ -42,26 +53,26 @@ export async function stopChild( const waitForProcessTreeExit = async (ms: number): Promise => { const deadlineAt = Date.now() + ms; while (Date.now() < deadlineAt) { - if (!isProcessTreeAlive(child)) { + if (!isProcessTreeAlive(child, processTreeOptions)) { return true; } await delay(Math.min(EXIT_POLL_MS, deadlineAt - Date.now())); } - return !isProcessTreeAlive(child); + return !isProcessTreeAlive(child, processTreeOptions); }; const cleanupExitedProcessTree = async ( exit: ChildExit, exitedBeforeTeardown: boolean, ): Promise => { - if (!isProcessTreeAlive(child)) { + if (!isProcessTreeAlive(child, processTreeOptions)) { return { ...exit, exitedBeforeTeardown }; } - const sentTeardownSignal = killProcessTree(child, "SIGTERM"); + const sentTeardownSignal = killProcessTree(child, "SIGTERM", processTreeOptions); if (sentTeardownSignal) { await waitForProcessTreeExit(teardownGraceMs); } - if (sentTeardownSignal && isProcessTreeAlive(child)) { - killProcessTree(child, "SIGKILL"); + if (sentTeardownSignal && isProcessTreeAlive(child, processTreeOptions)) { + killProcessTree(child, "SIGKILL", processTreeOptions); await waitForProcessTreeExit(killGraceMs); } if (!sentTeardownSignal) { @@ -106,7 +117,7 @@ export async function stopChild( return await cleanupExitedProcessTree(queuedExit, true); } - const sentTeardownSignal = killProcessTree(child, "SIGTERM"); + const sentTeardownSignal = killProcessTree(child, "SIGTERM", processTreeOptions); const gracefulExit = await waitForExit(teardownGraceMs); if (gracefulExit != null) { return { ...gracefulExit, exitedBeforeTeardown: !sentTeardownSignal }; @@ -121,7 +132,7 @@ export async function stopChild( return { exitCode: null, exitedBeforeTeardown: true, signal: null }; } - killProcessTree(child, "SIGKILL"); + killProcessTree(child, "SIGKILL", processTreeOptions); const killedExit = await waitForExit(killGraceMs); const finalExit = killedExit ?? currentExit(); if (finalExit != null) { @@ -139,8 +150,11 @@ function releaseUnsettledChild(child: ChildProcessWithoutNullStreams): void { child.unref(); } -function isProcessTreeAlive(child: ChildProcessWithoutNullStreams): boolean { - if (process.platform === "win32" || child.pid === undefined) { +function isProcessTreeAlive( + child: ChildProcessWithoutNullStreams, + { platform = process.platform }: Pick = {}, +): boolean { + if (platform === "win32" || child.pid === undefined) { return false; } try { @@ -156,8 +170,12 @@ function isProcessStillExistsError(error: unknown): boolean { return code === "EPERM"; } -function killProcessTree(child: ChildProcessWithoutNullStreams, signal: NodeJS.Signals): boolean { - if (process.platform !== "win32" && child.pid !== undefined) { +function killProcessTree( + child: ChildProcessWithoutNullStreams, + signal: NodeJS.Signals, + { platform = process.platform, runTaskkill = spawnSync }: StopChildOptions = {}, +): boolean { + if (platform !== "win32" && child.pid !== undefined) { try { process.kill(-child.pid, signal); return true; @@ -165,5 +183,15 @@ function killProcessTree(child: ChildProcessWithoutNullStreams, signal: NodeJS.S // Fall back to the direct child below. } } + if (platform === "win32" && child.pid !== undefined) { + 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; + } + } return child.kill(signal); } diff --git a/test/scripts/bench-gateway-child-test-support.ts b/test/scripts/bench-gateway-child-test-support.ts index e85dc71c705..b28aaa9cb8f 100644 --- a/test/scripts/bench-gateway-child-test-support.ts +++ b/test/scripts/bench-gateway-child-test-support.ts @@ -1,4 +1,5 @@ // Gateway benchmark child test support simulates child process behavior for script tests. +import type { spawnSync } from "node:child_process"; import { EventEmitter } from "node:events"; import { expect, it, vi } from "vitest"; @@ -10,7 +11,12 @@ type StopChildResult = { type StopChild = ( child: TChild, - options?: { killGraceMs?: number; teardownGraceMs?: number }, + options?: { + killGraceMs?: number; + platform?: NodeJS.Platform; + runTaskkill?: typeof spawnSync; + teardownGraceMs?: number; + }, ) => Promise; export function registerStopChildBehaviorTests(params: { @@ -101,6 +107,52 @@ export function registerStopChildBehaviorTests(params: { expect(child.unref).toHaveBeenCalledOnce(); }); + it("signals Windows child process trees with taskkill", 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 = 4450; + child.signalCode = null; + child.stderr = { destroy: vi.fn() }; + child.stdin = { destroy: vi.fn() }; + child.stdout = { destroy: vi.fn() }; + child.unref = vi.fn(); + const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 })); + + await expect( + params.stopChild(child as unknown as TChild, { + killGraceMs: 1, + platform: "win32", + runTaskkill, + teardownGraceMs: 1, + }), + ).resolves.toEqual({ + exitedBeforeTeardown: false, + exitCode: null, + signal: "SIGKILL", + }); + expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "4450", "/T"], { + stdio: "ignore", + }); + expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "4450", "/T", "/F"], { + stdio: "ignore", + }); + expect(child.kill).not.toHaveBeenCalled(); + expect(child.stdin.destroy).toHaveBeenCalledOnce(); + expect(child.stdout.destroy).toHaveBeenCalledOnce(); + 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 () => { @@ -144,9 +196,7 @@ export function registerStopChildBehaviorTests(params: { child.exitCode = 0; child.emit("exit", 0, null); }); - await expect( - stopped, - ).resolves.toEqual({ + await expect(stopped).resolves.toEqual({ exitedBeforeTeardown: true, exitCode: 0, signal: null,