From af497642062c2fb8c7a43c72568926f0e5477187 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 13 Aug 2026 08:46:22 +0800 Subject: [PATCH] fix(ci): ignore zombie-only Vitest process groups (#122759) --- scripts/vitest-process-group.mts | 163 ++++++++++++-- test/scripts/run-vitest.test.ts | 40 ++-- test/scripts/vitest-process-group.test.ts | 253 +++++++++++++++++++++- 3 files changed, 418 insertions(+), 38 deletions(-) diff --git a/scripts/vitest-process-group.mts b/scripts/vitest-process-group.mts index 379546602bd1..49f2e3fdb7d5 100644 --- a/scripts/vitest-process-group.mts +++ b/scripts/vitest-process-group.mts @@ -1,5 +1,6 @@ // Shared Vitest child process-group signal forwarding helpers. import { execFileSync, type ChildProcess } from "node:child_process"; +import fs from "node:fs"; type VitestProcessSignal = "SIGINT" | "SIGKILL" | "SIGTERM"; type KillProcess = (pid: number, signal?: VitestProcessSignal | 0) => boolean; @@ -101,27 +102,141 @@ function isVitestProcessGroupAlive(target: number, kill: KillProcess) { } } -export function parseVitestProcessGroupMembers(output: string, processGroupId: number): string { - const members: string[] = []; - for (const line of output.split(/\r?\n/)) { - const match = /^\s*(\d+)\s+(\d+)\s+(\d+)\s+(\S+)\s+(.+?)\s*$/.exec(line); - if (!match || Number(match[3]) !== processGroupId) { - continue; - } - members.push( - `pid=${match[1]} ppid=${match[2]} state=${match[4]} comm=${match[5]?.slice(0, 80)}`, - ); - if (members.length >= 20) { - break; - } +function parseLinuxProcStat(raw: string, expectedId: number) { + const head = /^([1-9]\d*) \(/.exec(raw); + const end = raw.lastIndexOf(") "); + if (!head || end < head[0].length || Number(head[1]) !== expectedId) { + return undefined; } - return members.length > 0 ? members.join("; ") : "none"; + const suffix = raw.slice(end + 2).trim(); + const fields = suffix.split(/\s+/); + const state = fields[0] ?? "", + ppid = Number(fields[1]), + pgid = Number(fields[2]); + if ( + !/^[A-Za-z]$/.test(state) || + ![ppid, pgid].every((value) => Number.isSafeInteger(value) && value >= 0) + ) { + return undefined; + } + const comm = raw + .slice(head[0].length, end) + .replace(/\p{Cc}+/gu, " ") + .trim() + .slice(0, 80); + return { comm, pgid, ppid, state }; } -function inspectVitestProcessGroup(processGroupId: number, platform: NodeJS.Platform): string { - if (platform === "win32") { - return "unavailable"; +function inspectLinuxVitestProcessGroup(processGroupId: number) { + let pids: string[]; + try { + const mounts = fs + .readFileSync("/proc/self/mounts", "utf8") + .trimEnd() + .split(/\r?\n/) + .map((line) => line.split(" ")); + const procMounts = mounts.filter((fields) => fields[1] === "/proc" && fields[2] === "proc"); + const options = procMounts[0]?.[3]?.split(",") ?? []; + const restricted = options.some((option) => + /^(?:pidns=|hidepid=(?!0$|off$)|subset=(?!pid$))/u.test(option), + ); + if (mounts.some((fields) => fields.length < 6) || procMounts.length !== 1 || restricted) { + return { stopped: false, diagnostics: "unavailable" }; + } + pids = fs + .readdirSync("/proc") + .filter((entry) => /^[1-9]\d*$/.test(entry)) + .toSorted((left, right) => Number(left) - Number(right)); + } catch { + return { stopped: false, diagnostics: "unavailable" }; } + let matching = 0, + allStopped = true; + const diagnostics: string[] = []; + processes: for (const pid of pids) { + try { + const leader = parseLinuxProcStat(fs.readFileSync(`/proc/${pid}/stat`, "utf8"), Number(pid)); + if (!leader) { + return { stopped: false, diagnostics: "unavailable" }; + } + if (leader.pgid !== processGroupId) { + continue; + } + } catch (error) { + if (errorCode(error) !== "ENOENT") { + return { stopped: false, diagnostics: "unavailable" }; + } + continue; + } + + const parsedTids = new Set(); + const taskRoot = `/proc/${pid}/task`; + for (let scan = 0; scan < 2; scan += 1) { + let tids: string[]; + try { + tids = fs.readdirSync(taskRoot).toSorted((left, right) => Number(left) - Number(right)); + if ( + tids.length === 0 || + tids.some((tid) => !/^[1-9]\d*$/.test(tid) || (scan === 1 && !parsedTids.has(tid))) + ) { + return { stopped: false, diagnostics: "unavailable" }; + } + } catch (error) { + if (errorCode(error) !== "ENOENT") { + return { stopped: false, diagnostics: "unavailable" }; + } + try { + fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + } catch (leaderError) { + if (errorCode(leaderError) === "ENOENT") { + continue processes; + } + } + return { stopped: false, diagnostics: "unavailable" }; + } + if (scan === 1) { + continue; + } + for (const tid of tids) { + try { + const task = parseLinuxProcStat( + fs.readFileSync(`${taskRoot}/${tid}/stat`, "utf8"), + Number(tid), + ); + if (!task || task.pgid !== processGroupId) { + return { stopped: false, diagnostics: "unavailable" }; + } + parsedTids.add(tid); + matching += 1; + allStopped &&= task.state === "Z" || task.state === "X"; + if (diagnostics.length < 20) { + diagnostics.push( + `pid=${pid} tid=${tid} ppid=${task.ppid} state=${task.state} comm=${task.comm}`, + ); + } + } catch (error) { + if (errorCode(error) !== "ENOENT") { + return { stopped: false, diagnostics: "unavailable" }; + } + } + } + } + } + return { stopped: matching > 0 && allStopped, diagnostics: diagnostics.join("; ") || "none" }; +} + +export function parseVitestProcessGroupMembers(output: string, processGroupId: number): string { + const members = output.split(/\r?\n/).flatMap((line) => { + const match = /^\s*(\d+)\s+(\d+)\s+(\d+)\s+(\S+)\s+(.+?)\s*$/.exec(line); + if (!match || Number(match[3]) !== processGroupId) { + return []; + } + return [`pid=${match[1]} ppid=${match[2]} state=${match[4]} comm=${match[5]?.slice(0, 80)}`]; + }); + return members.slice(0, 20).join("; ") || "none"; +} + +function inspectVitestProcessGroup(processGroupId: number): string { try { const output = execFileSync("ps", ["-axo", "pid=,ppid=,pgid=,stat=,comm="], { encoding: "utf8", @@ -145,10 +260,19 @@ async function joinVitestProcessGroup( } forwardSignalToVitestProcessGroup({ child, kill, platform, signal: "SIGKILL" }); const deadlineAt = Date.now() + PROCESS_GROUP_JOIN_TIMEOUT_MS; - while (isVitestProcessGroupAlive(target, kill)) { + let alive = isVitestProcessGroupAlive(target, kill); + if (alive && platform === "linux" && inspectLinuxVitestProcessGroup(child.pid!).stopped) { + return; + } + while (alive) { const remainingMs = deadlineAt - Date.now(); if (remainingMs <= 0) { - const members = inspectVitestProcessGroup(child.pid!, platform); + const inspection = + platform === "linux" ? inspectLinuxVitestProcessGroup(child.pid!) : undefined; + if (inspection?.stopped || !isVitestProcessGroupAlive(target, kill)) { + return; + } + const members = inspection?.diagnostics ?? inspectVitestProcessGroup(child.pid!); throw new Error( `[vitest] process group ${child.pid ?? "unknown"} remained alive ${PROCESS_GROUP_JOIN_TIMEOUT_MS}ms after SIGKILL; members: ${members}.`, ); @@ -156,6 +280,7 @@ async function joinVitestProcessGroup( await new Promise((resolve) => { setTimeout(resolve, Math.min(25, remainingMs)); }); + alive = isVitestProcessGroupAlive(target, kill); } } diff --git a/test/scripts/run-vitest.test.ts b/test/scripts/run-vitest.test.ts index 9bbc06a69326..3348a9b7930a 100644 --- a/test/scripts/run-vitest.test.ts +++ b/test/scripts/run-vitest.test.ts @@ -913,7 +913,7 @@ describe("scripts/run-vitest", () => { } }); - posixIt("reaps residual process-group descendants before completing", async () => { + posixIt("stops residual process-group descendants before completing", async () => { const descendantPidPath = nodePath.join( os.tmpdir(), `openclaw-run-vitest-residual-${process.pid}-${Date.now()}.pid`, @@ -959,19 +959,34 @@ describe("scripts/run-vitest", () => { process.kill(watched.child.pid!, "SIGTERM"); const snapshot = await Promise.race([ - watched.completion.then((result) => ({ - descendantAlive: isProcessAlive(descendantPid), - groupAlive: isProcessGroupAlive(watched.child.pid!), - result, - })), + watched.completion.then((result) => { + const psArgs = + process.platform === "linux" ? ["-eL", "-o", "pgid=,state="] : ["-axo", "pgid=,state="]; + const stateResult = spawnSync("ps", psArgs, { + encoding: "utf8", + }); + const rows = stateResult.stdout + .split(/\r?\n/) + .filter(Boolean) + .map((line) => /^\s*(\d+)\s+(\S+)\s*$/.exec(line)); + const groupStopped = + !stateResult.error && + stateResult.signal === null && + stateResult.stderr.trim() === "" && + stateResult.status === 0 && + rows.every(Boolean) && + rows + .filter((row) => Number(row?.[1]) === watched.child.pid) + .every((row) => /^[ZX]/.test(row?.[2] ?? "")); + return { groupStopped, result }; + }), delay(LOAD_SENSITIVE_PROCESS_TIMEOUT_MS, undefined, { ref: false }).then(() => { throw new Error("timed out waiting for watched Vitest completion"); }), ]); expect(snapshot).toEqual({ - descendantAlive: false, - groupAlive: false, + groupStopped: true, result: { code: 0, signal: null }, }); } finally { @@ -1260,12 +1275,3 @@ function isProcessAlive(pid: number) { return false; } } - -function isProcessGroupAlive(pgid: number) { - try { - process.kill(-pgid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException).code === "EPERM"; - } -} diff --git a/test/scripts/vitest-process-group.test.ts b/test/scripts/vitest-process-group.test.ts index d102022ebefd..70b60255b42a 100644 --- a/test/scripts/vitest-process-group.test.ts +++ b/test/scripts/vitest-process-group.test.ts @@ -1,6 +1,6 @@ -// Vitest Process Group tests cover vitest process group script behavior. import { EventEmitter } from "node:events"; -import { describe, expect, it, vi } from "vitest"; +import fs from "node:fs"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createVitestProcessCompletion, forwardSignalToVitestProcessGroup, @@ -11,6 +11,77 @@ import { } from "../../scripts/vitest-process-group.mts"; describe("vitest process group helpers", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + function procStat(pid: number, state: string, ppid: number, pgid: number, comm = "node") { + return `${pid} (${comm}) ${state} ${ppid} ${pgid} 0`; + } + + function mockLinuxProc( + pids: string[], + stats: Record, + listError?: NodeJS.ErrnoException, + mounts: string | NodeJS.ErrnoException = "proc /proc proc rw 0 0\n", + taskLists: Record = {}, + ) { + const taskReads = new Map(); + vi.spyOn(fs, "readdirSync").mockImplementation((path) => { + if (String(path) === "/proc") { + if (listError) { + throw listError; + } + return pids as never; + } + const pid = /^\/proc\/(\d+)\/task$/.exec(String(path))?.[1] ?? ""; + const lists = taskLists[pid] ?? [[pid]]; + const index = taskReads.get(pid) ?? 0; + taskReads.set(pid, index + 1); + const result = lists[Math.min(index, lists.length - 1)]; + if (result instanceof Error) { + throw result; + } + return result as never; + }); + vi.spyOn(fs, "readFileSync").mockImplementation((file) => { + if (String(file) === "/proc/self/mounts") { + if (mounts instanceof Error) { + throw mounts; + } + return mounts; + } + const task = /^\/proc\/(\d+)\/task\/(\d+)\/stat$/.exec(String(file)); + const pid = /^\/proc\/(\d+)\/stat$/.exec(String(file))?.[1] ?? ""; + const taskStat = task ? stats[`${task[1]}/${task[2]}`] : undefined; + const stat = taskStat ?? stats[task && task[1] === task[2] ? task[1]! : pid]; + if (stat instanceof Error) { + throw stat; + } + if (typeof stat !== "string") { + throw new Error(`missing mocked stat for ${pid}`); + } + return stat; + }); + } + + function startLinuxCompletion( + pid = 4200, + kill: (pid: number, signal?: NodeJS.Signals | 0) => boolean = vi.fn(() => true), + ) { + const child = Object.assign(new EventEmitter(), { pid }); + const completion = createVitestProcessCompletion({ + child: child as never, + detached: true, + platform: "linux", + kill, + }); + child.emit("exit", 0, null); + child.emit("close", 0, null); + return { completion, kill }; + } + function getListenerSet(listeners: Map void>>, event: string) { const set = listeners.get(event); if (!set) { @@ -52,6 +123,184 @@ describe("vitest process group helpers", () => { ).toBe("pid=116 ppid=1 state=Z comm=node; pid=117 ppid=1 state=Sl comm=claude"); }); + it.each(["rw", "rw,hidepid=0", "rw,hidepid=off"])( + "accepts a complete zombie-only Linux process group with proc options %s", + async (mountOptions) => { + mockLinuxProc( + ["4200"], + { + "4200": procStat(4200, "Z", 1, 4200, "node (vitest)"), + "4200/4200": procStat(4200, "Z", 1, 4200, "node (vitest)"), + "4200/4201": procStat(4201, "X", 1, 4200, "worker"), + }, + undefined, + `proc /proc proc ${mountOptions} 0 0\n`, + { "4200": [["4201", "4200"]] }, + ); + + const { completion } = startLinuxCompletion(); + + await expect(completion).resolves.toEqual({ code: 0, signal: null }); + }, + ); + + const missingTask = Object.assign(new Error("gone"), { code: "ENOENT" }); + const taskCases: [ + string, + (string[] | NodeJS.ErrnoException)[], + string | NodeJS.ErrnoException | undefined, + string | undefined, + ][] = [ + ["runnable worker", [["4200", "4201"]], procStat(4201, "S", 1, 4200), "tid=4201"], + ["mismatched TID", [["4200", "4201"]], procStat(4202, "Z", 1, 4200), "unavailable"], + ["mismatched PGID", [["4200", "4201"]], procStat(4201, "Z", 1, 999), "unavailable"], + [ + "inaccessible task dir", + [Object.assign(new Error("denied"), { code: "EACCES" })], + undefined, + "unavailable", + ], + ["empty task dir", [[]], undefined, "unavailable"], + ["non-numeric task dir", [["4200", "worker"]], undefined, "unavailable"], + ["missing task dir with leader", [missingTask], undefined, "unavailable"], + ["disappeared TID", [["4200", "4201"], ["4200"]], missingTask, undefined], + ["still-present TID", [["4200", "4201"]], missingTask, "unavailable"], + ["new TID", [["4200"], ["4200", "4201"]], missingTask, "unavailable"], + ]; + + it.each(taskCases)("handles a %s fail-closed", async (_label, taskLists, workerStat, failure) => { + if (failure) { + vi.useFakeTimers(); + } + mockLinuxProc( + ["4200"], + { + "4200": procStat(4200, "Z", 1, 4200), + "4200/4200": procStat(4200, "Z", 1, 4200), + ...(workerStat ? { "4200/4201": workerStat } : {}), + }, + undefined, + undefined, + { "4200": taskLists }, + ); + const completion = startLinuxCompletion().completion; + if (!failure) { + await expect(completion).resolves.toEqual({ code: 0, signal: null }); + return; + } + const rejected = expect(completion).rejects.toThrow(failure); + + await vi.advanceTimersByTimeAsync(1_000); + await rejected; + }); + + it.each([ + ["hidepid=2", "proc /proc proc rw,hidepid=2 0 0\n"], + ["hidepid=invisible", "proc /proc proc rw,hidepid=invisible 0 0\n"], + ["hidepid=4", "proc /proc proc rw,hidepid=4 0 0\n"], + ["pid namespace", "proc /proc proc rw,pidns=host 0 0\n"], + ["missing proc mount", "tmpfs /tmp tmpfs rw 0 0\n"], + ["unreadable mounts", Object.assign(new Error("denied"), { code: "EACCES" })], + ])("fails closed before PID scans for %s", async (_label, mounts) => { + vi.useFakeTimers(); + mockLinuxProc(["4200"], { "4200": procStat(4200, "Z", 1, 4200) }, undefined, mounts); + const rejected = expect(startLinuxCompletion().completion).rejects.toThrow( + "members: unavailable", + ); + + await vi.advanceTimersByTimeAsync(1_000); + await rejected; + expect(fs.readdirSync).not.toHaveBeenCalled(); + }); + + it.each([ + ["already gone", 0], + ["gone during deadline inspection", 2], + ])("accepts a Linux process group that is %s", async (_label, scansBeforeGone) => { + if (scansBeforeGone > 0) { + vi.useFakeTimers(); + } + mockLinuxProc([], {}); + const missing = Object.assign(new Error("gone"), { code: "ESRCH" }); + const kill = vi.fn((_target: number, signal?: NodeJS.Signals | 0) => { + const scans = vi + .mocked(fs.readdirSync) + .mock.calls.filter(([path]) => String(path) === "/proc").length; + if (signal === 0 && scans >= scansBeforeGone) { + throw missing; + } + return true; + }); + + const { completion } = startLinuxCompletion(4200, kill); + const settled = expect(completion).resolves.toEqual({ code: 0, signal: null }); + + if (scansBeforeGone > 0) { + await vi.advanceTimersByTimeAsync(1_000); + } + await settled; + expect( + vi.mocked(fs.readdirSync).mock.calls.filter(([path]) => String(path) === "/proc"), + ).toHaveLength(scansBeforeGone); + }); + + it("skips ENOENT races and accepts PID/PGID 1 with PPID 0", async () => { + const missing = Object.assign(new Error("gone"), { code: "ENOENT" }); + mockLinuxProc(["2", "1"], { + "1": procStat(1, "Z", 0, 1, "init"), + "2": missing, + }); + + const { completion } = startLinuxCompletion(1); + + await expect(completion).resolves.toEqual({ code: 0, signal: null }); + }); + + it.each([ + ["an empty snapshot", [], {}, undefined], + ["a runnable member", ["4200"], { "4200": procStat(4200, "S", 1, 4200) }, undefined], + ["a malformed stat", ["4200"], { "4200": "malformed" }, undefined], + ["a mismatched stat PID", ["4200"], { "4200": procStat(4201, "Z", 1, 4200) }, undefined], + [ + "a non-ENOENT read failure", + ["4200"], + { "4200": Object.assign(new Error("denied"), { code: "EACCES" }) }, + undefined, + ], + ["unavailable proc", [], {}, Object.assign(new Error("missing"), { code: "EACCES" })], + ])("fails closed for %s", async (_label, pids, stats, listError) => { + vi.useFakeTimers(); + mockLinuxProc(pids, stats, listError); + const { completion } = startLinuxCompletion(); + const rejected = expect(completion).rejects.toThrow("process group 4200 remained alive 1000ms"); + + await vi.advanceTimersByTimeAsync(1_000); + await rejected; + }); + + it("sorts and bounds sanitized Linux process-group diagnostics", async () => { + vi.useFakeTimers(); + const pids = Array.from({ length: 22 }, (_, index) => String(4200 + index)).toReversed(); + const comm = `bad\n\t${"x".repeat(100)}`; + mockLinuxProc( + pids, + Object.fromEntries(pids.map((pid) => [pid, procStat(Number(pid), "S", 1, 4200, comm)])), + ); + const { completion } = startLinuxCompletion(); + const error = await (async () => { + const rejected = completion.catch((failure: unknown) => failure); + await vi.advanceTimersByTimeAsync(1_000); + return rejected; + })(); + + const message = (error as Error).message; + expect(message.indexOf("pid=4200")).toBeLessThan(message.indexOf("pid=4201")); + expect(message).toContain("pid=4219"); + expect(message).not.toContain("pid=4220"); + expect(message).toContain(`comm=bad ${"x".repeat(76)}`); + expect(message).not.toContain("\n"); + }); + it("forwards signals to the computed target and ignores cleanup races", () => { const kill = vi.fn(); expect(