From b9fb56566b90c3370b7df5c2c8a0905e31bb9d2d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 07:32:02 -0700 Subject: [PATCH] fix(browser): treat Linux zombies as exited (#125831) --- config/assertion-safety-baseline.txt | 2 +- extensions/acpx/src/process-reaper.ts | 13 +- .../src/browser/chrome.internal.test.ts | 137 +++++++++++++++++- extensions/browser/src/browser/chrome.ts | 48 +----- src/plugin-sdk/process-runtime.ts | 1 + 5 files changed, 148 insertions(+), 53 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index bb6ac632a980..9855a2a7dfc8 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -67,7 +67,7 @@ extensions/browser/src/browser/chrome-mcp-tabs.ts 1 extensions/browser/src/browser/chrome.diagnostics.ts 2 extensions/browser/src/browser/chrome.executables.ts 1 extensions/browser/src/browser/chrome.profile-decoration.ts 1 -extensions/browser/src/browser/chrome.ts 5 +extensions/browser/src/browser/chrome.ts 4 extensions/browser/src/browser/client-fetch.ts 2 extensions/browser/src/browser/config-mutations.ts 1 extensions/browser/src/browser/config.ts 1 diff --git a/extensions/acpx/src/process-reaper.ts b/extensions/acpx/src/process-reaper.ts index 92e87645223f..834f1e000257 100644 --- a/extensions/acpx/src/process-reaper.ts +++ b/extensions/acpx/src/process-reaper.ts @@ -4,7 +4,7 @@ */ import { createRequire } from "node:module"; import path from "node:path"; -import { runExec } from "openclaw/plugin-sdk/process-runtime"; +import { isPidAlive, runExec } from "openclaw/plugin-sdk/process-runtime"; import { CODEX_ACP_PACKAGE, LEGACY_CODEX_ACP_PACKAGE } from "./codex-adapter.js"; import { splitCommandParts } from "./command-line.js"; import { resolveAcpxPluginRoot } from "./config.js"; @@ -304,15 +304,6 @@ function uniquePids(processes: AcpxProcessInfo[]): number[] { ); } -function isProcessAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch { - return false; - } -} - async function terminatePids( pids: number[], deps: AcpxProcessCleanupDeps | undefined, @@ -339,7 +330,7 @@ async function terminatePids( } await sleep(750); for (const pid of terminated) { - if (deps?.killProcess || isProcessAlive(pid)) { + if (deps?.killProcess || isPidAlive(pid)) { try { killProcess(pid, "SIGKILL"); } catch { diff --git a/extensions/browser/src/browser/chrome.internal.test.ts b/extensions/browser/src/browser/chrome.internal.test.ts index 4c8070677a7c..4dc488daf0a7 100644 --- a/extensions/browser/src/browser/chrome.internal.test.ts +++ b/extensions/browser/src/browser/chrome.internal.test.ts @@ -1,5 +1,6 @@ // Browser tests cover chrome.internal plugin behavior. -import { EventEmitter } from "node:events"; +import { execFile } from "node:child_process"; +import { EventEmitter, once } from "node:events"; import fs from "node:fs"; import fsp from "node:fs/promises"; import { createServer } from "node:http"; @@ -179,6 +180,53 @@ function deferred() { return { promise, reject, resolve }; } +async function startLinuxZombieProcess(): Promise<{ pid: number; reap: () => Promise }> { + const parent = execFile("python3", [ + "-c", + [ + "import os, sys", + "pid = os.fork()", + "if pid == 0:", + " os._exit(0)", + "print(pid, flush=True)", + "sys.stdin.readline()", + "os.waitpid(pid, 0)", + ].join("\n"), + ]); + const closed = once(parent, "close"); + const pid = await new Promise((resolve, reject) => { + const onError = (err: Error) => reject(err); + parent.once("error", onError); + parent.stdout?.once("data", (chunk) => { + parent.off("error", onError); + resolve(Number.parseInt(String(chunk).trim(), 10)); + }); + }); + const deadline = Date.now() + 5_000; + while (Date.now() < deadline) { + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + if (stat.slice(stat.lastIndexOf(")") + 2).startsWith("Z ")) { + return { + pid, + reap: async () => { + parent.stdin?.end(); + await closed; + }, + }; + } + } catch { + // The child may not have reached zombie state yet. + } + await new Promise((resolve) => { + setTimeout(resolve, 10); + }); + } + parent.stdin?.end(); + await closed; + throw new Error(`child ${pid} did not enter zombie state`); +} + function linuxProcStatLine(pid: number, startTime: string): string { const fieldsAfterCommand = [ "S", @@ -981,6 +1029,93 @@ describe("chrome.ts internal", () => { }); }); + it.runIf(process.platform === "linux")( + "recovers a current-host profile locked by a zombie process", + async () => { + const zombie = await startLinuxZombieProcess(); + try { + let cdpReachable = false; + const originalFetch = globalThis.fetch; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + if (!cdpReachable) { + throw new Error("ECONNREFUSED"); + } + return await originalFetch(input, init); + }), + ); + const executablePath = path.join(tmpDir, "chrome"); + await fsp.writeFile(executablePath, ""); + const existsSync = fs.existsSync.bind(fs); + vi.spyOn(fs, "existsSync").mockImplementation((candidate) => { + const value = String(candidate); + if (value.endsWith("Local State") || value.endsWith("Preferences")) { + return true; + } + return existsSync(candidate); + }); + + const firstProc = makeFakeProc(); + const secondProc = makeFakeProc(); + let spawnCalls = 0; + mockExpiredLaunchPollingClock(); + spawnMock.mockImplementation(() => { + spawnCalls += 1; + if (spawnCalls === 1) { + queueMicrotask(() => { + firstProc.stderr.emit( + "data", + Buffer.from("The profile appears to be in use by another Chromium process"), + ); + }); + return firstProc; + } + cdpReachable = true; + return secondProc; + }); + + await withMockChromeCdpServer({ + wsPath: "/devtools/browser/ZOMBIE_SINGLETON_RETRY", + run: async (baseUrl) => { + const port = Number(new URL(baseUrl).port); + const profile = { + ...makeProfile(port), + cdpUrl: baseUrl, + executablePath, + } as ResolvedBrowserProfile; + const userDataDir = resolveOpenClawUserDataDir(profile.name); + await fsp.mkdir(userDataDir, { recursive: true }); + await fsp.writeFile(path.join(userDataDir, "SingletonCookie"), "cookie"); + await fsp.writeFile(path.join(userDataDir, "SingletonSocket"), "socket"); + await fsp.symlink( + `${os.hostname()}-${zombie.pid}`, + path.join(userDataDir, "SingletonLock"), + ); + + try { + const running = await launchOpenClawChrome( + makeResolved({ localLaunchTimeoutMs: 20 }), + profile, + ); + expect(running.proc).toBe(secondProc); + expect(firstProc.kill).toHaveBeenCalledWith("SIGKILL"); + expect(spawnCalls).toBe(2); + expect(fs.existsSync(path.join(userDataDir, "SingletonLock"))).toBe(false); + expect(fs.existsSync(path.join(userDataDir, "SingletonSocket"))).toBe(false); + running.proc.kill?.("SIGTERM"); + } finally { + await fsp.rm(userDataDir, { recursive: true, force: true }); + } + }, + }); + } finally { + await zombie.reap(); + } + }, + 15_000, + ); + it("preserves the exact surviving child when a singleton retry cleanup fails", async () => { vi.spyOn(fs, "existsSync").mockImplementation((p) => { const value = String(p); diff --git a/extensions/browser/src/browser/chrome.ts b/extensions/browser/src/browser/chrome.ts index 3078a2366567..babb8c69fb96 100644 --- a/extensions/browser/src/browser/chrome.ts +++ b/extensions/browser/src/browser/chrome.ts @@ -10,7 +10,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; -import { prepareOomScoreAdjustedSpawn } from "openclaw/plugin-sdk/process-runtime"; +import { isPidAlive, prepareOomScoreAdjustedSpawn } from "openclaw/plugin-sdk/process-runtime"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { sliceUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import type { SsrFPolicy } from "../infra/net/ssrf.js"; @@ -161,21 +161,6 @@ function createChromeLaunchStderrDiagnostics(maxBytes: number) { }; } -function processExists(pid: number): boolean { - if (!Number.isInteger(pid) || pid <= 0) { - return false; - } - try { - process.kill(pid, 0); - return true; - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EPERM") { - return true; - } - return false; - } -} - function readSingletonLockTarget(userDataDir: string): { hostname: string; pid: number } | null { let target: string; try { @@ -189,9 +174,6 @@ function readSingletonLockTarget(userDataDir: string): { hostname: string; pid: } const hostname = normalizeOptionalString(match.groups.lockHost) ?? ""; const pid = Number.parseInt(match.groups.pid ?? "", 10); - if (!Number.isInteger(pid) || pid <= 0) { - return null; - } return { hostname, pid }; } @@ -436,7 +418,7 @@ function readOwnedManagedChromeIdentity(params: { profile: ResolvedBrowserProfile; userDataDir: string; }): ManagedChromeProcessIdentity | null { - if (!processExists(params.pid) || !pidListensOnPort(params.pid, params.profile.cdpPort)) { + if (!isPidAlive(params.pid) || !pidListensOnPort(params.pid, params.profile.cdpPort)) { return null; } const command = readManagedProcessCommandLine(params.pid); @@ -471,7 +453,7 @@ function isPortInUseError(err: unknown): boolean { function readCurrentHostSingletonPid(userDataDir: string, hostname = os.hostname()): number | null { const lock = readSingletonLockTarget(userDataDir); - if (!lock || lock.hostname !== hostname || !processExists(lock.pid)) { + if (!lock || lock.hostname !== hostname || !isPidAlive(lock.pid)) { return null; } return lock.pid; @@ -489,22 +471,8 @@ function clearChromeSingletonArtifacts(userDataDir: string) { /** Remove stale Chrome singleton lock files from a user-data-dir. */ function clearStaleChromeSingletonLocks(userDataDir: string, hostname = os.hostname()): boolean { - const lockPath = path.join(userDataDir, "SingletonLock"); - let target: string; - try { - target = fs.readlinkSync(lockPath); - } catch { - return false; - } - - const match = /^(?.+)-(?\d+)$/.exec(target); - if (!match?.groups) { - return false; - } - - const lockHost = normalizeOptionalString(match.groups.lockHost) ?? ""; - const pid = Number.parseInt(match.groups.pid ?? "", 10); - if (lockHost === hostname && processExists(pid)) { + const lock = readSingletonLockTarget(userDataDir); + if (!lock || (lock.hostname === hostname && isPidAlive(lock.pid))) { return false; } @@ -567,14 +535,14 @@ async function terminateChromeForRetry(proc: ChildProcess, userDataDir: string): async function waitForPidExit(pid: number, timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - if (!processExists(pid)) { + if (!isPidAlive(pid)) { return true; } await new Promise((resolve) => { setTimeout(resolve, CHROME_BOOTSTRAP_EXIT_POLL_MS); }); } - return !processExists(pid); + return !isPidAlive(pid); } async function terminateOwnedStaleChromeProcess( @@ -619,7 +587,7 @@ async function terminateOwnedStaleChromeProcess( function clearRecoveredChromeSingletonArtifacts(userDataDir: string, pid: number): boolean { const lock = readSingletonLockTarget(userDataDir); - if (!lock || lock.hostname !== os.hostname() || lock.pid !== pid || processExists(pid)) { + if (!lock || lock.hostname !== os.hostname() || lock.pid !== pid || isPidAlive(pid)) { return false; } clearChromeSingletonArtifacts(userDataDir); diff --git a/src/plugin-sdk/process-runtime.ts b/src/plugin-sdk/process-runtime.ts index d205d74850b5..2f8302a2638d 100644 --- a/src/plugin-sdk/process-runtime.ts +++ b/src/plugin-sdk/process-runtime.ts @@ -12,3 +12,4 @@ export { } from "../process/exec.js"; export { prepareOomScoreAdjustedSpawn } from "../process/linux-oom-score.js"; export type { OomScoreAdjustedSpawn, OomWrapOptions } from "../process/linux-oom-score.js"; +export { isPidAlive } from "../shared/pid-alive.js";