Skip to content
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

Change SIGINT to SIGTERM #29

Merged
merged 1 commit into from
Sep 10, 2015
Merged

Change SIGINT to SIGTERM #29

merged 1 commit into from
Sep 10, 2015

Conversation

cbednarski
Copy link
Contributor

The docs casually mention that os.Interrupt doesn't do anything on Windows, so I'm curious whether proc.Signal does anything at all on Windows or is silently ignored.

unix.SIGTERM should work on any POSIX OS.

Closes #27

@@ -113,7 +115,7 @@ func (h *execHandle) Update(task *structs.Task) error {
// Kill is used to terminate the task. We send an Interrupt
// and then provide a 5 second grace period before doing a Kill.
func (h *execHandle) Kill() error {
h.proc.Signal(os.Interrupt)
h.proc.Signal(unix.SIGTERM)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not just use the constants in syscall here? syscall.SIGTERM seems like it could help us avoid the extra import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syscall docs say this:

NOTE: This package is locked down. Code outside the standard Go repository should be migrated to use the corresponding package in the golang.org/x/sys repository.

@mitchellh
Copy link
Contributor

I'd probably use a build tag file signal_windows.go signal_linux.go etc. to set a variable that we use to set that signal. I don't see a need to depend on that other package for this. And on Windows its probably something else, and I imagine some point in 2016 we'll get this thing to work on Windows too (Windows Server Containers with Docker)

@cbednarski
Copy link
Contributor Author

After digging further, the go signal implementation for windows doesn't handle signals at all. They all result in an error state. We don't check errors from the signal call so all this means for windows is we wait 5 seconds and then kill the process.

TL;DR this change makes POSIX systems use SIGTERM, and Windows is completely unaffected.

cbednarski added a commit that referenced this pull request Sep 10, 2015
@cbednarski cbednarski merged commit 9b763ed into master Sep 10, 2015
@cbednarski cbednarski deleted the b-sigterm branch September 10, 2015 00:08
benbuzbee pushed a commit to benbuzbee/nomad that referenced this pull request Jul 21, 2022
benbuzbee pushed a commit to benbuzbee/nomad that referenced this pull request Jul 21, 2022
Update StableStore documentation based on hashicorp#29
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants