From 8aad2a7c372a072eb3fcc57511e672a6b59649cb Mon Sep 17 00:00:00 2001 From: Neil Daquioag Date: Sat, 18 Apr 2026 15:43:43 +0800 Subject: [PATCH] fix(shutdown): make runCleanup idempotent under concurrent calls (LIB-HIGH-003) Addresses the actionable part of LIB-HIGH-003 from the deep top-level lib audit. The original finding mixed three concerns: ordering, idempotence, and signal/ beforeExit semantics. The smallest safe fix is to eliminate the actual race: concurrent runCleanup() calls can currently double-run cleanup handlers. Fix: cache the in-flight cleanup promise and return it to overlapping callers. Preserve current FIFO ordering and current signal/beforeExit semantics to avoid changing the shutdown contract in this PR. Added a focused regression test that concurrent callers receive the same in-flight promise and only one cleanup run executes. --- lib/shutdown.ts | 29 ++++++++++++++++++++--------- test/shutdown.test.ts | 22 +++++++++++++++++++++- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 66965627..ff1f6f7d 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -2,6 +2,7 @@ type CleanupFn = () => void | Promise; const cleanupFunctions: CleanupFn[] = []; let shutdownRegistered = false; +let cleanupPromise: Promise | null = null; export function registerCleanup(fn: CleanupFn): void { cleanupFunctions.push(fn); @@ -15,17 +16,27 @@ export function unregisterCleanup(fn: CleanupFn): void { } } -export async function runCleanup(): Promise { - const fns = [...cleanupFunctions]; - cleanupFunctions.length = 0; +export function runCleanup(): Promise { + if (cleanupPromise) { + return cleanupPromise; + } + + cleanupPromise = (async () => { + const fns = [...cleanupFunctions]; + cleanupFunctions.length = 0; - for (const fn of fns) { - try { - await fn(); - } catch { - // Ignore cleanup errors during shutdown + for (const fn of fns) { + try { + await fn(); + } catch { + // Ignore cleanup errors during shutdown + } } - } + })().finally(() => { + cleanupPromise = null; + }); + + return cleanupPromise; } function ensureShutdownHandler(): void { diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index c64ecf41..6d85d473 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -29,7 +29,7 @@ describe("Graceful shutdown", () => { expect(fn).not.toHaveBeenCalled(); }); - it("runs multiple cleanup functions in order", async () => { + it("runs multiple cleanup functions in registration order", async () => { const order: number[] = []; registerCleanup(() => { order.push(1); }); registerCleanup(() => { order.push(2); }); @@ -65,6 +65,26 @@ describe("Graceful shutdown", () => { expect(getCleanupCount()).toBe(0); }); + it("returns the same promise for concurrent runCleanup calls", async () => { + let release!: () => void; + const blocker = new Promise((resolve) => { + release = resolve; + }); + const cleanupFn = vi.fn(async () => { + await blocker; + }); + registerCleanup(cleanupFn); + + const first = runCleanup(); + const second = runCleanup(); + + expect(first).toBe(second); + expect(cleanupFn).toHaveBeenCalledTimes(1); + + release(); + await first; + }); + it("unregister is no-op for non-registered function", () => { const fn = vi.fn(); unregisterCleanup(fn);