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>

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