mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-06 16:34:36 +00:00
fix(server): confirm stale lock before reclaim
This commit is contained in:
@@ -16,6 +16,11 @@ const mocks = vi.hoisted(() => ({
|
||||
},
|
||||
runExternalCliJsonCommand: vi.fn(),
|
||||
runExternalCliTextCommand: vi.fn(),
|
||||
createNodeEntrypointInvocation: vi.fn(() => ({
|
||||
command: "node",
|
||||
args: [],
|
||||
env: {},
|
||||
})),
|
||||
spawnProcess: vi.fn(),
|
||||
logInfo: vi.fn(),
|
||||
logError: vi.fn(),
|
||||
@@ -59,11 +64,7 @@ vi.mock("../settings/desktop-settings-electron.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./runtime-paths.js", () => ({
|
||||
createNodeEntrypointInvocation: vi.fn(() => ({
|
||||
command: "node",
|
||||
args: [],
|
||||
env: {},
|
||||
})),
|
||||
createNodeEntrypointInvocation: mocks.createNodeEntrypointInvocation,
|
||||
resolveDaemonRunnerEntrypoint: vi.fn(() => ({
|
||||
entryPath: "/tmp/daemon.js",
|
||||
execArgv: [],
|
||||
@@ -112,6 +113,8 @@ describe("daemon-manager commands", () => {
|
||||
mocks.settings = DEFAULT_DESKTOP_SETTINGS;
|
||||
mocks.runExternalCliJsonCommand.mockReset();
|
||||
mocks.runExternalCliTextCommand.mockReset();
|
||||
mocks.createNodeEntrypointInvocation.mockReset();
|
||||
mocks.createNodeEntrypointInvocation.mockReturnValue({ command: "node", args: [], env: {} });
|
||||
mocks.spawnProcess.mockReset();
|
||||
mocks.logInfo.mockReset();
|
||||
mocks.logError.mockReset();
|
||||
@@ -439,6 +442,9 @@ describe("daemon-manager commands", () => {
|
||||
expect(message).toContain("Daemon failed to start: exit code 1");
|
||||
expect(recentLogsLabel?.split(/[\\/]/).at(-1)).toBe("daemon.log");
|
||||
expect(message).toContain("recent daemon failure");
|
||||
expect(mocks.createNodeEntrypointInvocation).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ args: ["--reclaim-stale-pid-lock"] }),
|
||||
);
|
||||
expect(mocks.spawnProcess).toHaveBeenCalledWith(
|
||||
"node",
|
||||
[],
|
||||
|
||||
@@ -388,7 +388,8 @@ async function startDaemon(): Promise<DesktopDaemonStatus> {
|
||||
const invocation = createNodeEntrypointInvocation({
|
||||
entrypoint: daemonRunner,
|
||||
argvMode: "node-script",
|
||||
args: [],
|
||||
// The status probe above confirmed that the recorded daemon is unreachable.
|
||||
args: ["--reclaim-stale-pid-lock"],
|
||||
baseEnv: process.env,
|
||||
});
|
||||
|
||||
|
||||
@@ -18,11 +18,13 @@ process.title = "Paseo Supervisor";
|
||||
|
||||
interface DaemonRunnerConfig {
|
||||
devMode: boolean;
|
||||
reclaimStalePidLock: boolean;
|
||||
workerArgs: string[];
|
||||
}
|
||||
|
||||
function parseConfig(argv: string[]): DaemonRunnerConfig {
|
||||
let devMode = false;
|
||||
let reclaimStalePidLock = false;
|
||||
const workerArgs: string[] = [];
|
||||
|
||||
for (const arg of argv) {
|
||||
@@ -30,10 +32,14 @@ function parseConfig(argv: string[]): DaemonRunnerConfig {
|
||||
devMode = true;
|
||||
continue;
|
||||
}
|
||||
if (arg === "--reclaim-stale-pid-lock") {
|
||||
reclaimStalePidLock = true;
|
||||
continue;
|
||||
}
|
||||
workerArgs.push(arg);
|
||||
}
|
||||
|
||||
return { devMode, workerArgs };
|
||||
return { devMode, reclaimStalePidLock, workerArgs };
|
||||
}
|
||||
|
||||
function resolveWorkerEntry(): string {
|
||||
@@ -110,7 +116,7 @@ async function main(): Promise<void> {
|
||||
try {
|
||||
await acquirePidLock(paseoHome, null, {
|
||||
ownerPid: process.pid,
|
||||
reclaimLegacyDesktopLock: process.env.PASEO_DESKTOP_MANAGED === "1",
|
||||
reclaimStaleDesktopLock: config.reclaimStalePidLock,
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof PidLockError) {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { mkdtemp, rm, utimes, writeFile } from "node:fs/promises";
|
||||
import { mkdtemp, open, rm, utimes, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, test } from "vitest";
|
||||
@@ -6,6 +6,7 @@ import { describe, expect, test } from "vitest";
|
||||
import {
|
||||
acquirePidLock,
|
||||
getPidLockInfo,
|
||||
isLocked,
|
||||
PidLockError,
|
||||
refreshPidLock,
|
||||
releasePidLock,
|
||||
@@ -58,7 +59,7 @@ describe("pid-lock ownership", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("reclaims a stale heartbeat lock when the recorded pid is alive", async () => {
|
||||
test("keeps a stale heartbeat lock when the recorded pid is alive without a reachability check", async () => {
|
||||
const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-stale-heartbeat-"));
|
||||
const replacementOwnerPid = process.pid + 10_000;
|
||||
|
||||
@@ -79,7 +80,43 @@ describe("pid-lock ownership", () => {
|
||||
const staleTime = new Date(Date.now() - 10 * 60_000);
|
||||
await utimes(pidPath, staleTime, staleTime);
|
||||
|
||||
await acquirePidLock(paseoHome, null, { ownerPid: replacementOwnerPid });
|
||||
await expect(isLocked(paseoHome)).resolves.toMatchObject({ locked: true });
|
||||
await expect(
|
||||
acquirePidLock(paseoHome, null, { ownerPid: replacementOwnerPid }),
|
||||
).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 desktop heartbeat lock after desktop confirms the daemon is unreachable", async () => {
|
||||
const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-stale-desktop-heartbeat-"));
|
||||
const replacementOwnerPid = process.pid + 10_000;
|
||||
|
||||
try {
|
||||
const pidPath = join(paseoHome, "paseo.pid");
|
||||
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,
|
||||
heartbeat: true,
|
||||
}),
|
||||
);
|
||||
const staleTime = new Date(Date.now() - 10 * 60_000);
|
||||
await utimes(pidPath, staleTime, staleTime);
|
||||
|
||||
await acquirePidLock(paseoHome, null, {
|
||||
ownerPid: replacementOwnerPid,
|
||||
reclaimStaleDesktopLock: true,
|
||||
});
|
||||
|
||||
const lock = await getPidLockInfo(paseoHome);
|
||||
expect(lock?.pid).toBe(replacementOwnerPid);
|
||||
@@ -141,7 +178,7 @@ describe("pid-lock ownership", () => {
|
||||
|
||||
await acquirePidLock(paseoHome, null, {
|
||||
ownerPid: replacementOwnerPid,
|
||||
reclaimLegacyDesktopLock: true,
|
||||
reclaimStaleDesktopLock: true,
|
||||
});
|
||||
|
||||
const lock = await getPidLockInfo(paseoHome);
|
||||
@@ -166,6 +203,29 @@ describe("pid-lock ownership", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("retries a heartbeat refresh while its owner is rewriting the lock", async () => {
|
||||
const paseoHome = await mkdtemp(join(tmpdir(), "paseo-pid-lock-refresh-rewrite-"));
|
||||
const pidPath = join(paseoHome, "paseo.pid");
|
||||
|
||||
try {
|
||||
await acquirePidLock(paseoHome, null, { ownerPid: process.pid });
|
||||
const lock = await getPidLockInfo(paseoHome);
|
||||
expect(lock).not.toBeNull();
|
||||
|
||||
const rewriteHandle = await open(pidPath, "r+");
|
||||
await rewriteHandle.truncate(0);
|
||||
|
||||
const refresh = refreshPidLock(paseoHome, { ownerPid: process.pid });
|
||||
await new Promise((resolve) => setTimeout(resolve, 250));
|
||||
await rewriteHandle.writeFile(JSON.stringify(lock));
|
||||
await rewriteHandle.close();
|
||||
|
||||
await expect(refresh).resolves.toBeUndefined();
|
||||
} 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-"));
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { open, readFile, stat, unlink, mkdir, utimes } from "node:fs/promises";
|
||||
import type { FileHandle } from "node:fs/promises";
|
||||
import { existsSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { hostname } from "node:os";
|
||||
@@ -38,6 +39,8 @@ export class PidLockError extends Error {
|
||||
// Stale recovery is for abandoned locks, so keep this well above ordinary event-loop stalls.
|
||||
const PID_LOCK_STALE_MS = 5 * 60_000;
|
||||
const PID_LOCK_HEARTBEAT_INTERVAL_MS = 30_000;
|
||||
const PID_LOCK_READ_RETRY_ATTEMPTS = 10;
|
||||
const PID_LOCK_READ_RETRY_DELAY_MS = 50;
|
||||
|
||||
function isPidRunning(pid: number): boolean {
|
||||
try {
|
||||
@@ -84,20 +87,16 @@ function resolveOwnerPid(ownerPid?: number): number {
|
||||
|
||||
interface AcquirePidLockOptions {
|
||||
ownerPid?: number;
|
||||
reclaimLegacyDesktopLock?: boolean;
|
||||
reclaimStaleDesktopLock?: 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;
|
||||
return options?.reclaimStaleDesktopLock === true && lock.desktopManaged === true;
|
||||
}
|
||||
|
||||
function isSamePidLock(left: PidLockInfo, right: PidLockInfo): boolean {
|
||||
@@ -222,12 +221,7 @@ export async function refreshPidLock(
|
||||
}
|
||||
|
||||
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.
|
||||
}
|
||||
const lock = await readPidLockFromHandleWithRetry(fd);
|
||||
if (!lock) {
|
||||
throw new PidLockError("Cannot refresh PID lock: invalid lock file");
|
||||
}
|
||||
@@ -241,6 +235,33 @@ export async function refreshPidLock(
|
||||
}
|
||||
}
|
||||
|
||||
async function readPidLockFromHandle(fd: FileHandle): Promise<PidLockInfo | null> {
|
||||
try {
|
||||
const { size } = await fd.stat();
|
||||
if (size === 0) {
|
||||
return null;
|
||||
}
|
||||
const content = Buffer.alloc(size);
|
||||
const { bytesRead } = await fd.read(content, 0, size, 0);
|
||||
return parsePidLockInfo(JSON.parse(content.subarray(0, bytesRead).toString("utf-8")));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async function readPidLockFromHandleWithRetry(fd: FileHandle): Promise<PidLockInfo | null> {
|
||||
for (let attempt = 0; attempt < PID_LOCK_READ_RETRY_ATTEMPTS; attempt += 1) {
|
||||
const lock = await readPidLockFromHandle(fd);
|
||||
if (lock) {
|
||||
return lock;
|
||||
}
|
||||
if (attempt < PID_LOCK_READ_RETRY_ATTEMPTS - 1) {
|
||||
await new Promise((resolve) => setTimeout(resolve, PID_LOCK_READ_RETRY_DELAY_MS));
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function startPidLockHeartbeat(
|
||||
paseoHome: string,
|
||||
options?: {
|
||||
@@ -282,23 +303,23 @@ export async function updatePidLock(
|
||||
): Promise<void> {
|
||||
const pidPath = getPidFilePath(paseoHome);
|
||||
const lockOwnerPid = resolveOwnerPid(options?.ownerPid);
|
||||
const content = await readFile(pidPath, "utf-8");
|
||||
const existingLock = parsePidLockInfo(JSON.parse(content));
|
||||
if (!existingLock) {
|
||||
throw new PidLockError("Cannot update PID lock: invalid lock file");
|
||||
}
|
||||
|
||||
if (existingLock.pid !== lockOwnerPid) {
|
||||
throw new PidLockError(`Cannot update PID lock owned by PID ${existingLock.pid}`, existingLock);
|
||||
}
|
||||
|
||||
const updatedLock: PidLockInfo = {
|
||||
...existingLock,
|
||||
...patch,
|
||||
};
|
||||
|
||||
const fd = await open(pidPath, "r+");
|
||||
try {
|
||||
const existingLock = await readPidLockFromHandleWithRetry(fd);
|
||||
if (!existingLock) {
|
||||
throw new PidLockError("Cannot update PID lock: invalid lock file");
|
||||
}
|
||||
if (existingLock.pid !== lockOwnerPid) {
|
||||
throw new PidLockError(
|
||||
`Cannot update PID lock owned by PID ${existingLock.pid}`,
|
||||
existingLock,
|
||||
);
|
||||
}
|
||||
|
||||
const updatedLock: PidLockInfo = {
|
||||
...existingLock,
|
||||
...patch,
|
||||
};
|
||||
await fd.truncate(0);
|
||||
await fd.writeFile(JSON.stringify(updatedLock));
|
||||
} finally {
|
||||
@@ -336,12 +357,8 @@ export async function isLocked(
|
||||
if (!info) {
|
||||
return { locked: false };
|
||||
}
|
||||
const pidPath = getPidFilePath(paseoHome);
|
||||
if (!isPidRunning(info.pid)) {
|
||||
return { locked: false, info };
|
||||
}
|
||||
if (info.heartbeat === true && !(await isPidLockFresh(pidPath))) {
|
||||
return { locked: false, info };
|
||||
}
|
||||
return { locked: true, info };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user