c0defellas / enzo

core utilities
11 stars 2 forks source link

bug: kill isn't a serial killer #40

Open i4ki opened 7 years ago

i4ki commented 7 years ago

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 commented 7 years ago

/cc @geyslan

geyslan commented 7 years ago

@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 commented 7 years ago

Please test https://github.com/geyslan/enzo/commit/163c13730b12265f911316039eae1ffc49b7b980.