New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Setup a signal context and use it. #207
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change overall. Just a performance related comment.
// Use of this source code is governed by an ISC | ||
// license that can be found in the LICENSE file. | ||
// | ||
//go:build windows || aix || android || darwin || dragonfly || freebsd || hurd || illumos || ios || linux || netbsd || openbsd || solaris |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think Windows should be on this list, it doesnt have SIGTERM afaik. Just os.Interrupt should suffice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIGTERM
is listed here: https://pkg.go.dev/syscall?GOOS=windows#pkg-constants
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's correct and necessary to handle shutdown events from Windows correctly (namely CTRL_CLOSE_EVENT
, CTRL_LOGOFF_EVENT
and CTRL_SHUTDOWN_EVENT
). Go added it a while back.
It's documented in the os/signal Windows section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, just tested on windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked. It was added in Go 1.14. See the Minor changes to the library -> os/signal in the release notes.
No description provided.