From 048014d1abe77e8d2f87745b471ed1776a06bccf Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 14:30:28 +0200 Subject: [PATCH] fix(memory): clean extension profiler child trees --- scripts/profile-extension-memory.mjs | 148 +++++++++++++-- test/scripts/profile-extension-memory.test.ts | 171 +++++++++++++++++- 2 files changed, 305 insertions(+), 14 deletions(-) diff --git a/scripts/profile-extension-memory.mjs b/scripts/profile-extension-memory.mjs index 68c45c02acbb..3235a84af445 100644 --- a/scripts/profile-extension-memory.mjs +++ b/scripts/profile-extension-memory.mjs @@ -18,6 +18,15 @@ const DEFAULT_TOP = 10; const OUTPUT_CAPTURE_MAX_CHARS = 128 * 1024; const STDERR_PREVIEW_MAX_CHARS = 8 * 1024; const RSS_MARKER = "__OPENCLAW_MAX_RSS_KB__="; +const PARENT_SIGNAL_EXIT_CODES = new Map([ + ["SIGHUP", 129], + ["SIGINT", 130], + ["SIGTERM", 143], +]); +const activeCaseChildren = new Set(); +const parentSignalHandlers = new Map(); +let parentSignalHandlersInstalled = false; +let parentSignalShutdownStarted = false; function printHelp() { console.log(`Usage: node scripts/profile-extension-memory.mjs [options] @@ -194,10 +203,12 @@ export async function runCase({ ["--import", hookPath, "--input-type=module", "--eval", body], { cwd: repoRoot, + detached: process.platform !== "win32", env, stdio: ["ignore", "pipe", "pipe"], }, ); + trackActiveCaseChild(child); let stdout = createOutputCapture(); let stderr = createOutputCapture(); @@ -207,7 +218,7 @@ export async function runCase({ let settled = false; const timer = setTimeout(() => { timedOut = true; - child.kill("SIGKILL"); + signalChildProcessTree(child, "SIGKILL"); }, timeoutMs); timer.unref?.(); @@ -217,6 +228,7 @@ export async function runCase({ } settled = true; clearTimeout(timer); + untrackActiveCaseChild(child); resolve(result); } @@ -243,21 +255,133 @@ export async function runCase({ }); }); child.on("close", (code, signal) => { - const stderrText = formatCapturedOutput(stderr); - settle({ - name, - code, - signal, - timedOut, - error: null, - stdout: formatCapturedOutput(stdout), - stderr: stderrText, - maxRssMb: maxRssMb ?? parseMaxRssMb(stderrText), - }); + void (async () => { + if (timedOut) { + await waitForChildProcessTreeExit(child, 1_000); + } + const stderrText = formatCapturedOutput(stderr); + settle({ + name, + code, + signal, + timedOut, + error: null, + stdout: formatCapturedOutput(stdout), + stderr: stderrText, + maxRssMb: maxRssMb ?? parseMaxRssMb(stderrText), + }); + })(); }); }); } +function signalChildProcessTree(child, signal) { + if (process.platform !== "win32" && typeof child.pid === "number") { + try { + process.kill(-child.pid, signal); + return; + } catch { + child.kill(signal); + return; + } + } + child.kill(signal); +} + +async function waitForChildProcessTreeExit(child, timeoutMs) { + if (process.platform === "win32" || typeof child.pid !== "number") { + return true; + } + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!childProcessTreeIsAlive(child)) { + return true; + } + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + } + return !childProcessTreeIsAlive(child); +} + +function childProcessTreeIsAlive(child) { + try { + process.kill(-child.pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + +function trackActiveCaseChild(child) { + activeCaseChildren.add(child); + installParentSignalHandlers(); +} + +function untrackActiveCaseChild(child) { + activeCaseChildren.delete(child); + if (activeCaseChildren.size === 0) { + removeParentSignalHandlers(); + } +} + +function installParentSignalHandlers() { + if (parentSignalHandlersInstalled) { + return; + } + parentSignalHandlersInstalled = true; + for (const signal of PARENT_SIGNAL_EXIT_CODES.keys()) { + const handler = () => handleParentSignal(signal); + parentSignalHandlers.set(signal, handler); + process.on(signal, handler); + } +} + +function removeParentSignalHandlers() { + if (!parentSignalHandlersInstalled || parentSignalShutdownStarted) { + return; + } + removeInstalledParentSignalHandlers(); +} + +function removeInstalledParentSignalHandlers() { + if (!parentSignalHandlersInstalled) { + return; + } + parentSignalHandlersInstalled = false; + for (const [signal, handler] of parentSignalHandlers) { + process.off(signal, handler); + } + parentSignalHandlers.clear(); +} + +function handleParentSignal(signal) { + if (parentSignalShutdownStarted) { + for (const child of activeCaseChildren) { + signalChildProcessTree(child, "SIGKILL"); + } + return; + } + parentSignalShutdownStarted = true; + void cleanupActiveCaseChildrenForParentSignal(signal); +} + +async function cleanupActiveCaseChildrenForParentSignal(signal) { + const children = [...activeCaseChildren]; + for (const child of children) { + signalChildProcessTree(child, signal); + } + await Promise.all(children.map((child) => waitForChildProcessTreeExit(child, 1_000))); + for (const child of children) { + if (childProcessTreeIsAlive(child)) { + signalChildProcessTree(child, "SIGKILL"); + } + } + await Promise.all(children.map((child) => waitForChildProcessTreeExit(child, 1_000))); + removeInstalledParentSignalHandlers(); + process.exit(PARENT_SIGNAL_EXIT_CODES.get(signal) ?? 1); +} + function buildImportBody(entryFiles, label) { const imports = entryFiles .map((filePath) => `await import(${JSON.stringify(filePath)});`) diff --git a/test/scripts/profile-extension-memory.test.ts b/test/scripts/profile-extension-memory.test.ts index d4281be90923..f8a6d55d965f 100644 --- a/test/scripts/profile-extension-memory.test.ts +++ b/test/scripts/profile-extension-memory.test.ts @@ -1,14 +1,37 @@ // Profile Extension Memory tests cover profile extension memory script behavior. -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { EventEmitter } from "node:events"; -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; +import { pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; import { parseArgs, runCase } from "../../scripts/profile-extension-memory.mjs"; const SCRIPT_PATH = path.resolve("scripts/profile-extension-memory.mjs"); +async function waitForCondition(predicate: () => boolean, timeoutMs = 5_000): Promise { + const started = Date.now(); + while (Date.now() - started < timeoutMs) { + if (predicate()) { + return; + } + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + } + throw new Error("timed out waiting for condition"); +} + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + function runProfileExtensionMemory(args: string[], cwd = process.cwd()) { return spawnSync(process.execPath, [SCRIPT_PATH, ...args], { cwd, @@ -16,6 +39,29 @@ function runProfileExtensionMemory(args: string[], cwd = process.cwd()) { }); } +async function waitForChildExit( + child: ReturnType, + timeoutMs = 8_000, +): Promise<{ status: number | null; signal: NodeJS.Signals | null }> { + let timer: ReturnType | undefined; + try { + 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 })); + }), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error("timed out waiting for child exit")), timeoutMs); + timer.unref?.(); + }), + ]); + } finally { + if (timer) { + clearTimeout(timer); + } + } +} + describe("scripts/profile-extension-memory", () => { it("prints help without requiring built plugin artifacts", () => { const result = runProfileExtensionMemory(["--help"]); @@ -174,4 +220,125 @@ describe("scripts/profile-extension-memory", () => { timedOut: false, }); }); + + it.runIf(process.platform !== "win32")( + "cleans timeout descendants before resolving the case", + async () => { + const root = mkdtempSync(path.join(tmpdir(), "openclaw-extension-memory-timeout-")); + const hookPath = path.join(root, "rss-hook.mjs"); + const descendantPidPath = path.join(root, "descendant.pid"); + let descendantPid = 0; + try { + writeFileSync(hookPath, "", "utf8"); + const descendantScript = [ + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join(""); + const body = [ + "const childProcess = await import('node:child_process');", + "const fs = await import('node:fs');", + "const descendant = childProcess.spawn(process.execPath, [", + " '--input-type=module',", + ` '--eval', ${JSON.stringify(descendantScript)},`, + "], { stdio: 'ignore' });", + `fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(descendant.pid));`, + "setInterval(() => {}, 1000);", + ].join("\n"); + const resultPromise = runCase({ + body, + env: process.env, + hookPath, + name: "timeout-descendant", + repoRoot: root, + timeoutMs: 1_000, + }); + + await waitForCondition(() => existsSync(descendantPidPath)); + descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10); + expect(Number.isInteger(descendantPid)).toBe(true); + expect(isProcessAlive(descendantPid)).toBe(true); + + await expect(resultPromise).resolves.toMatchObject({ + name: "timeout-descendant", + signal: "SIGKILL", + timedOut: true, + }); + await waitForCondition(() => !isProcessAlive(descendantPid)); + } finally { + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + rmSync(root, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")( + "cleans active case descendants on parent signal", + async () => { + const root = mkdtempSync(path.join(tmpdir(), "openclaw-extension-memory-parent-signal-")); + const hookPath = path.join(root, "rss-hook.mjs"); + const runnerPath = path.join(root, "parent-signal-runner.mjs"); + const descendantPidPath = path.join(root, "descendant.pid"); + let descendantPid = 0; + try { + writeFileSync(hookPath, "", "utf8"); + const descendantScript = [ + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join(""); + const body = [ + "const childProcess = await import('node:child_process');", + "const fs = await import('node:fs');", + "const descendant = childProcess.spawn(process.execPath, [", + " '--input-type=module',", + ` '--eval', ${JSON.stringify(descendantScript)},`, + "], { stdio: 'ignore' });", + `fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(descendant.pid));`, + "setInterval(() => {}, 1000);", + ].join("\n"); + writeFileSync( + runnerPath, + [ + `const { runCase } = await import(${JSON.stringify( + pathToFileURL(path.resolve("scripts/profile-extension-memory.mjs")).href, + )});`, + "void runCase({", + ` body: ${JSON.stringify(body)},`, + " env: process.env,", + ` hookPath: ${JSON.stringify(hookPath)},`, + " name: 'parent-signal-descendant',", + ` repoRoot: ${JSON.stringify(root)},`, + " timeoutMs: 30000,", + "});", + ].join("\n"), + "utf8", + ); + const runner = spawn(process.execPath, [runnerPath], { + stdio: "ignore", + }); + + 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 = waitForChildExit(runner); + process.kill(runner.pid!, "SIGTERM"); + await expect(runnerExit).resolves.toEqual({ status: 143, signal: null }); + await waitForCondition(() => !isProcessAlive(descendantPid)); + } finally { + if (runner.pid && isProcessAlive(runner.pid)) { + process.kill(runner.pid, "SIGKILL"); + } + } + } finally { + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + rmSync(root, { recursive: true, force: true }); + } + }, + ); });