mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(packaged): force-stop a lingering skewed desktop on relaunch (#5677)
An updated daemon can connect to an old desktop main process that never quit and still holds the fixed `desktop.sock`. The old desktop rejects newer sidecar messages (e.g. `render-slides`, added in 0.13.0) as "unknown sidecar message", which surfaces as the version-skew PPTX export failure. The launcher relaunch path only ever waited for the old process to leave on its own: `waitForLauncherAfterQuit` warned and continued on timeout, and `inspectExistingDesktopForLauncher` gave up (returned `exit`) when a running-but-stale-sidecar desktop ignored SHUTDOWN — leaving the user pinned to the skewed process. Meanwhile daemon/web children already get a SIGTERM->SIGKILL escalation via `stopProcesses` in `closeManagedChild`. Close that gap: escalate the same `stopProcesses` helper against the lingering desktop in both launcher spots, then restart fresh so the relaunched app binds a matching-version desktop on the socket. Scope: the JS launcher paths only. The mac/win deferred-installer scripts (`macDeferredInstallerScript` / `windowsDeferredInstallerScript`) still give up on timeout and are left as a follow-up for desktop infra review.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { appendFile, mkdir } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { waitForProcessExit } from "@open-design/platform";
|
||||
import { stopProcesses, waitForProcessExit, type StopProcessesResult } from "@open-design/platform";
|
||||
import type { LauncherAfterQuitRequest } from "@open-design/launcher-proto";
|
||||
import {
|
||||
APP_KEYS,
|
||||
@@ -30,21 +30,64 @@ async function writeLauncherAfterQuitLog(paths: PackagedNamespacePaths, message:
|
||||
);
|
||||
}
|
||||
|
||||
/** Injectable process controls so tests never signal real PIDs. */
|
||||
export type LauncherProcessControls = {
|
||||
stopProcesses: typeof stopProcesses;
|
||||
waitForExit: typeof waitForProcessExit;
|
||||
};
|
||||
|
||||
/**
|
||||
* Force a desktop process that outlived the launcher's graceful handshake off
|
||||
* the fixed `desktop.sock`.
|
||||
*
|
||||
* A packaged desktop that ignores SHUTDOWN or never quits keeps holding that
|
||||
* socket. A freshly updated daemon then connects to the *stale* desktop, and its
|
||||
* newer messages (e.g. `render-slides`, added in 0.13.0) are rejected as
|
||||
* "unknown sidecar message" — the version-skew export failure users hit after an
|
||||
* update. Escalating SIGTERM→SIGKILL here mirrors how `closeManagedChild`
|
||||
* already force-stops daemon/web children that ignore SHUTDOWN, so no
|
||||
* skewed desktop is left squatting on the socket the relaunched app must bind.
|
||||
*
|
||||
* @returns whether the process is confirmed gone (safe to rebind the socket).
|
||||
*/
|
||||
async function forceStopLingeringDesktop(
|
||||
pid: number | null | undefined,
|
||||
context: string,
|
||||
paths: PackagedNamespacePaths,
|
||||
logger: LauncherAfterQuitLogger,
|
||||
stop: typeof stopProcesses,
|
||||
): Promise<boolean> {
|
||||
if (pid == null) return true;
|
||||
const result: StopProcessesResult = await stop([pid]);
|
||||
const gone = !result.remainingPids.includes(pid);
|
||||
const outcome = !gone ? "survived" : result.forcedPids.includes(pid) ? "sigkill" : "sigterm";
|
||||
const message = `force-stop ${context} pid=${pid} outcome=${outcome}`;
|
||||
await writeLauncherAfterQuitLog(paths, message);
|
||||
if (!gone) logger.warn(`[open-design launcher] ${message}`);
|
||||
return gone;
|
||||
}
|
||||
|
||||
export async function waitForLauncherAfterQuit(
|
||||
request: LauncherAfterQuitRequest | null,
|
||||
paths: PackagedNamespacePaths,
|
||||
logger: LauncherAfterQuitLogger = console,
|
||||
controls: Partial<LauncherProcessControls> = {},
|
||||
): Promise<void> {
|
||||
if (request == null) return;
|
||||
const waitForExit = controls.waitForExit ?? waitForProcessExit;
|
||||
const stop = controls.stopProcesses ?? stopProcesses;
|
||||
await writeLauncherAfterQuitLog(paths, `armed targetPid=${request.targetPid} timeoutMs=${request.timeoutMs}`);
|
||||
const exited = await waitForProcessExit(request.targetPid, request.timeoutMs);
|
||||
const exited = await waitForExit(request.targetPid, request.timeoutMs);
|
||||
if (exited) {
|
||||
await writeLauncherAfterQuitLog(paths, `observed-exit targetPid=${request.targetPid}`);
|
||||
return;
|
||||
}
|
||||
const message = `timed-out targetPid=${request.targetPid}`;
|
||||
// The old process outlived its quit grace and still holds the fixed socket.
|
||||
// Force it off so the relaunched app binds cleanly instead of skewing.
|
||||
const message = `timed-out targetPid=${request.targetPid}; forcing stop`;
|
||||
await writeLauncherAfterQuitLog(paths, message);
|
||||
logger.warn(`[open-design launcher] ${message}`);
|
||||
await forceStopLingeringDesktop(request.targetPid, "after-quit-timeout", paths, logger, stop);
|
||||
}
|
||||
|
||||
export async function inspectExistingDesktopForLauncher(
|
||||
@@ -53,12 +96,14 @@ export async function inspectExistingDesktopForLauncher(
|
||||
logger?: LauncherAfterQuitLogger;
|
||||
paths: PackagedNamespacePaths;
|
||||
requestIpc?: typeof requestJsonIpc;
|
||||
stopProcesses?: typeof stopProcesses;
|
||||
waitForExit?: typeof waitForProcessExit;
|
||||
},
|
||||
): Promise<LauncherExistingDesktopGateResult> {
|
||||
const logger = options.logger ?? console;
|
||||
const requestIpc = options.requestIpc ?? requestJsonIpc;
|
||||
const waitForExit = options.waitForExit ?? waitForProcessExit;
|
||||
const stop = options.stopProcesses ?? stopProcesses;
|
||||
const ipcPath = resolveAppIpcPath({
|
||||
app: APP_KEYS.DESKTOP,
|
||||
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
@@ -120,7 +165,13 @@ export async function inspectExistingDesktopForLauncher(
|
||||
options.paths,
|
||||
`inspect-found-existing namespace=${namespace} shutdown=${exited ? "exited" : "timed-out"} reason=stale-sidecar pid=${pid}`,
|
||||
);
|
||||
if (!exited) return { action: "exit", reason: "existing-focus-failed" };
|
||||
// A skewed desktop (running, but its own daemon/web sidecars are stale)
|
||||
// that ignores SHUTDOWN would otherwise make us exit and leave the user
|
||||
// pinned to it. Force it off the socket instead, then restart fresh.
|
||||
if (!exited) {
|
||||
const gone = await forceStopLingeringDesktop(pid, "stale-sidecar", options.paths, logger, stop);
|
||||
if (!gone) return { action: "exit", reason: "existing-focus-failed" };
|
||||
}
|
||||
}
|
||||
return { action: "continue", reason: "stale-sidecar" };
|
||||
}
|
||||
|
||||
@@ -4,10 +4,28 @@ import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { SIDECAR_MESSAGES } from "@open-design/sidecar-proto";
|
||||
import type { StopProcessesResult, stopProcesses, waitForProcessExit } from "@open-design/platform";
|
||||
|
||||
import { inspectExistingDesktopForLauncher, waitForLauncherAfterQuit } from "../src/launcher-after-quit.js";
|
||||
import type { PackagedNamespacePaths } from "../src/paths.js";
|
||||
|
||||
/** Build a `stopProcesses` result without signalling any real PID. */
|
||||
function fakeStopResult(pid: number, opts: { forced?: boolean; survived?: boolean } = {}): StopProcessesResult {
|
||||
return {
|
||||
alreadyStopped: false,
|
||||
forcedPids: opts.forced ? [pid] : [],
|
||||
matchedPids: [pid],
|
||||
remainingPids: opts.survived ? [pid] : [],
|
||||
stoppedPids: opts.survived ? [] : [pid],
|
||||
};
|
||||
}
|
||||
|
||||
const neverExits = (async () => false) as typeof waitForProcessExit;
|
||||
|
||||
function fakeStop(pid: number, opts: { forced?: boolean; survived?: boolean } = {}) {
|
||||
return vi.fn(async () => fakeStopResult(pid, opts)) as unknown as typeof stopProcesses;
|
||||
}
|
||||
|
||||
function fakePaths(root: string): PackagedNamespacePaths {
|
||||
return {
|
||||
cacheRoot: join(root, "cache"),
|
||||
@@ -61,6 +79,26 @@ describe("waitForLauncherAfterQuit", () => {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("force-stops the lingering pid when the old process never exits", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "od-launcher-after-quit-forcekill-"));
|
||||
const stop = fakeStop(4242);
|
||||
try {
|
||||
const paths = fakePaths(root);
|
||||
|
||||
await waitForLauncherAfterQuit({ targetPid: 4242, timeoutMs: 5 }, paths, { warn: vi.fn() }, {
|
||||
stopProcesses: stop,
|
||||
waitForExit: neverExits,
|
||||
});
|
||||
|
||||
expect(stop).toHaveBeenCalledWith([4242]);
|
||||
const log = await readFile(join(root, "logs", "launcher", "after-quit.log"), "utf8");
|
||||
expect(log).toContain("timed-out targetPid=4242; forcing stop");
|
||||
expect(log).toContain("force-stop after-quit-timeout pid=4242 outcome=sigterm");
|
||||
} finally {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("inspectExistingDesktopForLauncher", () => {
|
||||
@@ -183,4 +221,74 @@ describe("inspectExistingDesktopForLauncher", () => {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("force-stops a skewed desktop that ignores SHUTDOWN, then restarts", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "od-launcher-inspect-stale-forcekill-"));
|
||||
const stop = fakeStop(1234, { forced: true });
|
||||
try {
|
||||
const paths = fakePaths(root);
|
||||
|
||||
const result = await inspectExistingDesktopForLauncher("release-beta-win", {
|
||||
paths,
|
||||
requestIpc: (async (ipcPath: string, message: unknown) => {
|
||||
if ((message as { type?: string }).type === SIDECAR_MESSAGES.STATUS) {
|
||||
if (ipcPath.includes("daemon")) {
|
||||
return { pid: 2345, state: "running", updatedAt: new Date().toISOString(), url: "http://127.0.0.1:1234" };
|
||||
}
|
||||
if (ipcPath.includes("web")) {
|
||||
throw new Error("web pipe missing");
|
||||
}
|
||||
return { pid: 1234, state: "running", updatedAt: new Date().toISOString() };
|
||||
}
|
||||
return { accepted: true };
|
||||
}) as typeof import("@open-design/sidecar").requestJsonIpc,
|
||||
stopProcesses: stop,
|
||||
waitForExit: neverExits,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ action: "continue", reason: "stale-sidecar" });
|
||||
expect(stop).toHaveBeenCalledWith([1234]);
|
||||
const log = await readFile(join(root, "logs", "launcher", "after-quit.log"), "utf8");
|
||||
expect(log).toContain("shutdown=timed-out reason=stale-sidecar pid=1234");
|
||||
expect(log).toContain("force-stop stale-sidecar pid=1234 outcome=sigkill");
|
||||
} finally {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("exits without restarting when a skewed desktop cannot be force-stopped", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "od-launcher-inspect-stale-survives-"));
|
||||
const logger = { warn: vi.fn() };
|
||||
const stop = fakeStop(1234, { survived: true });
|
||||
try {
|
||||
const paths = fakePaths(root);
|
||||
|
||||
const result = await inspectExistingDesktopForLauncher("release-beta-win", {
|
||||
logger,
|
||||
paths,
|
||||
requestIpc: (async (ipcPath: string, message: unknown) => {
|
||||
if ((message as { type?: string }).type === SIDECAR_MESSAGES.STATUS) {
|
||||
if (ipcPath.includes("daemon")) {
|
||||
return { pid: 2345, state: "running", updatedAt: new Date().toISOString(), url: "http://127.0.0.1:1234" };
|
||||
}
|
||||
if (ipcPath.includes("web")) {
|
||||
throw new Error("web pipe missing");
|
||||
}
|
||||
return { pid: 1234, state: "running", updatedAt: new Date().toISOString() };
|
||||
}
|
||||
return { accepted: true };
|
||||
}) as typeof import("@open-design/sidecar").requestJsonIpc,
|
||||
stopProcesses: stop,
|
||||
waitForExit: neverExits,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ action: "exit", reason: "existing-focus-failed" });
|
||||
expect(stop).toHaveBeenCalledWith([1234]);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("force-stop stale-sidecar pid=1234 outcome=survived"),
|
||||
);
|
||||
} finally {
|
||||
await rm(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user