From 126303b23d54584e9028a3873aa575d214600b86 Mon Sep 17 00:00:00 2001 From: zumbrunn Date: Thu, 22 Oct 2026 10:36:50 +0200 Subject: [PATCH] fix(collab): unref the session-eviction interval so it can't keep the process alive createCollabManager() started a ref'd 5-minute setInterval for session eviction and never cleared or unref'd it (the leading underscore on the variable was the tell). After `dune serve` received SIGTERM, closed its listener and printed "shutdown complete", that one timer kept the process alive forever at 0% CPU: an idle dune process with no listener. A second SIGTERM ended it. Found by recording who created each live timer after shutdown: this interval, created in mount() via createCollabManager, was the only one. Deno.unrefTimer() it: housekeeping must never decide process lifetime. There is no shutdown hook to clear it from (the manager has no dispose and core has no plugin-shutdown hook), and once unref'd there is nothing left to clear. The regression test runs in a subprocess because "does the process exit on its own?" is the property: it creates a collab manager and does nothing else, and must exit. Against the old code it hangs until the test's kill timeout; with the fix it exits in well under a second. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 +++++++ deno.json | 2 +- src/collab/manager.ts | 6 +++- tests/collab/eviction_timer_test.ts | 56 +++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 tests/collab/eviction_timer_test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6514cd6..ed8af71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ follows [Semantic Versioning](https://semver.org). --- +## [3.1.3] — 2026-10-22 + +### Fixed + +- **A `dune serve` process never exited after a graceful shutdown.** The + collab manager's session-eviction `setInterval` (every 5 minutes) was never + `unref()`'d, so after `dune serve` received SIGTERM, closed its listener and + printed `shutdown complete`, this one timer kept the process alive forever + at 0% CPU — an idle dune process holding nothing but a PID. (A second + SIGTERM ended it.) The timer is now unref'd, so it can never keep the + process alive. Found by listing the live timers after shutdown: it was the + only one. + ## [3.1.2] — 2026-10-15 ### Security diff --git a/deno.json b/deno.json index c614f71..1bf7189 100644 --- a/deno.json +++ b/deno.json @@ -1,6 +1,6 @@ { "name": "@dune/plugin-admin", - "version": "3.1.2", + "version": "3.1.3", "license": "MIT", "minimumDependencyAge": { "age": "P1D", diff --git a/src/collab/manager.ts b/src/collab/manager.ts index 2e514aa..da6f9ba 100644 --- a/src/collab/manager.ts +++ b/src/collab/manager.ts @@ -118,7 +118,7 @@ export function createCollabManager(options: CollabManagerOptions): CollabManage const connectionsPerUser = new Map(); // ── Session eviction timer (every 5 minutes) ─────────────────────────────── - const _evictionTimer = setInterval(() => { + const evictionTimer = setInterval(() => { const now = Date.now(); for (const [docId, session] of sessions) { if (session.clients.size === 0 && now - session.lastActivity > 5 * 60_000) { @@ -126,6 +126,10 @@ export function createCollabManager(options: CollabManagerOptions): CollabManage } } }, 5 * 60_000); + // Housekeeping must never keep the process alive. As a ref'd interval this + // outlived `dune serve`'s graceful shutdown (listener closed, "shutdown + // complete" printed) and the process then never exited. + Deno.unrefTimer(evictionTimer); // ── Load / get session ───────────────────────────────────────────────────── diff --git a/tests/collab/eviction_timer_test.ts b/tests/collab/eviction_timer_test.ts new file mode 100644 index 0000000..4d5c17d --- /dev/null +++ b/tests/collab/eviction_timer_test.ts @@ -0,0 +1,56 @@ +/** + * The collab manager's session-eviction interval must not keep the process + * alive. It used to be a ref'd 5-minute `setInterval`, so after `dune serve` + * finished its graceful shutdown (listener closed, "shutdown complete" + * printed) the process never exited: idle dune processes piled up. + * + * Runs in a subprocess because "does the process exit on its own?" is exactly + * the property under test; an in-process check can't see a ref'd timer. + */ + +import { assertEquals } from "jsr:@std/assert@^1"; +import { join, toFileUrl } from "jsr:@std/path@^1"; + +const REPO = join(import.meta.dirname!, "..", ".."); + +Deno.test({ + name: "createCollabManager: creating one does not keep the process alive", + sanitizeOps: false, + sanitizeResources: false, + fn: async () => { + const dir = await Deno.makeTempDir({ prefix: "plugin_admin_collab_timer_" }); + try { + const script = join(dir, "create.ts"); + await Deno.writeTextFile( + script, + `import { createCollabManager } from ${JSON.stringify(toFileUrl(join(REPO, "src", "collab", "mod.ts")).href)};\n` + + `// Stubs are fine: nothing touches them until a client connects.\n` + + `createCollabManager({ storage: {} as never, engine: {} as never, history: {} as never, contentDir: "content" });\n` + + `console.log("created");\n`, + ); + + const child = new Deno.Command(Deno.execPath(), { + args: ["run", "-A", `--config=${join(REPO, "deno.json")}`, script], + stdout: "piped", + stderr: "piped", + }).spawn(); + + // Generous: a cold dependency cache can make the first run slow. The + // regression is a hang of *minutes*, so any finite wait tells them apart. + const timer = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch { /* already exited */ } + }, 60_000); + const out = await child.output(); + clearTimeout(timer); + + const stdout = new TextDecoder().decode(out.stdout); + assertEquals(stdout.includes("created"), true, new TextDecoder().decode(out.stderr)); + assertEquals(out.success, true, "process should exit on its own once main finishes; a ref'd interval keeps it alive"); + assertEquals(out.signal, null, "process was killed by the test's timeout, meaning it never exited on its own"); + } finally { + await Deno.remove(dir, { recursive: true }).catch(() => {}); + } + }, +});