fix: remove process signals handling

[g11tech]

Remove the hardcoded signal processing as lodestar the main user of this library terminates thread explicitly

See

• Beacon node exits uncontrolled (too early) during graceful shutdown lodestar#5317

[nflaig]

@g11tech what are the changes required in Lodestar to make this work? I looked at it yesterday but didn't get it to properly close all workers, might be that I missed something

[g11tech]

i think process.exit will still need to be called after full node.close as i have no idea why the main thread would still remain hanging even though we terminate all workers in lodestar.

even this package still calls process.exit after terminating, so there is some reason exit needs to be called.

[wemeetagain]

We should debug why we still need an explicit process.exit (even if we add a process.exit in the short term to resolve the issue).
Its likely that stray event listeners are still attached.

[nflaig]

It does look like Lodestar actually depends on this package to exit the process.
As far as I understand adding a signal listener suppresses the default signal handling behavior which is what we are doing here https://github.com/ChainSafe/lodestar/blob/a2f749a6f08d5b8f6d1c52147b08f71d48f7abe2/packages/cli/src/util/process.ts#L15.

Don't we have to call process.exit(0) after cleanUpFunction is done?

[g11tech]

if there are no pending things to execute, the program should shut down (i think)

[nflaig]

I am not sure we can make sure there is no pending things to execute, a simple missed interval or timeout will keep the process alive

process.on("SIGINT", () => {
  console.log("Caught SIGINT!");
});

setInterval(() + {
  console.log("Still alive!");
}, 1000);

This process will not exit if you do Ctrl + C

Still alive!
Still alive!
^CCaught SIGINT!
Still alive!
Still alive!
Still alive!
^CCaught SIGINT!
^CCaught SIGINT!
Still alive!
Still alive!

I checked a bunch of libraries and they seem to explicity call process.exit

something like this from https://github.com/open-telemetry/opentelemetry-js#set-up-tracing

process.on('SIGTERM', () => {
  sdk.shutdown()
    .then(() => console.log('Tracing terminated'))
    .catch((error) => console.log('Error terminating tracing', error))
    .finally(() => process.exit(0));
});

[wemeetagain]

a simple missed interval or timeout will keep the process alive

Ideally a proper cleanup would close every timer. Not easy but 🤷

We can add a process.exit call in our cli since you tested and verified we need it.

[nflaig]

Ideally a proper cleanup would close every timer. Not easy but 🤷

Agreed...started attempt in ChainSafe/lodestar#5330

We can add a process.exit call in our cli since you tested and verified we need it.

Should clean this up as much as possible and find all active handlers which keep the process running. Once this is done I would still just call process.exit(0) or process.exit(1) on error, I think it is not such a bad thing to do and having a hanging process sounds annoying for users