Skip to content

Commit

Permalink
Do not error on signalling a just-stopped container
Browse files Browse the repository at this point in the history
Previous PR containers#12394 tried to address this, but made a mistake:
containers that have just exited do not move to the Exited state
but rather the Stopped state - as such, the code would never have
run (there is no way we start `podman kill`, and the container
transitions to Exited while we are doing it - that requires
holding the container lock, which Kill already does).

Fix the code to check Stopped as well (we omit Exited entirely
but it's a cheap check and our state logic could change in the
future). Also, return an error, instead of exiting cleanly - the
Kill failed, after all. ErrCtrStateInvalid is already handled by
the sig-proxy logic so there won't be issues.

[NO NEW TESTS NEEDED] This fixes a race that I cannot reproduce
myself, and I have no idea how we'd repro in CI.

Signed-off-by: Matthew Heon <[email protected]>
  • Loading branch information
mheon committed Jun 9, 2022
1 parent 643a692 commit c77691f
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions libpod/oci_conmon_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,8 @@ func (r *ConmonOCIRuntime) KillContainer(ctr *Container, signal uint, all bool)
if err2 := r.UpdateContainerStatus(ctr); err2 != nil {
logrus.Infof("Error updating status for container %s: %v", ctr.ID(), err2)
}
if ctr.state.State == define.ContainerStateExited {
return nil
if ctr.ensureState(define.ContainerStateStopped, define.ContainerStateExited) {
return define.ErrCtrStateInvalid
}
return errors.Wrapf(err, "error sending signal to container %s", ctr.ID())
}
Expand Down

0 comments on commit c77691f

Please sign in to comment.