From faee1cd9525b7d06fb16287a10159bb5bcd13a03 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 16:49:05 +0200 Subject: [PATCH] Start the daemon after a reboot hands its old PID to another process (#5277) Closes #5257 --- .../tests/40-daemon-stale-boot-lock.test.ts | 96 +++++++++++++++++++ .../server/src/server/daemon-instance.test.ts | 96 +++++++++++++++++++ packages/server/src/server/daemon-instance.ts | 8 +- packages/server/src/server/pid-lock.test.ts | 8 +- packages/server/src/server/pid-lock.ts | 34 ++++++- 5 files changed, 229 insertions(+), 13 deletions(-) create mode 100644 packages/cli/tests/40-daemon-stale-boot-lock.test.ts create mode 100644 packages/server/src/server/daemon-instance.test.ts diff --git a/packages/cli/tests/40-daemon-stale-boot-lock.test.ts b/packages/cli/tests/40-daemon-stale-boot-lock.test.ts new file mode 100644 index 0000000000..ae8b2b990c --- /dev/null +++ b/packages/cli/tests/40-daemon-stale-boot-lock.test.ts @@ -0,0 +1,96 @@ +#!/usr/bin/env npx tsx + +/** + * Regression: a reboot reassigns the supervisor's PID to an unrelated process. The lock + * left behind must not make the daemon look like it is running, and must never be treated + * as a handle on whatever now holds that PID. + */ + +import assert from "node:assert"; +import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir, uptime } from "node:os"; +import { join } from "node:path"; +import { runLocalPaseo } from "./helpers/local-cli.ts"; + +console.log("=== Daemon lock left behind by a reboot ===\n"); + +const paseoHome = await mkdtemp(join(tmpdir(), "paseo-stale-boot-lock-")); +const env = { + PASEO_HOME: paseoHome, + PASEO_LOCAL_SPEECH_AUTO_DOWNLOAD: "0", + PASEO_DICTATION_ENABLED: "0", + PASEO_VOICE_MODE_ENABLED: "0", +}; + +// Stands in for the process that inherits the supervisor's PID after a reboot. It records a +// signal rather than dying of it, so a late delivery cannot slip past the assertion. +const signalMarker = join(paseoHome, "bystander-signalled"); +const bystander = spawn( + process.execPath, + [ + "-e", + `process.on("SIGTERM", () => require("node:fs").writeFileSync(${JSON.stringify(signalMarker)}, "SIGTERM"));` + + `setTimeout(() => {}, 120_000);`, + ], + { stdio: "ignore" }, +); +let bystanderExited = false; +bystander.once("exit", () => { + bystanderExited = true; +}); +assert(bystander.pid, "bystander process should have a pid"); + +try { + const beforeThisBoot = new Date(Date.now() - uptime() * 1000 - 60 * 60_000); + await writeFile( + join(paseoHome, "paseo.pid"), + JSON.stringify({ + pid: bystander.pid, + startedAt: beforeThisBoot.toISOString(), + hostname: "before-reboot", + uid: process.getuid?.() ?? 0, + listen: "127.0.0.1:6767", + desktopManaged: true, + heartbeat: true, + }), + ); + + const statusResult = await runLocalPaseo( + ["daemon", "status", "--home", paseoHome, "--json"], + env, + ); + assert.strictEqual( + statusResult.exitCode, + 0, + `daemon status should succeed:\nstdout:\n${statusResult.stdout}\nstderr:\n${statusResult.stderr}`, + ); + const status = JSON.parse(statusResult.stdout) as { localDaemon?: string; pid?: number | null }; + assert.strictEqual( + status.localDaemon, + "stopped", + `daemon left behind by a reboot should read as stopped, got: ${statusResult.stdout}`, + ); + assert.strictEqual(status.pid, null, "status should not report the reused pid as the daemon"); + console.log("✓ daemon status reports stopped\n"); + + const stopResult = await runLocalPaseo(["daemon", "stop", "--home", paseoHome], env); + assert.strictEqual( + stopResult.exitCode, + 0, + `daemon stop should succeed:\nstdout:\n${stopResult.stdout}\nstderr:\n${stopResult.stderr}`, + ); + assert.strictEqual( + existsSync(signalMarker), + false, + "daemon stop must not signal the process that inherited the pid", + ); + assert.strictEqual(bystanderExited, false, "the process that inherited the pid should survive"); + console.log("✓ daemon stop leaves the process holding that pid alone\n"); +} finally { + bystander.kill("SIGKILL"); + await rm(paseoHome, { recursive: true, force: true }); +} + +console.log("=== Daemon lock left behind by a reboot passed ==="); diff --git a/packages/server/src/server/daemon-instance.test.ts b/packages/server/src/server/daemon-instance.test.ts new file mode 100644 index 0000000000..9d7ea66c5a --- /dev/null +++ b/packages/server/src/server/daemon-instance.test.ts @@ -0,0 +1,96 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import { existsSync } from "node:fs"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir, uptime } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +import { readDaemonInstance, stopDaemonInstance } from "./daemon-instance.js"; +import { acquirePidLock, getPidLockInfo, isLocked, type PidLockInfo } from "./pid-lock.js"; + +// A process cannot have started before the machine booted, so a lock stamped +// before this boot names a PID that some unrelated process now holds. +function bootedAt(): number { + return Date.now() - uptime() * 1000; +} + +async function writeLock(paseoHome: string, lock: PidLockInfo): Promise { + await writeFile(join(paseoHome, "paseo.pid"), JSON.stringify(lock)); +} + +function lockFor(pid: number, startedAt: Date): PidLockInfo { + return { + pid, + startedAt: startedAt.toISOString(), + hostname: "old-host", + uid: process.getuid?.() ?? 0, + listen: "127.0.0.1:6767", + desktopManaged: true, + heartbeat: true, + }; +} + +describe("daemon instance identity across a reboot", () => { + let paseoHome: string; + let bystander: ChildProcess | undefined; + + beforeEach(async () => { + paseoHome = await mkdtemp(join(tmpdir(), "paseo-daemon-instance-")); + }); + + afterEach(async () => { + bystander?.kill("SIGKILL"); + bystander = undefined; + await rm(paseoHome, { recursive: true, force: true }); + }); + + test("a lock stamped before this boot has no running owner", async () => { + await writeLock(paseoHome, lockFor(process.pid, new Date(bootedAt() - 60 * 60_000))); + + expect(await readDaemonInstance(paseoHome)).toBeNull(); + expect(await isLocked(paseoHome)).toMatchObject({ locked: false }); + }); + + test("a supervisor started during this boot still holds the lock", async () => { + await writeLock(paseoHome, lockFor(process.pid, new Date())); + + expect(await readDaemonInstance(paseoHome)).toMatchObject({ pid: process.pid }); + expect(await isLocked(paseoHome)).toMatchObject({ locked: true }); + }); + + test("a new supervisor takes over a lock stamped before this boot", async () => { + await writeLock(paseoHome, lockFor(process.pid, new Date(bootedAt() - 60 * 60_000))); + + await acquirePidLock(paseoHome, null, { ownerPid: process.pid + 10_000 }); + + expect(await getPidLockInfo(paseoHome)).toMatchObject({ pid: process.pid + 10_000 }); + }); + + test("stopping a lock stamped before this boot leaves the process holding that pid alone", async () => { + // Records delivery rather than dying of it, so a signal cannot be missed by arriving late. + const signalMarker = join(paseoHome, "bystander-signalled"); + bystander = spawn( + process.execPath, + [ + "-e", + `process.on("SIGTERM", () => require("node:fs").writeFileSync(${JSON.stringify(signalMarker)}, "SIGTERM"));` + + `setTimeout(() => {}, 120_000);`, + ], + { stdio: "ignore" }, + ); + const bystanderPid = bystander.pid; + if (bystanderPid === undefined) throw new Error("bystander process did not start"); + let exited = false; + bystander.once("exit", () => { + exited = true; + }); + + await writeLock(paseoHome, lockFor(bystanderPid, new Date(bootedAt() - 60 * 60_000))); + + expect(await stopDaemonInstance(paseoHome)).toMatchObject({ action: "not_running" }); + + expect(existsSync(signalMarker)).toBe(false); + expect(exited).toBe(false); + await expect(readFile(join(paseoHome, "paseo.pid"), "utf-8")).rejects.toThrow(/ENOENT/); + }); +}); diff --git a/packages/server/src/server/daemon-instance.ts b/packages/server/src/server/daemon-instance.ts index 6ef315e58e..0cf140aeeb 100644 --- a/packages/server/src/server/daemon-instance.ts +++ b/packages/server/src/server/daemon-instance.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { getPidLockInfo, - isPidRunning, + isPidLockOwnerRunning, isSamePidLock, releasePidLock, type PidLockInfo, @@ -44,7 +44,7 @@ export class DaemonInstanceError extends Error { export async function readDaemonInstance(home: string): Promise { const lock = await getPidLockInfo(home); - return lock && isPidRunning(lock.pid) ? lock : null; + return lock && isPidLockOwnerRunning(lock) ? lock : null; } export function daemonLogPath(home: string): string { @@ -160,7 +160,7 @@ export async function stopDaemonInstance( `Supervisor changed for ${home}; refusing to stop PID ${instance.pid}.`, ); } - if (!instance || !isPidRunning(instance.pid)) { + if (!instance || !isPidLockOwnerRunning(instance)) { if (instance) await releasePidLock(home, { ownerPid: instance.pid, startedAt: instance.startedAt }); return { @@ -175,7 +175,7 @@ export async function stopDaemonInstance( let { forced, usedLifecycleRpc } = await requestInstanceStop(home, instance, options); const waitForExit = async (waitMs: number) => { const exitDeadline = Date.now() + waitMs; - while (isPidRunning(instance.pid)) { + while (isPidLockOwnerRunning(instance)) { const current = await getPidLockInfo(home); if (current && !isSamePidLock(instance, current)) throw new DaemonInstanceError( diff --git a/packages/server/src/server/pid-lock.test.ts b/packages/server/src/server/pid-lock.test.ts index 457e1e7edc..e54093f177 100644 --- a/packages/server/src/server/pid-lock.test.ts +++ b/packages/server/src/server/pid-lock.test.ts @@ -73,7 +73,7 @@ describe("pid-lock ownership", () => { pidPath, JSON.stringify({ pid: process.pid, - startedAt: "2026-01-01T00:00:00.000Z", + startedAt: new Date().toISOString(), hostname: "old-host", uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", @@ -106,7 +106,7 @@ describe("pid-lock ownership", () => { pidPath, JSON.stringify({ pid: process.pid, - startedAt: "2026-01-01T00:00:00.000Z", + startedAt: new Date().toISOString(), hostname: "old-host", uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", @@ -138,7 +138,7 @@ describe("pid-lock ownership", () => { pidPath, JSON.stringify({ pid: process.pid, - startedAt: "2026-01-01T00:00:00.000Z", + startedAt: new Date().toISOString(), hostname: "old-host", uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", @@ -169,7 +169,7 @@ describe("pid-lock ownership", () => { pidPath, JSON.stringify({ pid: process.pid, - startedAt: "2026-01-01T00:00:00.000Z", + startedAt: new Date().toISOString(), hostname: "old-host", uid: process.getuid?.() ?? 0, listen: "127.0.0.1:6767", diff --git a/packages/server/src/server/pid-lock.ts b/packages/server/src/server/pid-lock.ts index f7ad90bcd1..bffab63063 100644 --- a/packages/server/src/server/pid-lock.ts +++ b/packages/server/src/server/pid-lock.ts @@ -2,7 +2,7 @@ import { open, readFile, unlink, utimes } from "node:fs/promises"; import type { FileHandle } from "node:fs/promises"; import { ensurePrivateDirectory } from "./private-files.js"; import { join } from "node:path"; -import { hostname } from "node:os"; +import { hostname, uptime } from "node:os"; import { z } from "zod"; export const pidLockInfoSchema = z.object({ @@ -40,7 +40,7 @@ const PID_LOCK_HEARTBEAT_INTERVAL_MS = 30_000; const PID_LOCK_READ_RETRY_ATTEMPTS = 10; const PID_LOCK_READ_RETRY_DELAY_MS = 50; -export function isPidRunning(pid: number): boolean { +function isPidRunning(pid: number): boolean { try { process.kill(pid, 0); return true; @@ -49,6 +49,30 @@ export function isPidRunning(pid: number): boolean { } } +// `uptime()` reports whole seconds on some platforms, so the derived boot instant carries +// about a second of error. This covers that resolution and nothing more: every extra second +// is a window in which a lock written just before a reboot still reads as current. +const BOOT_INSTANT_TOLERANCE_MS = 5_000; + +function precedesThisBoot(startedAt: string): boolean { + const stamped = Date.parse(startedAt); + if (Number.isNaN(stamped)) return false; + return stamped < Date.now() - uptime() * 1000 - BOOT_INSTANT_TOLERANCE_MS; +} + +/** + * Whether the process that wrote this lock is still running. + * + * A PID alone does not identify the supervisor: the operating system hands the number to + * something else once the supervisor is gone, and a reboot reassigns it freely. A process + * cannot predate the boot it runs under, so a lock stamped before this boot is abandoned + * however alive its PID looks. + */ +export function isPidLockOwnerRunning(lock: PidLockInfo): boolean { + if (precedesThisBoot(lock.startedAt)) return false; + return isPidRunning(lock.pid); +} + function getPidFilePath(paseoHome: string): string { return join(paseoHome, "paseo.pid"); } @@ -105,7 +129,7 @@ async function clearExistingPidLock( existingLock: PidLockInfo, lockOwnerPid: number, ): Promise<"already_owned" | "cleared"> { - const lockOwnerRunning = isPidRunning(existingLock.pid); + const lockOwnerRunning = isPidLockOwnerRunning(existingLock); if (existingLock.pid === lockOwnerPid && lockOwnerRunning) { await touchPidLockFile(pidPath); return "already_owned"; @@ -116,7 +140,7 @@ async function clearExistingPidLock( if ( !confirmedLock || !isSamePidLock(existingLock, confirmedLock) || - isPidRunning(confirmedLock.pid) + isPidLockOwnerRunning(confirmedLock) ) { throw new PidLockError("PID lock changed while checking whether it was abandoned"); } @@ -339,7 +363,7 @@ export async function isLocked( if (!info) { return { locked: false }; } - if (!isPidRunning(info.pid)) { + if (!isPidLockOwnerRunning(info)) { return { locked: false, info }; } return { locked: true, info };