fix: bound network worker termination to prevent shutdown hang

[lodekeeper]

Motivation

Lodestar can hang on graceful shutdown: terminating network worker is logged but terminated network worker never is, the node keeps ticking Synced (slot -N), and it only exits when force-killed (~5 min via docker SIGKILL). On ethpandaops devnets (glamsterdam-devnet-6, lodestar-geth nodes) this reproduces on every shutdown. Recurrence of #5775 / #6053, reintroduced by the libp2p v3 + QUIC upgrades.

Root cause

terminateWorkerThread awaited Thread.terminate(worker) outside the timeout race. Thread.terminate() delegates to Node's worker.terminate(), which forcibly stops the worker at the next JS safepoint but cannot preempt a worker blocked inside a synchronous native (napi) call. The network worker's only native transport is @chainsafe/libp2p-quic (Rust/quinn), so when a QUIC native call is in flight at terminate time the promise never resolves — and with the await outside the race the retryCount * retryMs budget is unreachable → unbounded hang.

A getActiveResourcesInfo() / _getActiveHandles() probe in the worker at shutdown showed no leftover UDP/TCP libuv handle — so it isn't a lingering handle keeping the loop alive, it's worker.terminate() being unable to preempt native code. The hang did not reproduce locally on unstable (6 SIGTERMs mid-QUIC-download, all terminated in 6–14 ms), so it's timing/condition-specific to the affected build; QUIC is implicated by elimination (@libp2p/tcp is pure-JS and can't block a forced terminate; discv5 is a separate worker, already closed earlier).

Fix

1. Bound the terminate — race Thread.terminate() inside the timeout so a stuck worker.terminate() fails within retryCount * retryMs (~3 s) instead of hanging.
2. Return a boolean instead of throwing. On the throw path BeaconNode.close() rejects at await this.network.close(), so chain.persistToDisk() etc. are skipped and the shutdown handler falls to its catch → db.close() + process.exit(1). Returning false lets node.close() run to completion (state persisted, everything closed) → process.exit(0). For a node that hangs on every shutdown, that's the difference between a clean exit-0 and an error exit-1 (skipping the state persist) on every restart.
3. unref() the worker when it can't be terminated so a still-running (native-blocked) worker can't keep the event loop alive. Harmless given the handler's explicit process.exit, but makes BeaconNode.close() self-sufficient and is a step toward removing that explicit-exit workaround (Beacon node active handles after close sequence is completed #5642).

Testing

• Unit test (workerEvents.test.ts): a worker that never terminates returns false in bounded time (~retryCount * retryMs) instead of hanging; the normal path returns true.
• Verified the unref semantics directly (a live worker keeps the process alive; unref lets it exit).
• biome + tsgo clean for the changed files.

Follow-up (not blocking)

Pinning the exact @chainsafe/libp2p-quic / quinn native frame (a worker-thread stack on a box during a live hang) for an upstream report. The bound here fixes the hang regardless of which native call it is.

[gemini-code-assist]

Code Review

This pull request updates terminateWorkerThread to race Thread.terminate(worker) against the timeout, preventing indefinite hangs when a worker is stuck in a synchronous native call, and adds corresponding unit tests. The review feedback recommends utilizing Vitest's fake timers (vi.useFakeTimers and vi.advanceTimersByTimeAsync) in the new tests to eliminate real-time delays and prevent potential flakiness in CI environments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Since afterEach already restores real timers via vi.useRealTimers(), it looks like the intention was to use fake timers. Enabling fake timers in beforeEach allows us to run the timeout tests deterministically and instantly without relying on real-world clock drift or introducing flakiness in CI environments.

Suggested change

	beforeEach(() => {
	vi.clearAllMocks();
	});
	beforeEach(() => {
	vi.clearAllMocks();
	vi.useFakeTimers();
	});

[lodekeeper]

Good catch — done in 46bdc72. You're right that the afterEach(vi.useRealTimers()) was vestigial (fake timers were never enabled in beforeEach), so I wired in vi.useFakeTimers() there to make the setup consistent.

[gemini-code-assist]

With fake timers enabled, we can avoid using Date.now() and real-time delays entirely. We can instead use vi.advanceTimersByTimeAsync to fast-forward the timers deterministically, making the test faster and completely immune to CI CPU scheduling delays.

  it("throws bounded instead of hanging when Thread.terminate() never resolves", async () => {
    // Simulate a worker stuck in a blocking native call: terminate() never resolves and no
    // termination event is ever emitted. The old implementation awaited terminate() outside the
    // race and would hang forever; the fix must fall through to the throw within the retry budget.
    mockEvents([]);
    vi.mocked(Thread.terminate).mockReturnValue(new Promise<void>(() => {}) as never);

    const promise = terminateWorkerThread({worker, retryMs, retryCount});

    // Fast-forward time to trigger the timeouts sequentially
    await vi.advanceTimersByTimeAsync(retryMs * retryCount);

    await expect(promise).rejects.toThrow(
      `Worker thread failed to terminate in ${retryCount * retryMs}ms.`
    );
    // Must have retried the terminate call each iteration and stayed bounded.
    expect(Thread.terminate).toHaveBeenCalledTimes(retryCount);
  });

