Skip to content

Commit a589f51

Browse files
Merge pull request #26971 from mheon/fix_26968
Fix a locking bug in that could cause a double-unlock
2 parents 6cc7467 + 2c6dadd commit a589f51

File tree

1 file changed

+15
-1
lines changed

1 file changed

+15
-1
lines changed

libpod/container_exec.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,19 @@ func (c *Container) healthCheckExec(config *ExecConfig, timeout time.Duration, s
859859
return -1, err
860860
}
861861
defer func() {
862+
// cleanupExecBundle MUST be called with the parent container locked.
863+
if !unlock && !c.batched {
864+
c.lock.Lock()
865+
unlock = true
866+
867+
if err := c.syncContainer(); err != nil {
868+
logrus.Errorf("Error syncing container %s state: %v", c.ID(), err)
869+
// Normally we'd want to continue here, get rid of the exec directory.
870+
// But the risk of proceeding into a function that can mutate state with a bad state is high.
871+
// Lesser of two evils is to bail and leak a directory.
872+
return
873+
}
874+
}
862875
if err := c.cleanupExecBundle(session.ID()); err != nil {
863876
logrus.Errorf("Container %s light exec session cleanup error: %v", c.ID(), err)
864877
}
@@ -971,7 +984,8 @@ func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resi
971984
return session.ExitCode, nil
972985
}
973986

974-
// cleanupExecBundle cleanups an exec session after its done
987+
// cleanupExecBundle cleans up an exec session after completion.
988+
// MUST BE CALLED with container `c` locked.
975989
// Please be careful when using this function since it might temporarily unlock
976990
// the container when os.RemoveAll($bundlePath) fails with ENOTEMPTY or EBUSY
977991
// errors.

0 commit comments

Comments
 (0)