From 03a9e022ed83f8d6c4e7b1f417a9d7919b6fc3da Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 5 Aug 2026 06:57:55 -0700 Subject: [PATCH] fix(managed-child): honor process groups that vanish before cleanup (#119614) --- scripts/lib/managed-child-process.mjs | 24 +++++---------- test/scripts/managed-child-process.test.ts | 34 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/scripts/lib/managed-child-process.mjs b/scripts/lib/managed-child-process.mjs index b6f6aa3e6061..4cf35f027778 100644 --- a/scripts/lib/managed-child-process.mjs +++ b/scripts/lib/managed-child-process.mjs @@ -144,7 +144,6 @@ export async function runManagedCommand({ let timeoutTimer = null; let signalTimeout; let timedOut = false; - let childResult; let timeoutTermination; const timeoutTriggered = new Promise((resolve) => { signalTimeout = resolve; @@ -222,14 +221,10 @@ export async function runManagedCommand({ } throw outcome.error; } - childResult = outcome.status; if (requireProcessTreeExit) { - await ensureManagedProcessTreeExit(child, platform, { - rejectIfLive: true, - terminateIfLive: true, - }); + await ensureManagedProcessTreeExit(child, platform, { terminateIfLive: true }); } - return childResult; + return outcome.status; } finally { if (timeoutTimer) { clearTimeout(timeoutTimer); @@ -276,7 +271,7 @@ function processGroupStatus(pid) { async function ensureManagedProcessTreeExit( child, platform, - { rejectIfLive = false, terminateIfLive = false, windowsTermination } = {}, + { terminateIfLive = false, windowsTermination } = {}, ) { if (platform === "win32") { if (windowsTermination?.processTreeState === "indeterminate") { @@ -294,10 +289,10 @@ async function ensureManagedProcessTreeExit( return; } let status = initialStatus; - let sawLive = initialStatus === "live"; - if (terminateIfLive) { - terminateManagedChild(child, "SIGKILL", { platform }); - } + // A missing group at signal time supersedes the earlier racy liveness probe. + const termination = terminateIfLive + ? terminateManagedChild(child, "SIGKILL", { platform }) + : undefined; const deadline = Date.now() + PROCESS_GROUP_DRAIN_TIMEOUT_MS; while (Date.now() < deadline) { await new Promise((resolve) => { @@ -305,7 +300,7 @@ async function ensureManagedProcessTreeExit( }); status = processGroupStatus(child.pid); if (status === "dead") { - if (rejectIfLive && sawLive) { + if (terminateIfLive && termination?.processTreeState !== "terminated") { throw createManagedCommandCleanupError( "Managed command exited while its process group remained active", child, @@ -315,9 +310,6 @@ async function ensureManagedProcessTreeExit( } return; } - if (status === "live") { - sawLive = true; - } } const processTreeState = status === "indeterminate" ? "indeterminate" : "live"; throw createManagedCommandCleanupError( diff --git a/test/scripts/managed-child-process.test.ts b/test/scripts/managed-child-process.test.ts index 81f6528b2945..46bd3b59151a 100644 --- a/test/scripts/managed-child-process.test.ts +++ b/test/scripts/managed-child-process.test.ts @@ -418,6 +418,40 @@ setInterval(() => {}, 1_000); expect(isProcessAlive(childPid)).toBe(false); }); + posixIt("accepts a process group that vanishes before its cleanup signal", async () => { + const originalKill = process.kill; + let childPid = 0; + let injectedLiveGroup = false; + process.kill = ((pid: number, signal?: NodeJS.Signals | number) => { + if (pid === -childPid && signal === 0 && !injectedLiveGroup) { + injectedLiveGroup = true; + return true; + } + return originalKill(pid, signal); + }) as typeof process.kill; + + try { + await expect( + runManagedCommand({ + bin: process.execPath, + args: ["-e", "process.exit(0)"], + onReady: (child) => { + childPid = expectProcessPid(child.pid); + }, + requireProcessTreeExit: true, + shell: false, + stdio: "ignore", + timeoutMs: 1_000, + }), + ).resolves.toBe(0); + } finally { + process.kill = originalKill; + } + + expect(injectedLiveGroup).toBe(true); + expect(isProcessAlive(childPid)).toBe(false); + }); + it("allows bounded retry output and normal long-running work to complete", async () => { await expect( runManagedCommand({