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 containers#9615

Signed-off-by: Matthew Heon <[email protected]>

libpod/container_internal.go

@@ -1304,9 +1304,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()
@@ -1315,13 +1313,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