stacklok / toolhive / 19936875169

59%

push

github

Gracefully Shutdown Foreground Server on Interrupt (#2863)

The solution here avoids the competing signal handlers by:

Removing the os.Exit(0) from the default handler.
When the default handler receives a signal, cancel the context that is plumbed to the run command.
Remove the run signal handler and instead rely on context cancellation.
This solution is not without risk: Commands that don’t check for context cancellation and relied on the os.Exit(0) may now continue running until they hit code that does respect the context cancellation. Based on my understanding of the code, thv run --foreground is one of the few commands that can be long running, so I think this risk is relatively small.

Beyond the scope of this pull request, I think we should prefer to rely on context cancellation rather than using signal.Notify deep in the implementation of commands. This “competing signal handlers” problem likely exists elsewhere: I count 10 uses of signal.Notify.

---------

Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>

0 of 10 new or added lines in 2 files covered. (0.0%)

26222 of 52647 relevant lines covered (49.81%)

59.54 hits per line