From c77691f06f619bf15d0f83690a1dbbfe72f92b7a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 8 Jun 2022 09:27:19 -0400 Subject: [PATCH] Do not error on signalling a just-stopped container Previous PR #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 --- libpod/oci_conmon_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 6aa7ce6dce..0c1ee61d39 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -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()) }