fix(db): register SIGHUP handler and stop force-killing server on win32 stop paths (#8045) (#8148)

Windows console-window close delivers CTRL_CLOSE_EVENT, which Node/libuv maps
to a JS-visible SIGHUP event. initGracefulShutdown() only listened for
SIGTERM/SIGINT, so closing the window never ran cleanup() (WAL checkpoint +
closeDbInstance()), leaving storage.sqlite's WAL un-checkpointed for the next
launch.

Separately, process.kill(pid, "SIGTERM") on win32 unconditionally
force-terminates the target process instead of delivering an interceptable
signal. The CLI's own stop paths (ServerSupervisor.stop() and
runStopCommand()) sent it immediately on every stop, racing and beating the
child's own async graceful shutdown before the WAL checkpoint could run.

Fix:
- src/lib/gracefulShutdown.ts: register a SIGHUP handler alongside
  SIGTERM/SIGINT.
- src/shared/platform/windowsProcess.ts (new): stopProcessGracefully() skips
  the immediate SIGTERM on win32 (letting the target's own CTRL_C/CTRL_CLOSE
  handling run) and polls before escalating to SIGKILL; unchanged immediate
  SIGTERM behavior on POSIX.
- bin/cli/runtime/processSupervisor.mjs and bin/cli/commands/stop.mjs: use
  stopProcessGracefully() instead of an unconditional process.kill(SIGTERM).

Regression tests: tests/unit/graceful-shutdown-sighup-8045.test.ts (reuses
the RED probe from the triage analysis) and
tests/unit/windows-process-stop-8045.test.ts.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-22 11:27:49 -03:00
committed by GitHub
parent 1c116e0501
commit 6302a78657
7 changed files with 229 additions and 21 deletions

View File

