Absorb a re-raised interrupt still in flight when a --load run ends (#2973)

clients/cli/__tests__/load-summary.test.ts

@@ -43,19 +43,34 @@ import {
  * Deliver a signal to the listener `withLoadInterrupt` added, and to nothing
  * else: `process.emit` would also reach the test worker's own SIGINT/SIGTERM
  * handlers, which end the worker. Returns false while no new listener exists.
+ *
+ * Calls the RAW listener, so a `process.once` one (the re-raise absorber,
+ * #2973) removes itself as a real delivery would; `process.listeners` hands
+ * back the unwrapped function, and calling that would leave it registered.
  */
 function signalLoadRun(
   signal: "SIGINT" | "SIGTERM",
   existing: NodeJS.SignalsListener[],
 ): boolean {
   const added = process
-    .listeners(signal)
-    .find((listener) => !existing.includes(listener));
+    .rawListeners(signal)
+    .find((raw) => !existing.includes(unwrapOnce(raw)));
   if (!added) return false;
-  (added as (s: NodeJS.Signals) => void)(signal);
+  (added as NodeJS.SignalsListener)(signal);
   return true;
 }
 
+/**
+ * The function a `process.once` wrapper calls, or the listener itself.
+ * `rawListeners` is typed `Function[]`, hence the single narrowing cast.
+ */
+function unwrapOnce(raw: object): NodeJS.SignalsListener {
+  const listener = raw as NodeJS.SignalsListener & {
+    listener?: NodeJS.SignalsListener;
+  };
+  return listener.listener ?? listener;
+}
+
 describe("success and failure latency (#2836)", () => {
   it("times successes apart from failures and counts ok throughput", async () => {
     const clock = fakeClock();
@@ -585,6 +600,8 @@ describe("withLoadInterrupt (#2834)", () => {
     const run = withLoadInterrupt(untilAborted);
     expect(process.listenerCount("SIGTERM")).toBe(before + 1);
     expect(signalLoadRun("SIGTERM", existing)).toBe(true);
+    // `when-exit`'s re-raise, landing while the run is still pending.
+    expect(signalLoadRun("SIGTERM", existing)).toBe(true);
     await expect(run).resolves.toEqual({
       reason: "interrupted",
       detail: "received SIGTERM",
@@ -632,6 +649,8 @@ describe("withLoadInterrupt (#2834)", () => {
     expect(exit).toHaveBeenCalledWith(EXIT_CODES.INTERRUPTED);
     finish();
     await run;
+    // The SIGINT re-raise never came, so deliver it to the absorber (#2973).
+    expect(signalLoadRun("SIGINT", ints)).toBe(true);
   });
 
   it("exits the real process by default on the second signal", async () => {
@@ -651,6 +670,28 @@ describe("withLoadInterrupt (#2834)", () => {
     expect(exit).toHaveBeenCalledWith(130);
     finish();
     await run;
+    // The SIGINT re-raise never came, so deliver it to the absorber (#2973).
+    expect(signalLoadRun("SIGINT", ints)).toBe(true);
+  });
+
+  it("absorbs a re-raise still in flight when the run ends (#2973)", async () => {
+    const exit = vi.fn();
+    const existing = process.listeners("SIGINT");
+    const before = existing.length;
+    const terms = process.listenerCount("SIGTERM");
+    // The run settles on the abort, before the re-raise has arrived.
+    const run = withLoadInterrupt(untilAborted, { quiet: true, exit });
+    signalLoadRun("SIGINT", existing);
+    await run;
+    // One listener is left, for the first signal only, so the pending copy
+    // never meets Node's default disposition (which would end the process by
+    // the signal rather than with 130).
+    expect(process.listenerCount("SIGINT")).toBe(before + 1);
+    expect(process.listenerCount("SIGTERM")).toBe(terms);
+    // The copy arrives — at any time, there is no window — and is swallowed.
+    expect(signalLoadRun("SIGINT", existing)).toBe(true);
+    expect(exit).not.toHaveBeenCalled();
+    expect(process.listenerCount("SIGINT")).toBe(before);
   });
 
   it("removes its listeners when the run throws", async () => {
@@ -723,6 +764,9 @@ describe("--load summary fixes (CLI)", () => {
       () => expect(signalLoadRun("SIGINT", existing)).toBe(true),
       { timeout: 10_000, interval: 200 },
     );
+    // `when-exit`'s re-raise, as a real process would receive it; without
+    // it the listener that absorbs a late copy (#2973) is left behind.
+    expect(signalLoadRun("SIGINT", existing)).toBe(true);
     const result = await run;
     expect(result.exitCode).toBe(EXIT_CODES.INTERRUPTED);
     const lines = result.stdout.trim().split("\n");

clients/cli/src/handlers/load-interrupt.ts

@@ -33,7 +33,22 @@ import type {
  *
  * The listeners exist only while `run` is pending, so a signal before or after
  * the run keeps Node's default behaviour, and an in-process caller (the
- * launcher, the test runner) is left with no stray listener.
+ * launcher, the test runner) is left with no stray listener — with one
+ * exception, below.
+ *
+ * ⚠️ **The re-raise can still be in flight when the run ends (#2973).** A
+ * signal a process sends itself is not handled synchronously: the kernel may
+ * hand it to any thread, and on a loaded machine that thread can wait several
+ * milliseconds to be scheduled — longer than the in-flight calls take to
+ * settle. Removing the last listener restores the default disposition, so a
+ * copy still pending at that moment killed the process by the signal, after
+ * the summary was out but before it could exit 130. So when the run ends
+ * without the copy having arrived, a one-shot listener is left to absorb it.
+ * Its removal is driven by the copy itself rather than by a timer, which is
+ * what makes it deterministic: however late the copy lands, it is swallowed,
+ * and the default disposition returns only after it. When nothing re-raises
+ * (a caller that invokes the listener directly), that one no-op listener is
+ * the stray left behind; it swallows one later signal and removes itself.
  */
 /** How long after the first signal a repeat of it is treated as a re-raise. */
 export const REPEAT_GRACE_MS = 500;
@@ -52,10 +67,13 @@ export async function withLoadInterrupt<T>(
   const exit = options.exit ?? ((code: number) => process.exit(code));
   const now = options.now ?? (() => performance.now());
   let first: { signal: NodeJS.Signals; at: number } | undefined;
+  // Whether the dependency's re-raise of `first` has been seen yet (#2973).
+  let echoed = false;
   const onSignal = (signal: NodeJS.Signals): void => {
     if (first !== undefined) {
       // A dependency's re-raise of the first signal, not a second press.
       if (signal === first.signal && now() - first.at < REPEAT_GRACE_MS) {
+        echoed = true;
         return;
       }
       exit(EXIT_CODES.INTERRUPTED);
@@ -78,11 +96,21 @@ export async function withLoadInterrupt<T>(
   try {
     return await run(controller.signal);
   } finally {
+    // Added BEFORE the `off` calls, so the signal never has zero listeners:
+    // the pending copy is delivered on whichever thread the kernel picks, not
+    // between two lines of this one, so even a synchronous gap with the
+    // default disposition restored would be a window it could land in.
+    if (first !== undefined && !echoed) {
+      process.once(first.signal, absorbReRaise);
+    }
     process.off("SIGINT", onSignal);
     process.off("SIGTERM", onSignal);
   }
 }
 
+/** Swallows the in-flight re-raise of the first signal (#2973). */
+function absorbReRaise(): void {}
+
 /**
  * After the summary is printed: exit 130 (`interrupted`) when the run was
  * cut short by Ctrl+C / SIGTERM, the conventional status for a process ended