Browse code

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 <me@viktorstanchev.com>

Viktor Stanchev authored on 2016/03/23 05:24:09
Showing 2 changed files
... ...
@@ -127,9 +127,8 @@ func (s *VolumeStore) List() ([]volume.Volume, []string, error) {
127 127
 
128 128
 		s.locks.Lock(name)
129 129
 		storedV, exists := s.getNamed(name)
130
-		if !exists {
131
-			s.setNamed(v, "")
132
-		}
130
+		// Note: it's not safe to populate the cache here because the volume may have been
131
+		// deleted before we acquire a lock on its name
133 132
 		if exists && storedV.DriverName() != v.DriverName() {
134 133
 			logrus.Warnf("Volume name %s already exists for driver %s, not including volume returned by %s", v.Name(), storedV.DriverName(), v.DriverName())
135 134
 			s.locks.Unlock(v.Name())
... ...
@@ -26,19 +26,20 @@ func (NoopVolume) Unmount() error { return nil }
26 26
 
27 27
 // FakeVolume is a fake volume with a random name
28 28
 type FakeVolume struct {
29
-	name string
29
+	name       string
30
+	driverName string
30 31
 }
31 32
 
32 33
 // NewFakeVolume creates a new fake volume for testing
33
-func NewFakeVolume(name string) volume.Volume {
34
-	return FakeVolume{name: name}
34
+func NewFakeVolume(name string, driverName string) volume.Volume {
35
+	return FakeVolume{name: name, driverName: driverName}
35 36
 }
36 37
 
37 38
 // Name is the name of the volume
38 39
 func (f FakeVolume) Name() string { return f.name }
39 40
 
40 41
 // DriverName is the name of the driver
41
-func (FakeVolume) DriverName() string { return "fake" }
42
+func (f FakeVolume) DriverName() string { return f.driverName }
42 43
 
43 44
 // Path is the filesystem path to the volume
44 45
 func (FakeVolume) Path() string { return "fake" }
... ...
@@ -72,7 +73,7 @@ func (d *FakeDriver) Create(name string, opts map[string]string) (volume.Volume,
72 72
 	if opts != nil && opts["error"] != "" {
73 73
 		return nil, fmt.Errorf(opts["error"])
74 74
 	}
75
-	v := NewFakeVolume(name)
75
+	v := NewFakeVolume(name, d.name)
76 76
 	d.vols[name] = v
77 77
 	return v, nil
78 78
 }