From e635130043d7657d9bee6911995f649e2de9eb04 Mon Sep 17 00:00:00 2001 From: Jack Edge Date: Thu, 24 Sep 2020 15:34:19 +0100 Subject: =?UTF-8?q?=F0=9F=95=B0=EF=B8=8F=20Use=20monotonic=20clock=20in=20?= =?UTF-8?q?Lock=20(and=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During a call to `acquire()`, if the call is `blocking` and has a `blocking_timeout` set, it uses `time.time()` calls to determine when to give up attempting to acquire the lock. However, since `time.time()` is marked as "adjustable", it is possible for it to go backwards or forwards at a rate other than 1 second per second, meaning the spinloop may exit earlier or later than expected. By changing the implementation to use `time.monotonic()`, which is guaranteed to never go backwards, and not be affected by system clock updates, this potential problem is fixed. For the same reason, some time dependent lock tests have also been changed to use `time.monotonic()`. --- redis/lock.py | 4 ++-- tests/test_lock.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/redis/lock.py b/redis/lock.py index e51f169..e36b2da 100644 --- a/redis/lock.py +++ b/redis/lock.py @@ -186,14 +186,14 @@ class Lock: blocking_timeout = self.blocking_timeout stop_trying_at = None if blocking_timeout is not None: - stop_trying_at = mod_time.time() + blocking_timeout + stop_trying_at = mod_time.monotonic() + blocking_timeout while True: if self.do_acquire(token): self.local.token = token return True if not blocking: return False - next_try_at = mod_time.time() + sleep + next_try_at = mod_time.monotonic() + sleep if stop_trying_at is not None and next_try_at > stop_trying_at: return False mod_time.sleep(sleep) diff --git a/tests/test_lock.py b/tests/test_lock.py index 8bb3c7e..fa76385 100644 --- a/tests/test_lock.py +++ b/tests/test_lock.py @@ -101,10 +101,10 @@ class TestLock: bt = 0.2 sleep = 0.05 lock2 = self.get_lock(r, 'foo', sleep=sleep, blocking_timeout=bt) - start = time.time() + start = time.monotonic() assert not lock2.acquire() # The elapsed duration should be less than the total blocking_timeout - assert bt > (time.time() - start) > bt - sleep + assert bt > (time.monotonic() - start) > bt - sleep lock1.release() def test_context_manager(self, r): @@ -126,11 +126,11 @@ class TestLock: sleep = 60 bt = 1 lock2 = self.get_lock(r, 'foo', sleep=sleep, blocking_timeout=bt) - start = time.time() + start = time.monotonic() assert not lock2.acquire() # the elapsed timed is less than the blocking_timeout as the lock is # unattainable given the sleep/blocking_timeout configuration - assert bt > (time.time() - start) + assert bt > (time.monotonic() - start) lock1.release() def test_releasing_unlocked_lock_raises_error(self, r): -- cgit v1.2.1