From dafb486819fb2fe66c9187c57d59dfd368443453 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 15 Jul 2026 14:45:56 +0200 Subject: [PATCH] fix(server): make stale lock recovery upgrade-safe Only heartbeat-capable locks age out by default. Desktop-managed startup may reclaim legacy locks after its reachability probe, while supervisors shut down if heartbeat detects ownership loss. --- .../server/scripts/supervisor-entrypoint.ts | 12 +- packages/server/scripts/supervisor.ts | 8 +- packages/server/src/server/pid-lock.test.ts | 91 ++++++++- packages/server/src/server/pid-lock.ts | 184 +++++++++++++----- 4 files changed, 242 insertions(+), 53 deletions(-) diff --git a/packages/server/scripts/supervisor-entrypoint.ts b/packages/server/scripts/supervisor-entrypoint.ts index fced9b0e56..c6c1491227 100644 --- a/packages/server/scripts/supervisor-entrypoint.ts +++ b/packages/server/scripts/supervisor-entrypoint.ts @@ -110,6 +110,7 @@ async function main(): Promise { try { await acquirePidLock(paseoHome, null, { ownerPid: process.pid, + reclaimLegacyDesktopLock: process.env.PASEO_DESKTOP_MANAGED === "1", }); } catch (error) { if (error instanceof PidLockError) { @@ -121,8 +122,16 @@ async function main(): Promise { } let lockReleased = false; + let requestSupervisorShutdown: ((reason: string) => void) | null = null; const stopLockHeartbeat = startPidLockHeartbeat(paseoHome, { ownerPid: process.pid, + onError: (error) => { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`PID lock heartbeat failed: ${message}\n`); + if (error instanceof PidLockError) { + requestSupervisorShutdown?.("pid_lock_ownership_lost"); + } + }, }); const releaseLock = async (): Promise => { if (lockReleased) { @@ -135,7 +144,7 @@ async function main(): Promise { }); }; - runSupervisor({ + const supervisor = runSupervisor({ name: "DaemonRunner", startupMessage: "Starting daemon worker (IPC restart and crash restart enabled)", resolveWorkerEntry: () => workerEntry, @@ -164,6 +173,7 @@ async function main(): Promise { }, onSupervisorExit: releaseLock, }); + requestSupervisorShutdown = supervisor.requestShutdown; } void main().catch((error) => { diff --git a/packages/server/scripts/supervisor.ts b/packages/server/scripts/supervisor.ts index 6e7c1b0321..4e56c96125 100644 --- a/packages/server/scripts/supervisor.ts +++ b/packages/server/scripts/supervisor.ts @@ -47,6 +47,10 @@ interface SupervisorOptions { logFile?: SupervisorLogFileOptions; } +export interface SupervisorController { + requestShutdown(reason: string): void; +} + function describeExit(code: number | null, signal: NodeJS.Signals | null): string { return signal ?? (typeof code === "number" ? `code ${code}` : "unknown"); } @@ -105,7 +109,7 @@ function createSupervisorLogStream(options: SupervisorLogFileOptions | undefined }); } -export function runSupervisor(options: SupervisorOptions): void { +export function runSupervisor(options: SupervisorOptions): SupervisorController { const restartOnCrash = options.restartOnCrash ?? false; const workerArgs = options.workerArgs ?? process.argv.slice(2); const workerEnv = options.workerEnv ?? process.env; @@ -331,4 +335,6 @@ export function runSupervisor(options: SupervisorOptions): void { process.stdout.write(`[${options.name}] ${options.startupMessage}\n`); writeLifecycleLog(options.startupMessage); spawnWorker(); + + return { requestShutdown }; } diff --git a/packages/server/src/server/pid-lock.test.ts b/packages/server/src/server/pid-lock.test.ts index 9834abe4ec..f37b375b44 100644 --- a/packages/server/src/server/pid-lock.test.ts +++ b/packages/server/src/server/pid-lock.test.ts @@ -3,7 +3,14 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, test } from "vitest"; -import { acquirePidLock, getPidLockInfo, releasePidLock, updatePidLock } from "./pid-lock.js"; +import { + acquirePidLock, + getPidLockInfo, + PidLockError, + refreshPidLock, + releasePidLock, + updatePidLock, +} from "./pid-lock.js"; describe("pid-lock ownership", () => { test("writes and releases lock for explicit owner pid", async () => { @@ -22,6 +29,7 @@ describe("pid-lock ownership", () => { const lock = await getPidLockInfo(paseoHome); expect(lock?.pid).toBe(ownerPid); expect(lock?.listen).toBeNull(); + expect(lock?.heartbeat).toBe(true); await ( updatePidLock as unknown as ( @@ -50,7 +58,7 @@ describe("pid-lock ownership", () => { } }); - test("reclaims a stale lock when the recorded pid is alive but not refreshing it", async () => { + test("reclaims a stale heartbeat lock when the recorded pid is alive", async () => { const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-stale-heartbeat-")); const replacementOwnerPid = process.pid + 10_000; @@ -65,6 +73,7 @@ describe("pid-lock ownership", () => { uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", desktopManaged: true, + heartbeat: true, }), ); const staleTime = new Date(Date.now() - 10 * 60_000); @@ -80,6 +89,83 @@ describe("pid-lock ownership", () => { } }); + test("keeps a stale live lock written by a pre-heartbeat daemon", async () => { + const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-legacy-live-")); + const pidPath = join(paseoHome, "paseo.pid"); + + try { + await writeFile( + pidPath, + JSON.stringify({ + pid: process.pid, + startedAt: "2026-01-01T00:00:00.000Z", + hostname: "old-host", + uid: process.getuid?.() ?? 0, + listen: "127.0.0.1:6767", + desktopManaged: true, + }), + ); + const staleTime = new Date(Date.now() - 10 * 60_000); + await utimes(pidPath, staleTime, staleTime); + + await expect( + acquirePidLock(paseoHome, null, { ownerPid: process.pid + 10_000 }), + ).rejects.toThrow("Another Paseo daemon is already running"); + + const lock = await getPidLockInfo(paseoHome); + expect(lock?.pid).toBe(process.pid); + } finally { + await rm(paseoHome, { recursive: true, force: true }); + } + }); + + test("reclaims a stale legacy desktop lock after desktop confirms the daemon is unreachable", async () => { + const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-legacy-desktop-")); + const replacementOwnerPid = process.pid + 10_000; + const pidPath = join(paseoHome, "paseo.pid"); + + try { + await writeFile( + pidPath, + JSON.stringify({ + pid: process.pid, + startedAt: "2026-01-01T00:00:00.000Z", + hostname: "old-host", + uid: process.getuid?.() ?? 0, + listen: "127.0.0.1:6767", + desktopManaged: true, + }), + ); + const staleTime = new Date(Date.now() - 10 * 60_000); + await utimes(pidPath, staleTime, staleTime); + + await acquirePidLock(paseoHome, null, { + ownerPid: replacementOwnerPid, + reclaimLegacyDesktopLock: true, + }); + + const lock = await getPidLockInfo(paseoHome); + expect(lock?.pid).toBe(replacementOwnerPid); + expect(lock?.heartbeat).toBe(true); + } finally { + await rm(paseoHome, { recursive: true, force: true }); + } + }); + + test("rejects a heartbeat refresh after another supervisor takes ownership", async () => { + const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-refresh-owner-")); + + try { + await acquirePidLock(paseoHome, null, { ownerPid: process.pid + 10_000 }); + + await expect(refreshPidLock(paseoHome, { ownerPid: process.pid })).rejects.toBeInstanceOf( + PidLockError, + ); + } finally { + await rm(paseoHome, { recursive: true, force: true }); + } + }); + test("keeps a fresh lock when the recorded pid is alive", async () => { const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-fresh-heartbeat-")); @@ -93,6 +179,7 @@ describe("pid-lock ownership", () => { uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", desktopManaged: true, + heartbeat: true, }), ); diff --git a/packages/server/src/server/pid-lock.ts b/packages/server/src/server/pid-lock.ts index 1d2c116b19..7a3dd9aee9 100644 --- a/packages/server/src/server/pid-lock.ts +++ b/packages/server/src/server/pid-lock.ts @@ -11,6 +11,7 @@ export const pidLockInfoSchema = z.object({ uid: z.number(), listen: z.string().nullable(), desktopManaged: z.boolean().optional(), + heartbeat: z.literal(true).optional(), }); export interface PidLockInfo extends z.infer {} @@ -81,10 +82,95 @@ function resolveOwnerPid(ownerPid?: number): number { return process.pid; } +interface AcquirePidLockOptions { + ownerPid?: number; + reclaimLegacyDesktopLock?: boolean; +} + +function canReclaimLiveLock( + lock: PidLockInfo, + options: AcquirePidLockOptions | undefined, +): boolean { + if (lock.heartbeat === true) { + return true; + } + + // COMPAT(pidLockHeartbeat): v0.1.108 desktop startup has already confirmed the old daemon is + // unreachable before it launches the supervisor. Remove after 2027-01-15. + return options?.reclaimLegacyDesktopLock === true && lock.desktopManaged === true; +} + +function isSamePidLock(left: PidLockInfo, right: PidLockInfo): boolean { + return left.pid === right.pid && left.startedAt === right.startedAt; +} + +function createLockHeldError(lock: PidLockInfo): PidLockError { + return new PidLockError( + `Another Paseo daemon is already running (PID ${lock.pid}, started ${lock.startedAt})`, + lock, + ); +} + +async function clearExistingPidLock( + pidPath: string, + existingLock: PidLockInfo, + lockOwnerPid: number, + options: AcquirePidLockOptions | undefined, +): Promise<"already_owned" | "cleared"> { + const lockOwnerRunning = isPidRunning(existingLock.pid); + if (existingLock.pid === lockOwnerPid && lockOwnerRunning) { + await touchPidLockFile(pidPath); + return "already_owned"; + } + + if (lockOwnerRunning) { + const reclaimable = canReclaimLiveLock(existingLock, options); + if (!reclaimable || (await isPidLockFresh(pidPath))) { + throw createLockHeldError(existingLock); + } + + // Re-read immediately before unlinking so a heartbeat at the stale boundary wins. + const confirmedLock = await readPidLock(pidPath); + if ( + !confirmedLock || + !isSamePidLock(existingLock, confirmedLock) || + (await isPidLockFresh(pidPath)) + ) { + throw new PidLockError("PID lock changed while checking whether it was abandoned"); + } + } + + await unlink(pidPath).catch(() => {}); + return "cleared"; +} + +async function writeNewPidLock(pidPath: string, lockInfo: PidLockInfo): Promise { + let fd; + try { + fd = await open(pidPath, "wx"); + await fd.write(JSON.stringify(lockInfo)); + } catch (error) { + if (!isErrnoException(error) || error.code !== "EEXIST") { + throw error; + } + + const raceLock = await readPidLock(pidPath); + if (raceLock) { + throw new PidLockError( + `Another Paseo daemon is already running (PID ${raceLock.pid})`, + raceLock, + ); + } + throw new PidLockError("Failed to acquire PID lock due to race condition"); + } finally { + await fd?.close(); + } +} + export async function acquirePidLock( paseoHome: string, listen: string | null, - options?: { ownerPid?: number }, + options?: AcquirePidLockOptions, ): Promise { const pidPath = getPidFilePath(paseoHome); @@ -99,19 +185,10 @@ export async function acquirePidLock( // Check if existing lock is stale const lockOwnerPid = resolveOwnerPid(options?.ownerPid); if (existingLock) { - const lockOwnerRunning = isPidRunning(existingLock.pid); - if (existingLock.pid === lockOwnerPid && lockOwnerRunning) { - await touchPidLockFile(pidPath); + const result = await clearExistingPidLock(pidPath, existingLock, lockOwnerPid, options); + if (result === "already_owned") { return; } - if (lockOwnerRunning && (await isPidLockFresh(pidPath))) { - throw new PidLockError( - `Another Paseo daemon is already running (PID ${existingLock.pid}, started ${existingLock.startedAt})`, - existingLock, - ); - } - // Stale lock - remove it - await unlink(pidPath).catch(() => {}); } // Create new lock with exclusive flag @@ -121,36 +198,11 @@ export async function acquirePidLock( hostname: hostname(), uid: process.getuid?.() ?? 0, listen, + heartbeat: true, ...(process.env.PASEO_DESKTOP_MANAGED === "1" ? { desktopManaged: true } : {}), }; - let fd; - try { - fd = await open(pidPath, "wx"); - await fd.write(JSON.stringify(lockInfo)); - } catch (err) { - if (isErrnoException(err) && err.code === "EEXIST") { - // Race condition - another process created the file - // Re-read and check - try { - const content = await readFile(pidPath, "utf-8"); - const raceLock = parsePidLockInfo(JSON.parse(content)); - if (raceLock) { - throw new PidLockError( - `Another Paseo daemon is already running (PID ${raceLock.pid})`, - raceLock, - ); - } - throw new PidLockError("Failed to acquire PID lock due to race condition"); - } catch (innerErr) { - if (innerErr instanceof PidLockError) throw innerErr; - throw new PidLockError("Failed to acquire PID lock due to race condition"); - } - } - throw err; - } finally { - await fd?.close(); - } + await writeNewPidLock(pidPath, lockInfo); } export async function refreshPidLock( @@ -159,19 +211,43 @@ export async function refreshPidLock( ): Promise { const pidPath = getPidFilePath(paseoHome); const lockOwnerPid = resolveOwnerPid(options?.ownerPid); - const lock = await readPidLock(pidPath); - if (!lock) { - throw new PidLockError("Cannot refresh PID lock: invalid lock file"); + let fd; + try { + fd = await open(pidPath, "r+"); + } catch (error) { + if (isErrnoException(error) && error.code === "ENOENT") { + throw new PidLockError("Cannot refresh PID lock: lock file is missing"); + } + throw error; } - if (lock.pid !== lockOwnerPid) { - throw new PidLockError(`Cannot refresh PID lock owned by PID ${lock.pid}`, lock); + + try { + let lock: PidLockInfo | null = null; + try { + lock = parsePidLockInfo(JSON.parse(await fd.readFile("utf-8"))); + } catch { + // Report malformed contents as an ownership failure below. + } + if (!lock) { + throw new PidLockError("Cannot refresh PID lock: invalid lock file"); + } + if (lock.pid !== lockOwnerPid) { + throw new PidLockError(`Cannot refresh PID lock owned by PID ${lock.pid}`, lock); + } + const now = new Date(); + await fd.utimes(now, now); + } finally { + await fd.close(); } - await touchPidLockFile(pidPath); } export function startPidLockHeartbeat( paseoHome: string, - options?: { ownerPid?: number; intervalMs?: number }, + options?: { + ownerPid?: number; + intervalMs?: number; + onError?: (error: unknown) => void; + }, ): () => void { const intervalMs = options?.intervalMs ?? PID_LOCK_HEARTBEAT_INTERVAL_MS; let refreshing = false; @@ -181,8 +257,15 @@ export function startPidLockHeartbeat( return; } refreshing = true; - refreshPidLock(paseoHome, options) - .catch(() => undefined) + refreshPidLock(paseoHome, { ownerPid: options?.ownerPid }) + .catch((error) => { + if (options?.onError) { + options.onError(error); + return; + } + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`PID lock heartbeat failed: ${message}\n`); + }) .finally(() => { refreshing = false; }); @@ -254,7 +337,10 @@ export async function isLocked( return { locked: false }; } const pidPath = getPidFilePath(paseoHome); - if (!isPidRunning(info.pid) || !(await isPidLockFresh(pidPath))) { + if (!isPidRunning(info.pid)) { + return { locked: false, info }; + } + if (info.heartbeat === true && !(await isPidLockFresh(pidPath))) { return { locked: false, info }; } return { locked: true, info };