fix: force process exit when a worker thread can not be terminated

packages/beacon-node/src/index.ts

@@ -19,3 +19,4 @@ export {type NodeJsLibp2pOpts, createNodeJsLibp2p} from "./network/index.js";
 export * from "./node/index.js";
 // Export type util for CLI - TEMP move to lodestar-types eventually
 export {getStateSlotFromBytes, getStateTypeFromBytes} from "./util/multifork.js";
+export {hasWorkerTerminationFailed} from "./util/workerEvents.js";

packages/beacon-node/src/util/workerEvents.ts

@@ -117,6 +117,21 @@ export function wireEventsOnMainThread<EventData>(
  * @returns `false` if it could not be terminated, the worker thread is then still running and the
  * caller has to decide how to proceed without it.
  */
+/**
+ * How many worker threads are believed to be still running after termination failed. It is a
+ * property of the process, not of any one instance, since `process.exit()` joins every worker
+ * regardless of which one is stuck.
+ *
+ * A count rather than a flag so that a worker terminating late, after its own attempt gave up but
+ * while the rest of the shutdown is still running, does not clear the record for a different
+ * worker that really is stuck.
+ */
+let unterminatedWorkers = 0;
+
+export function hasWorkerTerminationFailed(): boolean {
+  return unterminatedWorkers > 0;
+}
+
 export async function terminateWorkerThread({
   worker,
   retryMs,
@@ -151,5 +166,12 @@ export async function terminateWorkerThread({
   }
 
   logger?.debug(`Worker thread failed to terminate in ${retryCount * retryMs}ms`);
+  unterminatedWorkers++;
+  // It may still terminate while the rest of the shutdown runs, in which case it can no longer
+  // block `process.exit()` and there is nothing to force
+  void terminated.then(() => {
+    unterminatedWorkers--;
+    logger?.debug("Worker thread terminated after the deadline");
+  });
   return false;
 }

packages/beacon-node/test/unit/util/workerEvents.test.ts

@@ -51,4 +51,33 @@ describe("util / workerEvents / terminateWorkerThread", () => {
     await expect(promise).resolves.toBe(false);
     expect(Thread.terminate).toHaveBeenCalledTimes(retryCount);
   });
+
+  it("stops reporting a failure once the worker terminates after the deadline", async () => {
+    // `unterminatedWorkers` is module state and the test above leaves a worker permanently
+    // unterminated on purpose, so take a fresh module instance to isolate the count
+    vi.resetModules();
+    const {terminateWorkerThread: terminate, hasWorkerTerminationFailed: hasFailed} = await import(
+      "../../../src/util/workerEvents.js"
+    );
+
+    // no termination event during the retries, so it gives up and records the worker as running
+    let emit: ((event: {type: string}) => void) | undefined;
+    vi.mocked(Thread.events).mockReturnValue({
+      subscribe: (cb: (event: {type: string}) => void) => {
+        emit = cb;
+        return {unsubscribe: () => {}};
+      },
+    } as unknown as ReturnType<typeof Thread.events>);
+    vi.mocked(Thread.terminate).mockReturnValue(new Promise<void>(() => {}) as never);
+
+    const promise = terminate({worker, retryMs, retryCount});
+    await vi.advanceTimersByTimeAsync(retryMs * retryCount);
+    await expect(promise).resolves.toBe(false);
+    expect(hasFailed()).toBe(true);
+
+    // the worker terminates late, while the rest of the shutdown is still running
+    emit?.({type: "termination"});
+    await vi.advanceTimersByTimeAsync(0);
+    expect(hasFailed()).toBe(false);
+  });
 });

packages/cli/src/cmds/beacon/handler.ts

@@ -2,7 +2,7 @@ import path from "node:path";
 import {getHeapStatistics} from "node:v8";
 import {SignableENR} from "@chainsafe/enr";
 import {hasher} from "@chainsafe/persistent-merkle-tree";
-import {BeaconDb, BeaconNode} from "@lodestar/beacon-node";
+import {BeaconDb, BeaconNode, hasWorkerTerminationFailed} from "@lodestar/beacon-node";
 import {ChainForkConfig, createBeaconConfig} from "@lodestar/config";
 import {LevelDbController} from "@lodestar/db/controller/level";
 import {LoggerNode, getNodeLogger} from "@lodestar/logger/node";
@@ -146,6 +146,23 @@ export async function beaconHandler(args: BeaconArgs & GlobalArgs): Promise<void
         try {
           await node.close();
           logger.debug("Beacon node closed");
+          if (hasWorkerTerminationFailed()) {
+            // The state is archived and the db is closed by now, but `process.exit()` joins every
+            // worker via `stop_sub_worker_contexts()`, so a worker that could not be terminated
+            // blocks it until the process manager gives up. Leave on our own terms instead.
+            logger.warn("Worker thread still running, forcing process exit");
+            // A signal always reports 128+signum, the code can not be chosen, but SIGTERM (143) is
+            // what a container reports for a normal stop while SIGKILL (137) reads as a crash
+            process.removeAllListeners("SIGTERM");
+            process.kill(process.pid, "SIGTERM");
+            // Only reached if the signal was not delivered. The kernel discards signals sent to
+            // pid 1 that have no handler installed, including SIGKILL, so this is expected when
+            // running as pid 1 in a container without an init. `process.exit()` below then still
+            // blocks on the join, which is no worse than before this branch existed.
+            logger.error("Could not force exit, run with an init (docker --init, tini) so the process is not pid 1", {
+              pid: process.pid,
+            });
+          }
           // Explicitly exit until active handles issue is resolved
           // See https://github.com/ChainSafe/lodestar/issues/5642
           process.exit(0);