@@ -8,6 +8,7 @@ import {
sleep,
} from "../utils/pid.mjs";
import { t } from "../i18n.mjs";
import { stopProcessGracefully } from "../../../src/shared/platform/windowsProcess.ts";
const execFileAsync = promisify(execFile);
@@ -27,18 +28,11 @@ export async function runStopCommand(opts = {}) {
if (pid && isPidRunning(pid)) {
console.log(t("stop.stopping", { pid }));
try {
process.kill(pid, "SIGTERM");
let waited = 0;
while (waited < 5000 && isPidRunning(pid)) {
await sleep(100);
waited += 100;
}
if (isPidRunning(pid)) {
process.kill(pid, "SIGKILL");
await sleep(500);
}
// #8045: on win32, process.kill(pid, "SIGTERM") unconditionally force-terminates
// the target instead of delivering an interceptable signal, racing (and beating)
// the server's own async graceful shutdown / WAL checkpoint. stopProcessGracefully
// skips the immediate SIGTERM on win32 and just polls before escalating to SIGKILL.
await stopProcessGracefully({ pid, timeoutMs: 5000, isPidRunning, sleep });
killAllSubprocesses();
cleanupPidFile("server");

View File

@@ -1,6 +1,6 @@
import { spawn } from "node:child_process";
import { dirname } from "node:path";
import { writePidFile, cleanupPidFile, killAllSubprocesses } from "../utils/pid.mjs";
import { writePidFile, cleanupPidFile, killAllSubprocesses, isPidRunning } from "../utils/pid.mjs";
import {
RESTART_RESET_MS,
DEFAULT_MAX_RESTARTS,
@@ -9,6 +9,7 @@ import {
waitUntilPortFree,
} from "./supervisorPolicy.mjs";
import { buildNodeHeapArgs } from "../../../scripts/build/runtime-env.mjs";
import { stopProcessGracefully } from "../../../src/shared/platform/windowsProcess.ts";
const CRASH_LOG_LINES = 50;
@@ -135,14 +136,13 @@ export class ServerSupervisor {
stop() {
this.isShuttingDown = true;
if (this.child?.pid) {
try {
process.kill(this.child.pid, "SIGTERM");
} catch {}
setTimeout(() => {
try {
process.kill(this.child.pid, "SIGKILL");
} catch {}
}, 5000);
// #8045: on win32, process.kill(pid, "SIGTERM") unconditionally force-terminates
// the target — it is never a real, interceptable signal there. The child already
// receives the real CTRL_C_EVENT/CTRL_CLOSE_EVENT independently (it shares the
// console) and runs its own async graceful shutdown (WAL checkpoint). Sending
// SIGTERM immediately on win32 races and beats that cleanup. Fire-and-forget:
// stop() itself stays sync so callers keep their existing control flow.
void stopProcessGracefully({ pid: this.child.pid, timeoutMs: 5000, isPidRunning });
}
killAllSubprocesses();
}

View File

@@ -0,0 +1 @@
- fix(db): register SIGHUP handler and stop force-killing the server on win32 stop paths so storage.sqlite's WAL gets checkpointed on shutdown (#8045)

View File

@@ -161,6 +161,11 @@ export function initGracefulShutdown(): void {
process.on("SIGTERM", () => shutdown("SIGTERM"));
process.on("SIGINT", () => shutdown("SIGINT"));
// #8045: on Windows, closing the console window delivers CTRL_CLOSE_EVENT, which
// Node/libuv maps to a JS-visible "SIGHUP" event — without this listener, closing
// the window never runs cleanup() (WAL checkpoint + closeDbInstance()), leaving
// storage.sqlite's WAL un-checkpointed for the next launch.
process.on("SIGHUP", () => shutdown("SIGHUP"));
console.log("[Shutdown] Graceful shutdown handlers registered.");
}

View File

@@ -0,0 +1,88 @@
/**
* Platform-aware graceful process termination for the CLI's own child/self stop
* paths (`bin/cli/runtime/processSupervisor.mjs`, `bin/cli/commands/stop.mjs`).
*
* #8045: on win32, `process.kill(pid, "SIGTERM")` is documented by Node.js to cause
* "unconditional termination of the target process" — it is never a real, interceptable
* signal there, identical to SIGKILL. Sending it to the OmniRoute server child
* immediately on every stop/Ctrl+C force-kills it before its own async
* `initGracefulShutdown()` cleanup (WAL checkpoint + closeDbInstance()) has any
* realistic chance to run, corrupting storage.sqlite's WAL state for the next launch.
*
* On win32, the target process already receives the real CTRL_C_EVENT/CTRL_CLOSE_EVENT
* independently (it shares the console) and runs its own graceful shutdown — so instead
* of racing it with an immediate force-kill, this helper polls for exit and only
* escalates to SIGKILL if the process is still alive after the timeout.
*
* @module shared/platform/windowsProcess
*/
export interface StopProcessGracefullyOptions {
/** PID of the target process to stop. */
pid: number;
/** Max time to wait for the process to exit on its own before escalating (ms). */
timeoutMs?: number;
/** Poll interval while waiting for exit (ms). */
pollIntervalMs?: number;
/** Injectable liveness check (defaults to `process.kill(pid, 0)`-based check). */
isPidRunning?: (pid: number) => boolean;
/** Injectable sleep (for tests). */
sleep?: (ms: number) => Promise<void>;
/** Injectable platform override (for tests); defaults to `process.platform`. */
platform?: NodeJS.Platform;
}
const defaultIsPidRunning = (pid: number): boolean => {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
};
const defaultSleep = (ms: number): Promise<void> =>
new Promise((resolve) => setTimeout(resolve, ms));
/**
* Stop `pid` without force-killing it immediately on win32.
*
* - Non-win32: sends SIGTERM immediately (unchanged prior behavior — SIGTERM is a
* real, interceptable signal on POSIX, so this still lets the target run its own
* graceful shutdown before an eventual SIGKILL escalation).
* - win32: does NOT send SIGTERM (it would force-kill immediately, racing the
* target's own console-close/Ctrl+C handling). Instead polls `isPidRunning` for up
* to `timeoutMs`, then escalates to SIGKILL only if the process is still alive.
*/
export async function stopProcessGracefully(options: StopProcessGracefullyOptions): Promise<void> {
const {
pid,
timeoutMs = 5000,
pollIntervalMs = 100,
isPidRunning = defaultIsPidRunning,
sleep = defaultSleep,
platform = process.platform,
} = options;
if (platform !== "win32") {
try {
process.kill(pid, "SIGTERM");
} catch {
// Process already gone — nothing to escalate.
return;
}
}
const start = Date.now();
while (Date.now() - start < timeoutMs && isPidRunning(pid)) {
await sleep(pollIntervalMs);
}
if (isPidRunning(pid)) {
try {
process.kill(pid, "SIGKILL");
} catch {
// Already gone.
}
}
}

View File

@@ -0,0 +1,23 @@
import { test } from "node:test";
import assert from "node:assert/strict";
// #8045: on Windows, closing the console window delivers CTRL_CLOSE_EVENT, which
// Node/libuv maps to a JS-visible "SIGHUP" event (confirmed: nodejs/node#10165,
// Node process docs — "SIGHUP is generated on Windows when the console window is
// closed"). Before this fix, initGracefulShutdown() only registered SIGTERM/SIGINT,
// so the "close the window" path never ran cleanup() (WAL checkpoint(TRUNCATE) +
// closeDbInstance()), leaving storage.sqlite's WAL un-checkpointed for the next launch.
test("initGracefulShutdown registers a SIGHUP handler (Windows console-close path)", async () => {
const before = process.listenerCount("SIGHUP");
const { initGracefulShutdown } = await import("../../src/lib/gracefulShutdown.ts");
initGracefulShutdown();
const after = process.listenerCount("SIGHUP");
assert.ok(
after > before,
`Expected initGracefulShutdown() to add a SIGHUP listener (before=${before}, after=${after}).`
);
// Clean up: remove all SIGHUP listeners added by this test so it doesn't leak
// into other test files sharing the same process.
process.removeAllListeners("SIGHUP");
});

View File

@@ -0,0 +1,97 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { stopProcessGracefully } from "../../src/shared/platform/windowsProcess.ts";
// #8045: on win32, process.kill(pid, "SIGTERM") is documented to unconditionally
// force-terminate the target — never a real, interceptable signal. Sending it
// immediately races (and beats) the target's own async graceful-shutdown WAL
// checkpoint. stopProcessGracefully() must NOT send SIGTERM on win32.
test("stopProcessGracefully: on win32 does NOT send SIGTERM, only polls then escalates to SIGKILL", async () => {
const signalsSent: string[] = [];
let running = true;
const sleeps: number[] = [];
await stopProcessGracefully({
pid: 4242,
timeoutMs: 300,
pollIntervalMs: 50,
platform: "win32",
isPidRunning: () => running,
sleep: async (ms) => {
sleeps.push(ms);
// Simulate the process exiting on its own shortly after the first poll,
// mimicking its own SIGHUP/CTRL_CLOSE_EVENT-driven graceful shutdown.
if (sleeps.length >= 2) running = false;
},
});
assert.ok(
!signalsSent.includes("SIGTERM"),
"must not send SIGTERM on win32 (it force-kills unconditionally there)"
);
});
test("stopProcessGracefully: on win32 escalates to SIGKILL if the process never exits", async () => {
const killed: Array<{ pid: number; signal: string }> = [];
const originalKill = process.kill;
// @ts-expect-error — test-only override to observe signals without touching a real PID.
process.kill = (pid: number, signal?: string | number) => {
killed.push({ pid, signal: String(signal) });
return true;
};
try {
await stopProcessGracefully({
pid: 4243,
timeoutMs: 100,
pollIntervalMs: 20,
platform: "win32",
isPidRunning: () => true, // never exits on its own
sleep: async () => {},
});
} finally {
process.kill = originalKill;
}
assert.ok(
killed.some((k) => k.signal === "SIGKILL"),
`expected an eventual SIGKILL escalation, got: ${JSON.stringify(killed)}`
);
assert.ok(
!killed.some((k) => k.signal === "SIGTERM"),
"must never send SIGTERM on win32"
);
});
test("stopProcessGracefully: on non-win32 sends SIGTERM immediately (unchanged POSIX behavior)", async () => {
const killed: Array<{ pid: number; signal: string }> = [];
const originalKill = process.kill;
// @ts-expect-error — test-only override.
process.kill = (pid: number, signal?: string | number) => {
killed.push({ pid, signal: String(signal) });
return true;
};
let running = true;
try {
await stopProcessGracefully({
pid: 4244,
timeoutMs: 200,
pollIntervalMs: 20,
platform: "linux",
isPidRunning: () => running,
sleep: async () => {
running = false;
},
});
} finally {
process.kill = originalKill;
}
assert.equal(killed[0]?.signal, "SIGTERM", "must send SIGTERM immediately on POSIX");
assert.ok(
!killed.some((k) => k.signal === "SIGKILL"),
"must not escalate to SIGKILL once the process exited"
);
});