Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions src/update/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,10 @@ export interface RestartIo {
verifyOcxFn?: (pid: number) => number | null;
/** Liveness check when deciding whether a reclaim timeout still has live holders. */
isAliveFn?: (pid: number) => boolean;
/** Test seam for production pinned-start retries. */
spawnDetachedStartFn?: typeof spawnDetachedStart;
/** Test seam for terminating a still-running pinned-start attempt. */
killProxyFn?: typeof killProxy;
}

/**
Expand Down Expand Up @@ -937,6 +941,16 @@ async function restartAfterUpdate(
!!(await proxyIdentityAt(p, { hostname: host }))
));
const probeIdentity = io.probeProxyIdentity ?? defaultProbeProxyIdentity;
const now = io.now ?? (() => Date.now());
const spawnPinnedStart = io.spawnDetachedStartFn ?? spawnDetachedStart;
const killSpawnAttempt = (child: ChildProcess | null): void => {
// Once Node has observed this ChildProcess exit, its numeric PID is no longer
// owned by the start attempt and may already identify an unrelated process.
if (!child?.pid || child.exitCode !== null || child.signalCode !== null) return;
if (aliveFn(child.pid)) {
try { (io.killProxyFn ?? killProxy)(child.pid); } catch { /* best-effort */ }
}
};
const expectedVersion = typeof job.latestVersion === "string" && job.latestVersion.length > 0
? job.latestVersion
: null;
Expand Down Expand Up @@ -965,9 +979,7 @@ async function restartAfterUpdate(
`Pinned start attempt ${attempt - 1} did not become healthy on port ${port}; `
+ `retrying (${attempt}/${attempts}).`,
);
if (lastChild?.pid && aliveFn(lastChild.pid)) {
try { killProxy(lastChild.pid); } catch { /* best-effort */ }
}
killSpawnAttempt(lastChild);
lastChild = null;
}
preparePortForPinnedStart(job, port, listPids, aliveFn, verifyOcx);
Expand All @@ -987,17 +999,19 @@ async function restartAfterUpdate(
);
continue;
}
lastChild = spawnDetachedStart(job, job.installer, port);
const healthDeadline = Date.now() + perAttemptHealthMs;
while (Date.now() < healthDeadline) {
const child = spawnPinnedStart(job, job.installer, port);
lastChild = child;
child.once("exit", () => {
if (lastChild === child) lastChild = null;
});
const healthDeadline = now() + perAttemptHealthMs;
while (now() < healthDeadline) {
if (await probe(port, hostname)) return;
await sleep(500);
}
}
// Exhausted retries: do not leave a hung pinned-start child owning the port.
if (lastChild?.pid && aliveFn(lastChild.pid)) {
try { killProxy(lastChild.pid); } catch { /* best-effort */ }
}
killSpawnAttempt(lastChild);
}

/** Compact listen-holder summary for update-job logs when reclaim fails. */
Expand Down
45 changes: 45 additions & 0 deletions tests/update-job.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,51 @@ describe("GUI update execution decisions", () => {
expect(readUpdateJob(job.id)?.log.some(line => line.includes("waiting for ghost LISTEN rows to clear before pinned start"))).toBe(true);
});

test("restart retries never kill a PID retained from an exited pinned start", async () => {
let now = 0;
let spawnCount = 0;
const killed: number[] = [];
const job: UpdateJobState = {
id: "restart-stale-child-pid",
status: "restarting",
startedAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
currentVersion: "2.10.1",
latestVersion: "2.10.2",
channel: "latest",
installer: "npm",
restart: true,
command: "",
log: [],
};
writeFileSync(updateJobPath(job.id), JSON.stringify(job));

await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, {
serviceInstalledFn: () => false,
waitForPort: async () => true,
listListenPidsFn: () => [],
probeProxy: async () => false,
probeProxyIdentity: async () => null,
now: () => now,
sleepMs: async ms => { now += ms; },
isAliveFn: () => true,
spawnDetachedStartFn: () => {
spawnCount += 1;
const child = {
pid: 4242,
exitCode: 1,
signalCode: null,
once: () => child,
};
return child as unknown as ReturnType<typeof import("node:child_process").spawn>;
},
killProxyFn: pid => { killed.push(pid); },
});

expect(spawnCount).toBe(3);
expect(killed).toEqual([]);
});

test("service restart waits on the captured port and clears OCX_BAKE_PORT after install", async () => {
const waited: Array<{ port: number; hostname: string }> = [];
const bakeDuringInstall: string[] = [];
Expand Down
Loading