Beacon node exits uncontrolled (too early) during graceful shutdown

[nflaig]

Problem

Lodestar BN does not wait for all jobs to be finished during graceful shutdown.
This is problematic as some operations are not executed such as writing BLSToExecutionChanges to disk (see discord discussion)

Root cause

Lodestar uses threads package which installs SIGTERM signal listeners that exit the main process once all workers are terminated, see terminateWorkersAndMaster.

Related

• Gracefully shutdown process on SIGTERM andywer/threads.js#198
• Should really not install signal handlers that call process.exit with no way to opt out andywer/threads.js#388
• Do not kill the master process andywer/threads.js#324
• Threads Signal Handling Hack MatrixAI/Polykey-Desktop#79
• Signal handling improvements andywer/threads.js#329

[g11tech]

yes i confirm this to be issue as pointed by @nflaig : https://github.com/andywer/threads.js/blob/0d1a882f09273cb8482902ae2601fe4ffed6fbab/src/master/implementation.node.ts#L142

as soon as i comment out the lines in the local nodemodules, node close follow all close steps (but obviously workers keep going on still), else it seems to be processing this event on the first true async await call (await network.close)

So we might need to export the terminateWorkerAndMaster and call it explicity and remove these

process.on("SIGINT", () => terminateWorkersAndMaster())
  process.on("SIGTERM", () => terminateWorkersAndMaster())

``

[nflaig]

As discussed in Apr 4 standup we want to push a fix for this before the Shapella hardfork to address the BLSToExecutionChanges issue and other possible issues related to not shutting down properly.

For the quickfix we will have to

• release a new version of @chainsafe/threads @wemeetagain
• explicitly call process.exit after node.close succeeds (code: 0) or fails (code: 1) @nflaig
• retest that BLSToExecutionChanges are correctly persistet, best if @g11tech can confirm this once patch is implemented

In #5330 a cleaner solution will be implemented to make sure all active handlers are actually terminated and we don't need to rely on process.exit. I documented how to work with the branch in case anyone is bored and wants to go on an active handles hunt

[philknows]

Due to not being able to make the timeline for the Shapella hard fork (wasn't super urgent since other clients were able to handle the load as well), we can wait for #5330 and fix this optimally rather than implementing a workaround.