RTS workaround converting SIGTERM into SIGINT #3641
Conversation
|
I should actually throw to the main thread rather than raise SIGINT. That's what the RTS does. I should also upstream this to GHC (see TopHandler). |
DavidEichmann
force-pushed
the
davide/sigterm_handler
branch
from
f25613b
to
fcba44c
Compare
|
I'm working on a GHC patch that'll make this the default behavior of the RTS https://gitlab.haskell.org/DavidEichmann/ghc/-/tree/davide/sigterm |
cardano-node/src/Cardano/Node/Run.hs
Outdated
| (Signals.CatchOnce $ do | ||
| runThreadIdMay <- deRefWeak runThreadIdWk | ||
| case runThreadIdMay of | ||
| Nothing -> return () |
There was a problem hiding this comment.
Why not a putStrLn "thread id no longer exists" or something to that effect?
cardano-node/src/Cardano/Node/Run.hs
Outdated
| runThreadIdMay <- deRefWeak runThreadIdWk | ||
| case runThreadIdMay of | ||
| Nothing -> return () | ||
| Just runThreadId -> killThread runThreadId |
There was a problem hiding this comment.
Should we also put some output here signalling that the thread was successfully killed?
There was a problem hiding this comment.
This code was lifted from GHC. We use a weak pointer so that the main thread can be garbage collected. If doRefWeak returns Nothing, then the main thread must have already exited. I can only imagine that might be possible if the main thread had shut down by some other means and a SIGTERM happened just after that. In that case I'd expect the process to exit gracefully any way, so simply doing nothing here (return ()) sounds correct. Is that convincing or do you still think we should output some message?
There was a problem hiding this comment.
.. in the case that the main thread is still live, you're asking if we should show a message still. There is already a message the appears: Shutting down.., but I dono for the life of me where it's coming from :-/ Do you think that message is enough, or if we need more?
There was a problem hiding this comment.
^^ Fair points. I think it's fine as is.
DavidEichmann
force-pushed
the
davide/sigterm_handler
branch
from
d83ac22
to
08cae7b
Compare
This is important for graceful shutdown, cleaning up resources in `bracket` statements. Fixes #1697
DavidEichmann
force-pushed
the
davide/sigterm_handler
branch
from
08cae7b
to
ce26b8e
Compare
|
bors merge |
|
Build succeeded: |
This is important for graceful shutdown, cleaning up resources in
bracketstatements.Fixes #1697