diff --git a/src/infra/stale-lock-file.test.ts b/src/infra/stale-lock-file.test.ts index 56c257de6f2..b792895fe7c 100644 --- a/src/infra/stale-lock-file.test.ts +++ b/src/infra/stale-lock-file.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { isPidDefinitelyDead, shouldRemoveDeadOwnerOrExpiredLock } from "./stale-lock-file.js"; +import { shouldRemoveDeadOwnerOrExpiredLock } from "./stale-lock-file.js"; describe("stale lock file ownership", () => { it("treats permission-denied process probes as not definitely dead", () => { @@ -27,8 +27,4 @@ describe("stale lock file ownership", () => { }), ).toBe(true); }); - - it("treats invalid pids as definitely dead", () => { - expect(isPidDefinitelyDead(-1)).toBe(true); - }); }); diff --git a/src/infra/stale-lock-file.ts b/src/infra/stale-lock-file.ts index 7d4ef534cbe..d59f3025309 100644 --- a/src/infra/stale-lock-file.ts +++ b/src/infra/stale-lock-file.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import { isPidDefinitelyDead as defaultIsPidDefinitelyDead } from "../shared/pid-alive.js"; export type LockFileSnapshot = { raw: string; @@ -55,7 +56,7 @@ export function shouldRemoveDeadOwnerOrExpiredLock(params: { }): boolean { const payload = readLockFileOwnerPayload(params.payload); if (payload?.pid) { - return (params.isPidDefinitelyDead ?? isPidDefinitelyDead)(payload.pid); + return (params.isPidDefinitelyDead ?? defaultIsPidDefinitelyDead)(payload.pid); } if (payload?.createdAt) { const createdAt = Date.parse(payload.createdAt); @@ -64,18 +65,6 @@ export function shouldRemoveDeadOwnerOrExpiredLock(params: { return false; } -export function isPidDefinitelyDead(pid: number): boolean { - if (!Number.isInteger(pid) || pid <= 0) { - return true; - } - try { - process.kill(pid, 0); - return false; - } catch (err) { - return (err as NodeJS.ErrnoException).code === "ESRCH"; - } -} - export async function removeLockFileIfSnapshotMatches(params: { lockPath: string; snapshot: LockFileSnapshot; diff --git a/src/shared/pid-alive.test.ts b/src/shared/pid-alive.test.ts index 36df219e2c3..f26cd7f5b6c 100644 --- a/src/shared/pid-alive.test.ts +++ b/src/shared/pid-alive.test.ts @@ -1,6 +1,10 @@ import fsSync from "node:fs"; -import { describe, expect, it, vi } from "vitest"; -import { getProcessStartTime, isPidAlive } from "./pid-alive.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { getProcessStartTime, isPidAlive, isPidDefinitelyDead } from "./pid-alive.js"; + +afterEach(() => { + vi.restoreAllMocks(); +}); function mockProcReads(entries: Record) { const originalReadFileSync = fsSync.readFileSync; @@ -80,6 +84,60 @@ describe("isPidAlive", () => { }); }); +describe("isPidDefinitelyDead", () => { + it("returns true for invalid PIDs", () => { + expect(isPidDefinitelyDead(0)).toBe(true); + expect(isPidDefinitelyDead(-1)).toBe(true); + expect(isPidDefinitelyDead(1.5)).toBe(true); + expect(isPidDefinitelyDead(Number.NaN)).toBe(true); + expect(isPidDefinitelyDead(Number.POSITIVE_INFINITY)).toBe(true); + }); + + it("returns true when process probing reports ESRCH", () => { + const error = Object.assign(new Error("missing process"), { code: "ESRCH" }); + vi.spyOn(process, "kill").mockImplementation(() => { + throw error; + }); + + expect(isPidDefinitelyDead(42)).toBe(true); + expect(process.kill).toHaveBeenCalledWith(42, 0); + }); + + it("returns false when process probing reports EPERM", () => { + const error = Object.assign(new Error("permission denied"), { code: "EPERM" }); + vi.spyOn(process, "kill").mockImplementation(() => { + throw error; + }); + + expect(isPidDefinitelyDead(42)).toBe(false); + expect(process.kill).toHaveBeenCalledWith(42, 0); + }); + + it("returns true for zombie processes on Linux", async () => { + const zombiePid = process.pid; + vi.spyOn(process, "kill").mockImplementation(() => true); + mockProcReads({ + [`/proc/${zombiePid}/status`]: `Name:\tnode\nUmask:\t0022\nState:\tZ (zombie)\nTgid:\t${zombiePid}\nPid:\t${zombiePid}\n`, + }); + + await withLinuxProcessPlatform(async () => { + expect(isPidDefinitelyDead(zombiePid)).toBe(true); + }); + }); + + it("returns false for live non-zombie processes", async () => { + const livePid = process.pid; + vi.spyOn(process, "kill").mockImplementation(() => true); + mockProcReads({ + [`/proc/${livePid}/status`]: `Name:\tnode\nUmask:\t0022\nState:\tS (sleeping)\nTgid:\t${livePid}\nPid:\t${livePid}\n`, + }); + + await withLinuxProcessPlatform(async () => { + expect(isPidDefinitelyDead(livePid)).toBe(false); + }); + }); +}); + describe("getProcessStartTime", () => { it("parses linux /proc stat start times and rejects malformed variants", async () => { const fakeStatPrefix = "42 (node) S 1 42 42 0 -1 4194304 12345 0 0 0 100 50 0 0 20 0 8 0 "; diff --git a/src/shared/pid-alive.ts b/src/shared/pid-alive.ts index 522566fb3fd..826ba2eebeb 100644 --- a/src/shared/pid-alive.ts +++ b/src/shared/pid-alive.ts @@ -36,6 +36,18 @@ export function isPidAlive(pid: number): boolean { return true; } +export function isPidDefinitelyDead(pid: number): boolean { + if (!isValidPid(pid)) { + return true; + } + try { + process.kill(pid, 0); + } catch (err) { + return (err as NodeJS.ErrnoException).code === "ESRCH"; + } + return isZombieProcess(pid); +} + /** * Read the process start time (field 22 "starttime") from /proc//stat. * Returns the value in clock ticks since system boot, or null on non-Linux