main: Support SIGTERM on Win and all unix variants. #381
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davecgh
force-pushed
the
signal
branch
2 times, most recently
from
6e35673
to
a3557ad
Compare
jholdstock
requested changes
Sep 18, 2023
jholdstock
reviewed
Sep 18, 2023
Go 1.14 added runtime support for handling the windows CTRL_CLOSE_EVENT, CTRL_LOGOFF_EVENT, and CTRL_SHUTDOWN_EVENT events by adding a definition for syscall.SIGTERM to windows and converting the events to that signal. It also added definitions for other common signals, including SIGHUP, and treats them as NOOPs on windows. Consequently, this modifies the logic that deals with conditionally handling SIGTERM and SIGHUP to handle them for windows as well as all unix-like operating systems, including newer ones that are supported by Go since the code was originally written. Although the additional signals could probably just be added unconditionally without too much issue now due to the runtime adding stubs to all operating systems the project officially supports, it is safer to be explicit when dealing with syscall definitions since there is no guarantee that they will exist for newly-added operating systems. Stated more plainly, this change means dcrpool will now be shutdown cleanly on more variants of unix as well as when being terminated by windows itself due to various things such as the user logging off, the window terminal being closed, and the system shutting down. It also pulls in the logic from dcrd so that logging is added when a signal is received so it is clear why the process is being shutdown.
jholdstock
approved these changes
Sep 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Go 1.14 added runtime support for handling the windows
CTRL_CLOSE_EVENT,CTRL_LOGOFF_EVENT, andCTRL_SHUTDOWN_EVENTevents by adding a definition forsyscall.SIGTERMto windows and converting the events to that signal. It also added definitions for other common signals, includingSIGHUP, and treats them as NOOPs on windows.Consequently, this modifies the logic that deals with conditionally handling
SIGTERMandSIGHUPto handle them for windows as well as all unix-like operating systems, including newer ones that are supported by Go since the code was originally written.Although the additional signals could probably just be added unconditionally without too much issue now due to the runtime adding stubs to all operating systems the project officially supports, it is safer to be explicit when dealing with syscall definitions since there is no guarantee that they will exist for newly-added operating systems.
Stated more plainly, this change means dcrpool will now be shutdown cleanly on more variants of unix as well as when being terminated by windows itself due to various things such as the user logging off, the window terminal being closed, and the system shutting down.
It also pulls in the logic from dcrd so that logging is added when a signal is received so it is clear why the process is being shutdown.