Skip to content

Commit a45e91e

Browse files
committed
update with a more minimal fix and a better understanding of the
problem. For the record: Our deadlock occurred when the __all_claimed check passed, but when another thread released the last element in the pool before we took back up the lock. Doing the check inside of the lock (thus preventing mutation of the claimed state for elements) closes the loophole for the race. Making the pool lock re-entrant and setting the condition variable might not be required, but it affords us a little bit of extra safety.
1 parent 3db29c8 commit a45e91e

File tree

1 file changed

+8
-15
lines changed

1 file changed

+8
-15
lines changed

riak/transports/pool.py

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ def __init__(self, obj):
4646
self.object = obj
4747
"""Whether the resource is currently in use."""
4848
self.claimed = False
49-
"""Whether the object has been deleted."""
50-
self.tomb = False
5149

5250

5351
class Pool(object):
@@ -84,8 +82,8 @@ def __init__(self):
8482
Creates a new Pool. This should be called manually if you
8583
override the __init__ method in a subclass.
8684
"""
87-
self.lock = threading.Lock()
88-
self.releaser = threading.Condition()
85+
self.lock = threading.RLock()
86+
self.releaser = threading.Condition(self.lock)
8987
self.elements = list()
9088

9189
@contextmanager
@@ -142,8 +140,8 @@ def delete_element(self, element):
142140
"""
143141
with self.lock:
144142
self.elements.remove(element)
145-
element.tomb = True
146143
self.destroy_resource(element.object)
144+
del element
147145

148146
def __iter__(self):
149147
"""
@@ -205,20 +203,15 @@ def __iter__(self):
205203
def next(self):
206204
if len(self.targets) == 0:
207205
raise StopIteration
208-
while len(self.unlocked) == 0:
206+
if len(self.unlocked) == 0:
209207
self.__claim_elements()
210-
ele = None
211-
while not ele:
212-
ele = self.unlocked.pop(0)
213-
if ele.tomb:
214-
ele = None
215-
return ele
208+
return self.unlocked.pop(0)
216209

217210
def __claim_elements(self):
218211
with self.lock:
219-
if self.__all_claimed():
220-
with self.releaser:
221-
self.releaser.wait(.05)
212+
with self.releaser:
213+
if self.__all_claimed():
214+
self.releaser.wait()
222215
for element in self.targets:
223216
if not element.claimed:
224217
self.targets.remove(element)

0 commit comments

Comments
 (0)