[lodekeeper]

Applied in 46bdc72, adapted to the current code. The test now creates the promise, drives the per-retry sleep(retryMs) timeouts with await vi.advanceTimersByTimeAsync(retryMs * retryCount), and asserts on the result. Two deltas from the snippet:

1. terminateWorkerThread now returns false instead of throwing (changed in 6ec9952, after this review), so the assertion is resolves.toBe(false) rather than rejects.toThrow(...).
2. Dropped the wall-clock Date.now() bound — under fake timers it's meaningless (Date.now() is faked), and the bound is now enforced deterministically by the controlled timer advancement.

Verified 2/2 green; the bounded test dropped from ~60ms real time to ~2ms.

[lodekeeper]

Thanks for the review — applied the fake-timer suggestion in 46bdc72: vi.useFakeTimers() in beforeEach + vi.advanceTimersByTimeAsync to drive the retry sleeps, so the bounded-shutdown test is deterministic on CI instead of relying on real-time delays. Adapted for the throw→return-false change (6ec9952). Replied inline on both comments; tests green.

[nflaig]

this seems like a workaround to me, we should rather fix the root cause

[lodekeeper]

You're right, the bound is a workaround — agreed. Laying out where the root cause actually sits so we can decide whether to ship it as a safety net vs. block on the real fix:

Graceful shutdown already runs before terminate. NetworkCoreWorkerHandler.close() awaits the worker's NetworkCore.close() (→ gossip.stop() → reqResp.stop() → libp2p.stop(), which stops the QUIC transport) and only then calls terminateWorkerThread(). So the hang is at Worker.terminate() after a fully-awaited libp2p/QUIC stop — not a case of terminating while the transport is still live. A JS reordering on our side won't move the needle.

Why terminate itself hangs. Worker.terminate() can only tear the isolate down at a JS safepoint. If the QUIC worker's native (napi) side has left a blocking synchronous call or an unjoined native thread, V8 never reaches a safepoint and terminate() never resolves — which matches what we see on prod: NetworkCore.close() returns, then terminate hangs until docker SIGKILL (~2min). That points at @chainsafe/libp2p-quic's native teardown, not Lodestar JS. (Open question worth pulling on upstream: does libp2p.stop() resolving actually guarantee the QUIC native resources are fully released, or is some cleanup detached?)

The true fix needs evidence we don't have yet: a native stack from a stuck worker on prod (gdb / eu-stack before SIGKILL) to pinpoint the blocking native call, then a fix upstream. It doesn't reproduce locally (a local handle probe found no leftover libuv/QUIC handle), so I can't get that stack without capturing it on a hanging lodestar-geth node — happy to coordinate that with Barnabas.

Case for shipping the bound now as defense-in-depth (explicitly not as "the fix"):

1. Today every shutdown on the affected nodes hangs ~2min → docker SIGKILL, which skips clean state persist. With the bound + return-false + unref, BeaconNode.close() runs to completion (persistToDisk + db.close) and we process.exit(0) — a bounded, clean shutdown with state saved, instead of a hard kill.
2. Even once the upstream fix lands, bounding terminate is correct hygiene — a misbehaving worker should never be able to hang shutdown indefinitely.

Suggestion: merge this as the safety net and track the root cause separately — I can open a tracking issue (capture native stack → upstream @chainsafe/libp2p-quic report) and link it here. But it's your call: if you'd rather hold the PR until we have the root-cause fix in hand, I'll pivot to getting the native stack off a stuck prod node first.

[nflaig]

@lodekeeper please open a PR on libp2p-quic instead to fix the root cause

[lodekeeper]

Opened the root-cause PR: ChainSafe/js-libp2p-quic#59.

Root cause: QuicTransport creates its dialer Client endpoints (per-family quinn::Endpoint = UDP socket + Tokio driver task) in the constructor and shares them across all dials, but implements no Startable lifecycle — so libp2p's component teardown (which only stops isStartable components) never stops it, and unlike the listener's Server (torn down in QuicListener.close()) nothing aborts the client endpoints. The driver keeps polling the socket forever, which is what stops the network worker from reaching a V8 teardown safepoint → Worker.terminate() hangs.

Fix: make the transport Startable and abort() both client endpoints in stop() (mirrors how the listener releases its Server; Client.abort() already exists). I couldn't reproduce the terminate-hang locally so I flagged that in the PR for maintainer validation, but this closes the endpoint leak that is the mechanism.

That makes #9582's bound pure defense-in-depth on top of the real fix — your call whether to keep it as a belt-and-suspenders safety net or close it once #59 lands.

[nflaig]

• we can revisit this if this is gonna be a problem after feat: implement Startable lifecycle for QuicTransport js-libp2p-quic#59 still

closing for now