Skip to content

Commit

Permalink
Ensure we do not double-lock the same volume in create
Browse files Browse the repository at this point in the history
When making containers, we want to lock all named volumes we are
adding the container to, to ensure they aren't removed from under
us while we are working. Unfortunately, this code did not account
for a container having the same volume mounted in multiple places
so it could deadlock. Add a map to ensure that we don't lock the
same name more than once to resolve this.

Fixes containers#8221

Signed-off-by: Matthew Heon <[email protected]>
  • Loading branch information
mheon committed Nov 11, 2020
1 parent dc58d4e commit 0f637e0
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 0 deletions.
1 change: 1 addition & 0 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,7 @@ func (c *Container) mountStorage() (_ string, deferredErr error) {
// config.
// Returns the volume that was mounted.
func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) (*Volume, error) {
logrus.Debugf("Going to mount named volume %s", v.Name)
vol, err := c.runtime.state.Volume(v.Name)
if err != nil {
return nil, errors.Wrapf(err, "error retrieving named volume %s for container %s", v.Name, c.ID())
Expand Down
7 changes: 7 additions & 0 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,15 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai

// Lock all named volumes we are adding ourself to, to ensure we can't
// use a volume being removed.
volsLocked := make(map[string]bool)
for _, namedVol := range ctrNamedVolumes {
toLock := namedVol
// Ensure that we don't double-lock a named volume that is used
// more than once.
if volsLocked[namedVol.Name()] {
continue
}
volsLocked[namedVol.Name()] = true
toLock.lock.Lock()
defer toLock.lock.Unlock()
}
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/run_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,4 +540,12 @@ VOLUME /test/`
session = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:%s:O", mountPath, mountDest), "--mount", fmt.Sprintf("type=tmpfs,target=%s", mountDest), ALPINE})
Expect(session.ExitCode()).To(Not(Equal(0)))
})

It("same volume in multiple places does not deadlock", func() {
volName := "testVol1"
session := podmanTest.Podman([]string{"run", "-t", "-i", "-v", fmt.Sprintf("%s:/test1", volName), "-v", fmt.Sprintf("%s:/test2", volName), "--rm", ALPINE, "sh", "-c", "mount | grep /test"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(len(session.OutputToStringArray())).To(Equal(2))
})
})

0 comments on commit 0f637e0

Please sign in to comment.