Browse code

Release memoryStore locks before filter/apply

Rework memoryStore so that filters and apply run
on a cloned list of containers after the lock has
been released. This avoids possible deadlocks when
these filter/apply callbacks take locks for a
container.

Fixes #22732

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit bd2b3d363ff7c46e01cce4e6a41d41f24a0047da)

Tonis Tiigi authored on 2016/05/24 03:45:04
Showing 2 changed files
... ...
@@ -24,11 +24,6 @@ func (history *History) Swap(i, j int) {
24 24
 	containers[i], containers[j] = containers[j], containers[i]
25 25
 }
26 26
 
27
-// Add the given container to history.
28
-func (history *History) Add(container *Container) {
29
-	*history = append(*history, container)
30
-}
31
-
32 27
 // sort orders the history by creation date in descendant order.
33 28
 func (history *History) sort() {
34 29
 	sort.Sort(history)
... ...
@@ -41,14 +41,9 @@ func (c *memoryStore) Delete(id string) {
41 41
 // List returns a sorted list of containers from the store.
42 42
 // The containers are ordered by creation date.
43 43
 func (c *memoryStore) List() []*Container {
44
-	containers := new(History)
45
-	c.RLock()
46
-	for _, cont := range c.s {
47
-		containers.Add(cont)
48
-	}
49
-	c.RUnlock()
44
+	containers := History(c.all())
50 45
 	containers.sort()
51
-	return *containers
46
+	return containers
52 47
 }
53 48
 
54 49
 // Size returns the number of containers in the store.
... ...
@@ -60,9 +55,7 @@ func (c *memoryStore) Size() int {
60 60
 
61 61
 // First returns the first container found in the store by a given filter.
62 62
 func (c *memoryStore) First(filter StoreFilter) *Container {
63
-	c.RLock()
64
-	defer c.RUnlock()
65
-	for _, cont := range c.s {
63
+	for _, cont := range c.all() {
66 64
 		if filter(cont) {
67 65
 			return cont
68 66
 		}
... ...
@@ -74,11 +67,8 @@ func (c *memoryStore) First(filter StoreFilter) *Container {
74 74
 // This operation is asyncronous in the memory store.
75 75
 // NOTE: Modifications to the store MUST NOT be done by the StoreReducer.
76 76
 func (c *memoryStore) ApplyAll(apply StoreReducer) {
77
-	c.RLock()
78
-	defer c.RUnlock()
79
-
80 77
 	wg := new(sync.WaitGroup)
81
-	for _, cont := range c.s {
78
+	for _, cont := range c.all() {
82 79
 		wg.Add(1)
83 80
 		go func(container *Container) {
84 81
 			apply(container)
... ...
@@ -89,4 +79,14 @@ func (c *memoryStore) ApplyAll(apply StoreReducer) {
89 89
 	wg.Wait()
90 90
 }
91 91
 
92
+func (c *memoryStore) all() []*Container {
93
+	c.RLock()
94
+	containers := make([]*Container, 0, len(c.s))
95
+	for _, cont := range c.s {
96
+		containers = append(containers, cont)
97
+	}
98
+	c.RUnlock()
99
+	return containers
100
+}
101
+
92 102
 var _ Store = &memoryStore{}