Skip to content

Commit bd2b3d3

Browse files
committedMay 23, 2016
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 moby#22732 Signed-off-by: Tonis Tiigi <[email protected]>
1 parent 3ab9049 commit bd2b3d3

File tree

2 files changed

+14
-19
lines changed

2 files changed

+14
-19
lines changed
 

‎container/history.go

-5
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@ func (history *History) Swap(i, j int) {
2424
containers[i], containers[j] = containers[j], containers[i]
2525
}
2626

27-
// Add the given container to history.
28-
func (history *History) Add(container *Container) {
29-
*history = append(*history, container)
30-
}
31-
3227
// sort orders the history by creation date in descendant order.
3328
func (history *History) sort() {
3429
sort.Sort(history)

‎container/memory_store.go

+14-14
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,9 @@ func (c *memoryStore) Delete(id string) {
4141
// List returns a sorted list of containers from the store.
4242
// The containers are ordered by creation date.
4343
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())
5045
containers.sort()
51-
return *containers
46+
return containers
5247
}
5348

5449
// Size returns the number of containers in the store.
@@ -60,9 +55,7 @@ func (c *memoryStore) Size() int {
6055

6156
// First returns the first container found in the store by a given filter.
6257
func (c *memoryStore) First(filter StoreFilter) *Container {
63-
c.RLock()
64-
defer c.RUnlock()
65-
for _, cont := range c.s {
58+
for _, cont := range c.all() {
6659
if filter(cont) {
6760
return cont
6861
}
@@ -74,11 +67,8 @@ func (c *memoryStore) First(filter StoreFilter) *Container {
7467
// This operation is asyncronous in the memory store.
7568
// NOTE: Modifications to the store MUST NOT be done by the StoreReducer.
7669
func (c *memoryStore) ApplyAll(apply StoreReducer) {
77-
c.RLock()
78-
defer c.RUnlock()
79-
8070
wg := new(sync.WaitGroup)
81-
for _, cont := range c.s {
71+
for _, cont := range c.all() {
8272
wg.Add(1)
8373
go func(container *Container) {
8474
apply(container)
@@ -89,4 +79,14 @@ func (c *memoryStore) ApplyAll(apply StoreReducer) {
8979
wg.Wait()
9080
}
9181

82+
func (c *memoryStore) all() []*Container {
83+
c.RLock()
84+
containers := make([]*Container, 0, len(c.s))
85+
for _, cont := range c.s {
86+
containers = append(containers, cont)
87+
}
88+
c.RUnlock()
89+
return containers
90+
}
91+
9292
var _ Store = &memoryStore{}

0 commit comments

Comments
 (0)