diff --git a/CHANGES.rst b/CHANGES.rst index 3a94d716..a5d7aa18 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -23,6 +23,13 @@ Features Bug Handling ************ +- #291: Kazoo lock recipe was only partially re-entrant in that multiple + calls to `acquire` would obtain the the lock but the first call to `release` + would remove the underlying lock. This would leave the X - 1 other `acquire` + statements unprotected (and no longer holding there expected lock). To fix + this the comment about that lock recipe being re-entrant has been removed + and multiple acquires will now raise a ``RuntimeError`` when attempted. + - #78: Kazoo now uses socketpairs instead of pipes making it compatible with Windows. diff --git a/kazoo/recipe/lock.py b/kazoo/recipe/lock.py index 047a3066..3f278a4a 100644 --- a/kazoo/recipe/lock.py +++ b/kazoo/recipe/lock.py @@ -40,8 +40,8 @@ class Lock(object): with lock: # blocks waiting for lock acquisition # do something with the lock - Note: This lock is re-entrant. Repeat calls after acquired will - continue to return ''True''. + Note: This lock is not *re-entrant*. Repeated calls after already + acquired will raise a ``RuntimeError``. """ _NODE_NAME = '__lock__' @@ -106,6 +106,9 @@ def acquire(self, blocking=True, timeout=None): .. versionadded:: 1.1 The timeout option. """ + if self.is_acquired: + raise RuntimeError("Lock at path '%s' has already been" + " acquired" % self.path) try: retry = self._retry.copy() retry.deadline = timeout diff --git a/kazoo/tests/test_lock.py b/kazoo/tests/test_lock.py index da2b37c6..8a5f668d 100644 --- a/kazoo/tests/test_lock.py +++ b/kazoo/tests/test_lock.py @@ -240,12 +240,14 @@ def test_lock_cancel(self): thread1.join() client2.stop() - def test_lock_double_calls(self): + def test_lock_no_double_calls(self): lock1 = self.client.Lock(self.lockpath, "one") lock1.acquire() - lock1.acquire() - lock1.release() + self.assertTrue(lock1.is_acquired) + self.assertRaises(RuntimeError, lock1.acquire) + self.assertTrue(lock1.is_acquired) lock1.release() + self.assertFalse(lock1.is_acquired) def test_lock_reacquire(self): lock = self.client.Lock(self.lockpath, "one")