From f04334ea040500181727c47dc626171e98660cae Mon Sep 17 00:00:00 2001 From: Viktor Stanchev Date: Tue, 22 Mar 2016 13:24:09 -0700 Subject: [PATCH] fix race condition between list and remove volume This was done by making List not populate the cache. fixes #21403 Signed-off-by: Viktor Stanchev (cherry picked from commit 800b9c5a2698aae5c43f42d4c9c1a41280b556a6) --- volume/store/store.go | 5 ++--- volume/testutils/testutils.go | 11 ++++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/volume/store/store.go b/volume/store/store.go index 1686b93b15..154258d8f9 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -127,9 +127,8 @@ func (s *VolumeStore) List() ([]volume.Volume, []string, error) { s.locks.Lock(name) storedV, exists := s.getNamed(name) - if !exists { - s.setNamed(v, "") - } + // Note: it's not safe to populate the cache here because the volume may have been + // deleted before we acquire a lock on its name if exists && storedV.DriverName() != v.DriverName() { logrus.Warnf("Volume name %s already exists for driver %s, not including volume returned by %s", v.Name(), storedV.DriverName(), v.DriverName()) s.locks.Unlock(v.Name()) diff --git a/volume/testutils/testutils.go b/volume/testutils/testutils.go index 46ada378c7..653533696a 100644 --- a/volume/testutils/testutils.go +++ b/volume/testutils/testutils.go @@ -26,19 +26,20 @@ func (NoopVolume) Unmount() error { return nil } // FakeVolume is a fake volume with a random name type FakeVolume struct { - name string + name string + driverName string } // NewFakeVolume creates a new fake volume for testing -func NewFakeVolume(name string) volume.Volume { - return FakeVolume{name: name} +func NewFakeVolume(name string, driverName string) volume.Volume { + return FakeVolume{name: name, driverName: driverName} } // Name is the name of the volume func (f FakeVolume) Name() string { return f.name } // DriverName is the name of the driver -func (FakeVolume) DriverName() string { return "fake" } +func (f FakeVolume) DriverName() string { return f.driverName } // Path is the filesystem path to the volume func (FakeVolume) Path() string { return "fake" } @@ -72,7 +73,7 @@ func (d *FakeDriver) Create(name string, opts map[string]string) (volume.Volume, if opts != nil && opts["error"] != "" { return nil, fmt.Errorf(opts["error"]) } - v := NewFakeVolume(name) + v := NewFakeVolume(name, d.name) d.vols[name] = v return v, nil }