From 7ebc6c2a3386090870015a8541dcd580ca14fc0d Mon Sep 17 00:00:00 2001 From: zumbrunn Date: Thu, 22 Oct 2026 10:41:05 +0200 Subject: [PATCH] fix(serve): exit explicitly after a graceful shutdown On SIGTERM `dune serve` closed the listener, drained, printed "shutdown complete" and then simply returned, leaving the process alive for as long as the event loop had work. One ref'd plugin timer (plugin-admin's collab eviction setInterval, fixed in 3.1.3) was enough to keep it alive forever at 0% CPU with no listener: an "idle" dune process, which a second SIGTERM was needed to end. Roughly 45 of them had piled up on a developer machine. Exit explicitly once the drain is done, in both the single-site and multi-site paths. By then the listener is closed and in-flight requests are drained (or the drain deadline has passed); there is nothing left worth waiting for, and core should not depend on every plugin unref'ing its timers. The existing graceful-shutdown tests re-implement their helpers locally and never run the real command, which is how this went unnoticed. The new test runs the real `dune serve` as a subprocess with a plugin that holds a ref'd timer (so it does not depend on which plugin-admin version is installed), sends one SIGTERM, and requires a clean exit. Against the old code it is still alive 15 s later and fails; with the fix it passes in under a second. The multi-site path has the same change but no end-to-end test. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 16 +++ deno.json | 2 +- src/cli/serve.ts | 17 ++++ tests/cli/serve_exit_after_shutdown_test.ts | 102 ++++++++++++++++++++ 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/cli/serve_exit_after_shutdown_test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e2b3e8fc..0b18c52a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ split, begin at v1.0.0 — see [VERSIONING.md](VERSIONING.md) for exactly what --- +## [0.34.15] — 2026-10-22 + +### Fixed + +- **`dune serve` did not exit after a graceful shutdown if anything was still + running.** On SIGTERM it closed the listener, drained, printed `shutdown + complete`, and then just returned, so the process lived only as long as the + event loop had work. One ref'd plugin timer (`@dune/plugin-admin`'s collab + eviction interval, fixed in 3.1.3) was enough to keep it alive forever at 0% + CPU with no listener — an "idle" instance that a *second* SIGTERM was needed + to end. `serve` now exits explicitly once the drain is done (single-site and + multi-site), so no plugin timer can hold the process open. Covered by a test + that runs the real `dune serve`, gives it a plugin holding a ref'd timer, and + checks that one SIGTERM ends it. (The multi-site shutdown path has the same + change but no end-to-end test.) + ## [0.34.14] — 2026-10-21 ### Security diff --git a/deno.json b/deno.json index da4f135c..63e14aa0 100644 --- a/deno.json +++ b/deno.json @@ -1,6 +1,6 @@ { "name": "@dune/core", - "version": "0.34.14", + "version": "0.34.15", "license": "MIT", "author": "zumbrunn (https://zumbrunn.com)", "minimumDependencyAge": { diff --git a/src/cli/serve.ts b/src/cli/serve.ts index 05ae1691..b44331ec 100644 --- a/src/cli/serve.ts +++ b/src/cli/serve.ts @@ -95,6 +95,21 @@ async function drainInFlight( } } +/** + * End the process once a graceful shutdown has finished draining. + * + * Returning from `serveCommand()` and letting the event loop empty is not + * enough: any plugin timer or handle that is still ref'd keeps the process + * alive, and it then sits there forever at 0% CPU with no listener (an "idle" + * instance that a second SIGTERM is needed to end). plugin-admin's collab + * eviction interval did exactly this until 3.1.3. By this point the listener + * is closed and in-flight requests are drained (or the drain deadline has + * passed), so there is nothing left worth waiting for. + */ +function exitAfterShutdown(): void { + Deno.exit(0); +} + // --------------------------------------------------------------------------- // Main command // --------------------------------------------------------------------------- @@ -174,6 +189,7 @@ export async function serveCommand(root: string, options: ServeOptions = {}) { await drainInFlight(() => inFlight, drainDeadlineMs); cleanup(); console.log("[dune] shutdown complete"); + exitAfterShutdown(); return; } @@ -388,4 +404,5 @@ export async function serveCommand(root: string, options: ServeOptions = {}) { cleanup(); console.log("[dune] shutdown complete"); + exitAfterShutdown(); } diff --git a/tests/cli/serve_exit_after_shutdown_test.ts b/tests/cli/serve_exit_after_shutdown_test.ts new file mode 100644 index 00000000..4e532119 --- /dev/null +++ b/tests/cli/serve_exit_after_shutdown_test.ts @@ -0,0 +1,102 @@ +/** + * `dune serve` must exit after a graceful shutdown, whatever plugins left + * running. + * + * Graceful shutdown closes the listener and prints "shutdown complete", but + * serveCommand() used to just return, so the process lived only as long as the + * event loop had work. One plugin timer that was never unref'd (plugin-admin's + * collab eviction interval, until 3.1.3) kept it alive forever: an "idle" dune + * process with no port, at 0% CPU, that a single SIGTERM never ended. + * + * Runs the real `serve` command as a subprocess. The plugin below holds a + * ref'd timer on purpose, standing in for any plugin that does, so this test + * does not depend on which plugin-admin version is installed. + */ + +import { assertEquals } from "https://deno.land/std@0.224.0/assert/mod.ts"; +import { dirname, join } from "@std/path"; + +const PORT = 18_952; +const READY_TIMEOUT_MS = 60_000; +const EXIT_TIMEOUT_MS = 15_000; + +const files: Record = { + "config/site.yaml": "title: Exit Test\nurl: http://localhost:18952\nplugins:\n - src: ./plugins/hold.ts\n", + // Fresh reads compilerOptions from the site root on the first build. + "deno.json": JSON.stringify({ compilerOptions: { jsx: "react-jsx", jsxImportSource: "preact" } }), + "content/01.home/default.md": "---\ntitle: Home\n---\nhi\n", + // A plugin whose timer is ref'd and never cleared. + "plugins/hold.ts": `setInterval(() => {}, 3_600_000);\nexport default { name: "hold", version: "1.0.0", hooks: {} };\n`, +}; + +Deno.test({ + name: "dune serve exits after SIGTERM even when a plugin holds a ref'd timer", + sanitizeOps: false, + sanitizeResources: false, + fn: async () => { + const root = await Deno.makeTempDir({ prefix: "dune_test_serve_exit_" }); + let child: Deno.ChildProcess | undefined; + let output = ""; + let drains: Promise | undefined; + try { + for (const [rel, text] of Object.entries(files)) { + await Deno.mkdir(dirname(join(root, rel)), { recursive: true }); + await Deno.writeTextFile(join(root, rel), text); + } + const duneDenoJson = join(import.meta.dirname!, "..", "..", "deno.json"); + const cliPath = join(import.meta.dirname!, "..", "..", "src", "cli.ts"); + + child = new Deno.Command(Deno.execPath(), { + args: ["run", "-A", `--config=${duneDenoJson}`, cliPath, "serve", "--root", root, "--port", String(PORT)], + env: { ...Deno.env.toObject(), DUNE_CONFIG_APPLIED: "1" }, + stdout: "piped", + stderr: "piped", + }).spawn(); + const decoder = new TextDecoder(); + const drain = async (s: ReadableStream) => { + for await (const c of s) output += decoder.decode(c); + }; + drains = Promise.all([drain(child.stdout), drain(child.stderr)]); + + // Ready? + let ready = false; + const readyDeadline = Date.now() + READY_TIMEOUT_MS; + while (Date.now() < readyDeadline) { + try { + const res = await fetch(`http://localhost:${PORT}/`, { signal: AbortSignal.timeout(1500) }); + await res.body?.cancel(); + ready = true; + break; + } catch { + await new Promise((r) => setTimeout(r, 250)); + } + } + assertEquals(ready, true, `server never came up:\n${output}`); + + // One SIGTERM, as a service manager or a plain `kill` would send. + child.kill("SIGTERM"); + const outcome = await Promise.race([ + child.status.then((s) => ({ exited: true as const, code: s.code })), + new Promise<{ exited: false }>((r) => setTimeout(() => r({ exited: false }), EXIT_TIMEOUT_MS)), + ]); + + assertEquals( + outcome.exited, + true, + `process was still alive ${EXIT_TIMEOUT_MS}ms after SIGTERM (a ref'd plugin timer kept it running):\n${output}`, + ); + assertEquals((outcome as { code: number }).code, 0); + assertEquals(output.includes("shutdown complete"), true, output); + child = undefined; + } finally { + if (child) { + try { + child.kill("SIGKILL"); + } catch { /* already gone */ } + await child.status.catch(() => {}); + } + await drains?.catch(() => {}); + await Deno.remove(root, { recursive: true }).catch(() => {}); + } + }, +});