diff options
author | Simon Westphahl <westphahl@gmail.com> | 2022-02-07 14:49:20 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-07 08:49:20 -0500 |
commit | 9bb849941deb1d62ca3ca4d74882ed41cf569513 (patch) | |
tree | 37bdae5e178c68b98df84c52d0fd3ca1192e055e | |
parent | 4042a8505cdac94dba5718b6a89f82d478fef0d6 (diff) | |
download | kazoo-9bb849941deb1d62ca3ca4d74882ed41cf569513.tar.gz |
fix(recipe): fix deadlock in r/w lock recipe (#650)
The lock must only consider contenders with a sequence number lower than
it's own sequence number as also stated in the Zookeeper recipe
description for shared locks[0].
This wasn't working correctly as the ReadLock also considered WriteLocks
with a higher sequence number as contenders. This can lead to a deadlock
as described in #649.
[0]: https://zookeeper.apache.org/doc/r3.7.0/recipes.html#Shared+Locks
Closes #649
-rw-r--r-- | kazoo/recipe/lock.py | 22 | ||||
-rw-r--r-- | kazoo/tests/test_lock.py | 58 |
2 files changed, 71 insertions, 9 deletions
diff --git a/kazoo/recipe/lock.py b/kazoo/recipe/lock.py index 8bcc236..88b98e7 100644 --- a/kazoo/recipe/lock.py +++ b/kazoo/recipe/lock.py @@ -294,6 +294,7 @@ class Lock(object): (e.g. rlock), this and also edge cases where the lock's ephemeral node is gone. """ + node_sequence = node[len(self.prefix):] children = self.client.get_children(self.path) found_self = False # Filter out the contenders using the computed regex @@ -301,7 +302,12 @@ class Lock(object): for child in children: match = self._contenders_re.search(child) if match is not None: - contender_matches.append(match) + contender_sequence = match.group(1) + # Only consider contenders with a smaller sequence number. + # A contender with a smaller sequence number has a higher + # priority. + if contender_sequence < node_sequence: + contender_matches.append(match) if child == node: # Remember the node's match object so we can short circuit # below. @@ -313,15 +319,13 @@ class Lock(object): # node was removed. raise ForceRetryError() - predecessor = None - # Sort the contenders using the sequence number extracted by the regex, - # then extract the original string. - for match in sorted(contender_matches, key=lambda m: m.groups()): - if match is found_self: - break - predecessor = match.string + if not contender_matches: + return None - return predecessor + # Sort the contenders using the sequence number extracted by the regex + # and return the original string of the predecessor. + sorted_matches = sorted(contender_matches, key=lambda m: m.groups()) + return sorted_matches[-1].string def _find_node(self): children = self.client.get_children(self.path) diff --git a/kazoo/tests/test_lock.py b/kazoo/tests/test_lock.py index edfdb90..ebb7b23 100644 --- a/kazoo/tests/test_lock.py +++ b/kazoo/tests/test_lock.py @@ -467,6 +467,64 @@ class KazooLockTests(KazooTestCase): gotten = lock2.acquire(blocking=False) assert gotten is False + def test_rw_lock(self): + reader_event = self.make_event() + reader_lock = self.client.ReadLock(self. lockpath, "reader") + reader_thread = self.make_thread( + target=self._thread_lock_acquire_til_event, + args=("reader", reader_lock, reader_event) + ) + + writer_event = self.make_event() + writer_lock = self.client.WriteLock(self. lockpath, "writer") + writer_thread = self.make_thread( + target=self._thread_lock_acquire_til_event, + args=("writer", writer_lock, writer_event) + ) + + # acquire a write lock ourselves first to make the others line up + lock = self.client.WriteLock(self.lockpath, "test") + lock.acquire() + + reader_thread.start() + writer_thread.start() + + # wait for everyone to line up on the lock + wait = self.make_wait() + wait(lambda: len(lock.contenders()) == 3) + contenders = lock.contenders() + + assert contenders[0] == "test" + remaining = contenders[1:] + + # release the lock and contenders should claim it in order + lock.release() + + contender_bits = { + "reader": (reader_thread, reader_event), + "writer": (writer_thread, writer_event), + } + + for contender in ("reader", "writer"): + thread, event = contender_bits[contender] + + with self.condition: + while not self.active_thread: + self.condition.wait() + assert self.active_thread == contender + + assert lock.contenders() == remaining + remaining = remaining[1:] + + event.set() + + with self.condition: + while self.active_thread: + self.condition.wait() + + reader_thread.join() + writer_thread.join() + class TestSemaphore(KazooTestCase): def __init__(self, *args, **kw): |