summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Heon <mheon@redhat.com>2021-03-04 16:22:41 -0500
committerMatthew Heon <mheon@redhat.com>2021-03-05 09:19:48 -0500
commit5bb8fa30b04e08761df6b412e7ef3c7cc0970650 (patch)
tree5e4d7ca9c67fabf1229e7d54b6e85b37e78af7f9
parent259bb5f7232432bce103c64cf1e68539ab7012e2 (diff)
downloadpodman-5bb8fa30b04e08761df6b412e7ef3c7cc0970650.tar.gz
podman-5bb8fa30b04e08761df6b412e7ef3c7cc0970650.tar.bz2
podman-5bb8fa30b04e08761df6b412e7ef3c7cc0970650.zip
Do not return from c.stop() before re-locking
Unlocking an already unlocked lock is a panic. As such, we have to make sure that the deferred c.lock.Unlock() in c.StopWithTimeout() always runs on a locked container. There was a case in c.stop() where we could return an error after we unlock the container to stop it, but before we re-lock it - thus allowing for a double-unlock to occur. Fix the error return to not happen until after the lock has been re-acquired. Fixes #9615 Signed-off-by: Matthew Heon <mheon@redhat.com>
-rw-r--r--libpod/container_internal.go16
1 files changed, 12 insertions, 4 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 7e8226de4..bace18825 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1307,9 +1307,7 @@ func (c *Container) stop(timeout uint) error {
c.lock.Unlock()
}
- if err := c.ociRuntime.StopContainer(c, timeout, all); err != nil {
- return err
- }
+ stopErr := c.ociRuntime.StopContainer(c, timeout, all)
if !c.batched {
c.lock.Lock()
@@ -1318,13 +1316,23 @@ func (c *Container) stop(timeout uint) error {
// If the container has already been removed (e.g., via
// the cleanup process), there's nothing left to do.
case define.ErrNoSuchCtr, define.ErrCtrRemoved:
- return nil
+ return stopErr
default:
+ if stopErr != nil {
+ logrus.Errorf("Error syncing container %s status: %v", c.ID(), err)
+ return stopErr
+ }
return err
}
}
}
+ // We have to check stopErr *after* we lock again - otherwise, we have a
+ // change of panicing on a double-unlock. Ref: GH Issue 9615
+ if stopErr != nil {
+ return stopErr
+ }
+
// Since we're now subject to a race condition with other processes who
// may have altered the state (and other data), let's check if the
// state has changed. If so, we should return immediately and log a