diff --git a/scripts/watch-node.mjs b/scripts/watch-node.mjs index aa90d15f9a7..c8fca7e627a 100644 --- a/scripts/watch-node.mjs +++ b/scripts/watch-node.mjs @@ -14,6 +14,7 @@ const WATCH_RESTARTABLE_CHILD_SIGNALS = new Set(["SIGTERM"]); const WATCH_IGNORED_PATH_SEGMENTS = new Set([".git", "dist", "node_modules"]); const WATCH_LOCK_WAIT_MS = 5_000; const WATCH_LOCK_POLL_MS = 100; +const WATCH_SHUTDOWN_KILL_GRACE_MS = 5_000; const WATCH_LOCK_DIR = path.join(".local", "watch-node"); const AUTO_DOCTOR_DISABLE_VALUES = new Set(["0", "false", "no", "off"]); @@ -292,12 +293,17 @@ export async function runWatchMain(params = {}) { let watcher = null; let lockHandle = null; let autoDoctorAttempted = false; + let shutdownExitCode = null; + let shutdownKillTimer = null; const settle = (code) => { if (settled) { return; } settled = true; + if (shutdownKillTimer) { + clearTimeout(shutdownKillTimer); + } if (onSigInt) { deps.process.off("SIGINT", onSigInt); } @@ -309,6 +315,30 @@ export async function runWatchMain(params = {}) { resolve(code); }; + const requestShutdown = (code) => { + shuttingDown = true; + shutdownExitCode = code; + if (!watchProcess || typeof watchProcess.kill !== "function") { + settle(code); + return; + } + watchProcess.kill(WATCH_RESTART_SIGNAL); + shutdownKillTimer ??= setTimeout(() => { + shutdownKillTimer = null; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill("SIGKILL"); + } + }, WATCH_SHUTDOWN_KILL_GRACE_MS); + }; + + const settleIfShuttingDown = () => { + if (!shuttingDown || shutdownExitCode === null) { + return false; + } + settle(shutdownExitCode); + return true; + }; + const startRunner = () => { watchProcess = deps.spawn(deps.process.execPath, buildRunnerArgs(deps.args), { cwd: deps.cwd, @@ -322,7 +352,7 @@ export async function runWatchMain(params = {}) { }); watchProcess.on("exit", (exitCode, exitSignal) => { watchProcess = null; - if (shuttingDown) { + if (settleIfShuttingDown()) { return; } if (restartRequested || shouldRestartAfterChildExit(exitCode, exitSignal)) { @@ -339,11 +369,7 @@ export async function runWatchMain(params = {}) { }; const handleWatcherError = () => { - shuttingDown = true; - if (watchProcess && typeof watchProcess.kill === "function") { - watchProcess.kill(WATCH_RESTART_SIGNAL); - } - settle(1); + requestShutdown(1); }; const rejectWatcherStartupError = (err) => { @@ -396,7 +422,7 @@ export async function runWatchMain(params = {}) { }); watchProcess.on("exit", (exitCode, exitSignal) => { watchProcess = null; - if (shuttingDown) { + if (settleIfShuttingDown()) { return; } if (exitCode === 0 && !exitSignal) { @@ -450,18 +476,10 @@ export async function runWatchMain(params = {}) { }; const onSigInt = () => { - shuttingDown = true; - if (watchProcess && typeof watchProcess.kill === "function") { - watchProcess.kill(WATCH_RESTART_SIGNAL); - } - settle(130); + requestShutdown(130); }; const onSigTerm = () => { - shuttingDown = true; - if (watchProcess && typeof watchProcess.kill === "function") { - watchProcess.kill(WATCH_RESTART_SIGNAL); - } - settle(143); + requestShutdown(143); }; deps.process.on("SIGINT", onSigInt); diff --git a/test/scripts/watch-node.test.ts b/test/scripts/watch-node.test.ts new file mode 100644 index 00000000000..9f606bea1ac --- /dev/null +++ b/test/scripts/watch-node.test.ts @@ -0,0 +1,89 @@ +import { EventEmitter } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runWatchMain } from "../../scripts/watch-node.mjs"; + +class FakeProcess extends EventEmitter { + execPath = process.execPath; + pid = 12345; + stderr = { + write: () => true, + }; +} + +class FakeChild extends EventEmitter { + signals: string[] = []; + + kill(signal: string): boolean { + this.signals.push(signal); + if (signal === "SIGKILL") { + this.emit("exit", null, "SIGKILL"); + } + return true; + } +} + +describe("watch-node shutdown cleanup", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("waits for the child and escalates when interrupted children ignore SIGTERM", async () => { + vi.useFakeTimers(); + const fakeProcess = new FakeProcess(); + const child = new FakeChild(); + let resolvedCode: number | undefined; + + const run = runWatchMain({ + args: ["gateway"], + createWatcher: () => ({ close: async () => {}, on: () => {} }), + lockDisabled: true, + process: fakeProcess as unknown as NodeJS.Process, + spawn: () => child as never, + }).then((code) => { + resolvedCode = code; + return code; + }); + + fakeProcess.emit("SIGTERM"); + await vi.advanceTimersByTimeAsync(4_999); + expect(resolvedCode).toBeUndefined(); + expect(child.signals).toEqual(["SIGTERM"]); + + await vi.advanceTimersByTimeAsync(1); + await expect(run).resolves.toBe(143); + expect(child.signals).toEqual(["SIGTERM", "SIGKILL"]); + }); + + it("waits for the auto-doctor child when interrupted during repair", async () => { + vi.useFakeTimers(); + const fakeProcess = new FakeProcess(); + const runner = new FakeChild(); + const doctor = new FakeChild(); + const children = [runner, doctor]; + let resolvedCode: number | undefined; + + const run = runWatchMain({ + args: ["gateway"], + createWatcher: () => ({ close: async () => {}, on: () => {} }), + env: {}, + lockDisabled: true, + process: fakeProcess as unknown as NodeJS.Process, + spawn: () => children.shift() as never, + }).then((code) => { + resolvedCode = code; + return code; + }); + + runner.emit("exit", 1, null); + expect(children).toHaveLength(0); + + fakeProcess.emit("SIGTERM"); + await vi.advanceTimersByTimeAsync(4_999); + expect(resolvedCode).toBeUndefined(); + expect(doctor.signals).toEqual(["SIGTERM"]); + + await vi.advanceTimersByTimeAsync(1); + await expect(run).resolves.toBe(143); + expect(doctor.signals).toEqual(["SIGTERM", "SIGKILL"]); + }); +});