From 87cccbc4b1f58ae004c4de6e87593e854a5d6c99 Mon Sep 17 00:00:00 2001 From: Angie Xiang Date: Mon, 9 Oct 2023 12:11:23 +0000 Subject: [PATCH] fix(recipe): prevent perpetual re-election if sequence node overflows Once sequence node is incremented beyond 2147483647, the sequence overflows into the negative space. When getting predecessors, cast the node sequence from String to int before comparing. Closes #730 --- kazoo/recipe/lock.py | 4 ++-- kazoo/tests/test_lock.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/kazoo/recipe/lock.py b/kazoo/recipe/lock.py index 59c6d8f3..b0f38f79 100644 --- a/kazoo/recipe/lock.py +++ b/kazoo/recipe/lock.py @@ -276,7 +276,7 @@ def _get_predecessor(self, node): (e.g. rlock), this and also edge cases where the lock's ephemeral node is gone. """ - node_sequence = node[len(self.prefix) :] + node_sequence = int(node[len(self.prefix) :]) children = self.client.get_children(self.path) found_self = False # Filter out the contenders using the computed regex @@ -284,7 +284,7 @@ def _get_predecessor(self, node): for child in children: match = self._contenders_re.search(child) if match is not None: - contender_sequence = match.group(1) + contender_sequence = int(match.group(1)) # Only consider contenders with a smaller sequence number. # A contender with a smaller sequence number has a higher # priority. diff --git a/kazoo/tests/test_lock.py b/kazoo/tests/test_lock.py index 397e971f..c73e19a9 100644 --- a/kazoo/tests/test_lock.py +++ b/kazoo/tests/test_lock.py @@ -799,6 +799,46 @@ def _thread(sem, event, timeout): class TestSequence(unittest.TestCase): def test_get_predecessor(self): + """Validate selection of predecessors.""" + pyLock_prefix = "514e5a831836450cb1a56c741e990fd8__lock__" + pyLock_predecessor = f"{pyLock_prefix}0000000030" + pyLock = f"{pyLock_prefix}0000000031" + pyLock_successor = f"{pyLock_prefix}0000000032" + children = [ + "hello", + pyLock_predecessor, + "world", + pyLock, + pyLock_successor, + ] + client = MagicMock() + client.get_children.return_value = children + lock = Lock(client, "test") + assert lock._get_predecessor(pyLock) == pyLock_predecessor + + def test_get_predecessor_with_overflowed_sequence(self): + """Validate selection of predecessors with negative sequence. + + This can occur in case of an integer overflow, if the sequence + counter is incremented beyond 2147483647. + """ + pyLock_prefix = "514e5a831836450cb1a56c741e990fd8__lock__" + pyLock_predecessor = f"{pyLock_prefix}-0000000032" + pyLock = f"{pyLock_prefix}-0000000031" + pyLock_successor = f"{pyLock_prefix}-0000000030" + children = [ + "hello", + pyLock_predecessor, + "world", + pyLock, + pyLock_successor, + ] + client = MagicMock() + client.get_children.return_value = children + lock = Lock(client, "test") + assert lock._get_predecessor(pyLock) == pyLock_predecessor + + def test_get_predecessor_no_predecessor(self): """Validate selection of predecessors.""" goLock = "_c_8eb60557ba51e0da67eefc47467d3f34-lock-0000000031" pyLock = "514e5a831836450cb1a56c741e990fd8__lock__0000000032"