Ignore error when killing, if error is 'process does not exist' #1339
Conversation
| let res = signal::kill(pid, signal); | ||
|
|
||
| match res { | ||
| Err(nix::errno::Errno::ESRCH) => { | ||
| /* the process does not exist, which is what we want */ | ||
| } | ||
| _ => res?, | ||
| } |
There was a problem hiding this comment.
Is there a more idiomatic way of doing this?
| let res = signal::kill(pid, signal); | ||
| match res { | ||
| Err(nix::errno::Errno::ESRCH) => { | ||
| /* the process does not exist, which is what we want */ | ||
| Ok(()) | ||
| } | ||
| _ => res, | ||
| } |
There was a problem hiding this comment.
Now thinking, I'm not sure if we need this, as we are freezing the cgroup before getting all pids, so whatever pids we get, the process must exist, as freezing cgroup will suspend all processes in the cgroup
I'm not sure what you mean by this. If we want to add a new test to validate this, I'm not sure how would I go about doing this, because this is basically a race condition. The problem here was that between we getting list of processes and we sending the kill signal, a process from list exits on its own, making the kill call fail. As we want to kill the process anyways, and it has exited, here we just ignore that specific failure. In my opinion, it would be difficult to consistently recreate this race condition in a test. Let me know if I'm getting this wrong |
You are right! |
Currently when killing a container, if the container process does not exist (already deleted) or when killing all processes in the container, a process exits between we getting its pid and sending the kill signal, kill command will produce error.
However, in this specific case, it is ok that killing failed, as the process to kill does not exist. So we ignore the error.
Note, I'd like the check of error code to be more idiomatic, but not sure how