This repository has no description
fix(daemon): hard-exit backstop when a graceful shutdown wedges (#74)
Incident follow-up #2. When cos's `claude --resume` froze on its exit screen,
the daemon was left alive+orphaned (ppid=1) needing kill -9. Cause: cleanShutdown
awaits server.close() then process.exit() with NO overall deadline. close()'s
child-exit wait is internally bounded (~2s), but the outer promise can still hang
indefinitely — socketServer.close()'s callback never fires for a lingering/
untracked socket, or eventWriter.flush() stalls — so the daemon never reaches
process.exit() and lingers forever.
Add a hard deadline (default 5s, PTY_SHUTDOWN_DEADLINE_MS-overridable) armed by
cleanShutdown: if the graceful close() hasn't completed by the deadline, the
daemon force-exits regardless AND SIGKILLs its child (PtyServer.forceKillChild)
so a frozen child isn't left orphaned to init still alive. cleanShutdown is now
idempotent too — SIGTERM/SIGINT/onExit/spawner-watchdog can overlap; only the
first arms the deadline and drives close(), the rest get the same in-flight
promise. Same class as #69/#72 but the frozen-child / stuck-close case.
tests/shutdown-backstop.test.ts: a child that traps SIGHUP wedges the graceful
path -> backstop force-exits the daemon and reaps the child (timing-independent
proof: only the backstop's SIGKILL can kill a SIGHUP-trapping child; the graceful
path only ever sends SIGHUP). A normal session still shuts down promptly, well
under the deadline. All existing lifecycle tests (spawner-watchdog, kill-wait,
exit-event-race, exit-signal, rm-kill-ephemeral, up-down) intact.
+167
-1
+44
-1
src/server.ts
+44
-1
src/server.ts
···
941
941
});
942
942
});
943
943
}
944
+
945
+
/** Hard-kill the child with SIGKILL, bypassing the graceful SIGHUP that
946
+
* close() sends. Used by the shutdown backstop: when a graceful close()
947
+
* has wedged (e.g. a frozen child that ignores SIGHUP), the daemon is about
948
+
* to force-exit and must not leave the child orphaned to init still alive.
949
+
* Best-effort — a SIGKILL is unblockable, but the child may already be gone. */
950
+
forceKillChild(): void {
951
+
try { this.ptyProcess.kill("SIGKILL"); } catch {}
952
+
}
944
953
}
945
954
946
955
/** How often the spawner-PID watchdog checks for liveness. 5s is fast enough
···
998
1007
999
1008
const isEphemeral = config.ephemeral === true;
1000
1009
1010
+
// Hard deadline for a graceful shutdown before the daemon force-exits. The
1011
+
// graceful path (server.close()) is itself only internally bounded on the
1012
+
// child-exit wait (~2s); the outer promise can still hang indefinitely if
1013
+
// socketServer.close()'s callback never fires (a lingering/untracked socket)
1014
+
// or eventWriter.flush() stalls. That is exactly how a restart wedged the
1015
+
// daemon alive+orphaned (ppid=1), needing kill -9. This backstop guarantees
1016
+
// the daemon exits — and reaps its child — regardless. Env-overridable so
1017
+
// tests can force the backstop path deterministically.
1018
+
const SHUTDOWN_DEADLINE_MS = (() => {
1019
+
const raw = Number(process.env.PTY_SHUTDOWN_DEADLINE_MS);
1020
+
return Number.isFinite(raw) && raw > 0 ? raw : 5000;
1021
+
})();
1022
+
1023
+
// Idempotent: SIGTERM, SIGINT, the child's onExit, and the spawner watchdog
1024
+
// can all trigger shutdown, sometimes overlapping. Only the first arms the
1025
+
// deadline and drives close(); later callers get the same in-flight promise.
1026
+
let shutdownPromise: Promise<never> | null = null;
1001
1027
function cleanShutdown(code: number): Promise<never> {
1002
-
return server.close().then(() => {
1028
+
if (shutdownPromise) return shutdownPromise;
1029
+
const deadline = setTimeout(() => {
1030
+
console.error(
1031
+
`pty daemon "${config.name}": graceful shutdown exceeded ` +
1032
+
`${SHUTDOWN_DEADLINE_MS}ms — forcing exit (child reaped)`,
1033
+
);
1034
+
// Don't leave a frozen child orphaned, and don't leave a stale pid/sock
1035
+
// pointing at a daemon that's about to vanish.
1036
+
server.forceKillChild();
1037
+
try {
1038
+
if (isEphemeral) cleanupAll(config.name);
1039
+
else cleanup(config.name);
1040
+
} catch {}
1041
+
process.exit(code);
1042
+
}, SHUTDOWN_DEADLINE_MS);
1043
+
shutdownPromise = server.close().then(() => {
1044
+
clearTimeout(deadline);
1003
1045
if (isEphemeral) cleanupAll(config.name);
1004
1046
process.exit(code);
1005
1047
});
1048
+
return shutdownPromise;
1006
1049
}
1007
1050
1008
1051
const server = new PtyServer({
+123
tests/shutdown-backstop.test.ts
+123
tests/shutdown-backstop.test.ts
···
1
+
import { describe, it, expect, afterAll } from "vitest";
2
+
import * as fs from "node:fs";
3
+
import * as os from "node:os";
4
+
import * as path from "node:path";
5
+
import { fileURLToPath } from "node:url";
6
+
import { spawn } from "node:child_process";
7
+
8
+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
9
+
const nodeBin = process.execPath;
10
+
const serverModule = path.join(__dirname, "..", "dist", "server.js");
11
+
12
+
const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "pty-backstop-"));
13
+
const spawnedPids: number[] = [];
14
+
afterAll(() => {
15
+
for (const pid of spawnedPids) { try { process.kill(pid, "SIGKILL"); } catch {} }
16
+
fs.rmSync(testRoot, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
17
+
});
18
+
19
+
function isAlive(pid: number): boolean {
20
+
try {
21
+
process.kill(pid, 0);
22
+
return true;
23
+
} catch (err) {
24
+
return (err as NodeJS.ErrnoException).code === "EPERM";
25
+
}
26
+
}
27
+
28
+
async function waitUntilDead(pid: number, timeoutMs: number): Promise<boolean> {
29
+
const start = Date.now();
30
+
while (Date.now() - start < timeoutMs) {
31
+
if (!isAlive(pid)) return true;
32
+
await new Promise((r) => setTimeout(r, 50));
33
+
}
34
+
return false;
35
+
}
36
+
37
+
function waitForFile(p: string, timeoutMs: number): Promise<void> {
38
+
const start = Date.now();
39
+
return new Promise((resolve, reject) => {
40
+
const tick = () => {
41
+
try { fs.statSync(p); resolve(); return; } catch {}
42
+
if (Date.now() - start > timeoutMs) { reject(new Error(`Timeout waiting for ${p}`)); return; }
43
+
setTimeout(tick, 50);
44
+
};
45
+
tick();
46
+
});
47
+
}
48
+
49
+
/** Spawn a daemon directly (the real server.js entry point), running `command`
50
+
* with `args`, and with the given extra env (e.g. PTY_SHUTDOWN_DEADLINE_MS).
51
+
* Returns the daemon PID once its socket exists. */
52
+
async function startDaemon(
53
+
sessionDir: string,
54
+
name: string,
55
+
command: string,
56
+
args: string[],
57
+
extraEnv: Record<string, string> = {},
58
+
): Promise<number> {
59
+
const config = JSON.stringify({
60
+
name, command, args, displayCommand: `${command} ${args.join(" ")}`,
61
+
cwd: os.tmpdir(), rows: 24, cols: 80,
62
+
});
63
+
const child = spawn(nodeBin, [serverModule], {
64
+
detached: true,
65
+
stdio: ["ignore", "ignore", "ignore"],
66
+
env: {
67
+
...(process.env as Record<string, string>),
68
+
PTY_SERVER_CONFIG: config,
69
+
PTY_SESSION_DIR: sessionDir,
70
+
...extraEnv,
71
+
},
72
+
});
73
+
child.unref();
74
+
spawnedPids.push(child.pid!);
75
+
await waitForFile(path.join(sessionDir, `${name}.sock`), 5000);
76
+
return child.pid!;
77
+
}
78
+
79
+
describe("daemon shutdown-hang backstop", () => {
80
+
it("force-exits and reaps a frozen child when graceful shutdown exceeds the deadline", async () => {
81
+
const sessionDir = fs.mkdtempSync(path.join(testRoot, "d-"));
82
+
const name = `bs-${Math.random().toString(36).slice(2, 6)}`;
83
+
const childPidFile = path.join(sessionDir, "child.pid");
84
+
85
+
// A child that traps SIGHUP: close()'s graceful `ptyProcess.kill()` (SIGHUP)
86
+
// is ignored, so childExited never resolves and the graceful shutdown drags.
87
+
// Only the backstop's SIGKILL can reap it — which is exactly what we assert.
88
+
const script = `echo $$ > "${childPidFile}"; trap "" HUP; while true; do sleep 1; done`;
89
+
const daemonPid = await startDaemon(
90
+
sessionDir, name, "sh", ["-c", script],
91
+
{ PTY_SHUTDOWN_DEADLINE_MS: "300" },
92
+
);
93
+
94
+
await waitForFile(childPidFile, 5000);
95
+
const childPid = Number(fs.readFileSync(childPidFile, "utf8").trim());
96
+
spawnedPids.push(childPid);
97
+
expect(isAlive(childPid)).toBe(true);
98
+
99
+
// Initiate shutdown. The graceful path can't complete (frozen child), so the
100
+
// 300ms backstop must fire: force-exit the daemon AND SIGKILL the child.
101
+
process.kill(daemonPid, "SIGTERM");
102
+
103
+
// Daemon self-exits despite the wedged close() (no kill -9 needed).
104
+
expect(await waitUntilDead(daemonPid, 4000)).toBe(true);
105
+
// The frozen child is reaped, not left orphaned to init. This is the
106
+
// timing-independent proof: without the backstop the child only ever
107
+
// receives a (trapped) SIGHUP and would survive.
108
+
expect(await waitUntilDead(childPid, 4000)).toBe(true);
109
+
}, 15000);
110
+
111
+
it("does not disturb a normal, prompt shutdown", async () => {
112
+
const sessionDir = fs.mkdtempSync(path.join(testRoot, "d-"));
113
+
const name = `bs-ok-${Math.random().toString(36).slice(2, 6)}`;
114
+
115
+
// Plain child that exits on the graceful SIGHUP — the default (5s) deadline
116
+
// never comes near firing; graceful close() completes and exits promptly.
117
+
const daemonPid = await startDaemon(sessionDir, name, "sleep", ["3600"]);
118
+
expect(isAlive(daemonPid)).toBe(true);
119
+
120
+
process.kill(daemonPid, "SIGTERM");
121
+
expect(await waitUntilDead(daemonPid, 3000)).toBe(true);
122
+
}, 10000);
123
+
});