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

bug: kill isn't a serial killer #40

Open
i4ki opened this issue Apr 26, 2017 · 3 comments
Open

bug: kill isn't a serial killer #40

i4ki opened this issue Apr 26, 2017 · 3 comments

Comments

@i4ki
Copy link
Collaborator

i4ki commented Apr 26, 2017

When we discussed the kill command we talked about that it should really kill (trying first SIGTERM and if it doesn't die then calling SIGKILL).
The problem is that it's not calling SIGKILL if process doesn't dies:

λ> strace kill 17140
execve("/home/i4k/.gvm/pkgsets/go1.8/global/bin/kill", ["kill", "17140"], [/* 32 vars */]) = 0
arch_prctl(ARCH_SET_FS, 0x511428)       = 0
sched_getaffinity(0, 8192, [0 1 2 3])   = 16
mmap(0xc000000000, 65536, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc000000000
...
...
kill(17140, SIGTERM)                    = 0
exit_group(0)                           = ?

The problem is that kill(2) returns 0 (ok) when it sends the signal successfully to the process, but that doesn't guarantees that it will die with SIGTERM...
See code here: https://github.com/c0defellas/enzo/blob/master/cmd/kill/kill_unix.go#L13

The right approach would be verify if process is still alive in the next few milliseconds and if so then send a SIGKILL.

@i4ki
Copy link
Collaborator Author

i4ki commented Apr 26, 2017

/cc @geyslan

@geyslan
Copy link
Member

geyslan commented May 3, 2017

@tiago4orion I think we can sleep a few milliseconds before testing if pid is alive with err = syscall.Kill(pid, 0) finally checking if err isn't ESRCH to kill it once and for all.

@geyslan
Copy link
Member

geyslan commented May 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants