From ad070d438dc094bc18d9666d5f00375aa00d5c9d Mon Sep 17 00:00:00 2001 From: novirium Date: Mon, 30 Mar 2020 21:28:19 +0800 Subject: [PATCH] Fixed release of expired holders --- hold_lock.py | 40 ++++++++++++++++++++++++++++------------ test_hold_lock.py | 11 +++++++++++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/hold_lock.py b/hold_lock.py index e26c360..5a04e1a 100644 --- a/hold_lock.py +++ b/hold_lock.py @@ -1,4 +1,5 @@ import time +import itertools import contextlib import threading @@ -23,7 +24,11 @@ class HoldLock(contextlib.AbstractContextManager): main waiting thread calls `holders()` or iterates `waiting_for()` - as then it gets access to these identifiers. The common use case here is to use a string explaining the reason for the `hold()` as the identifier, which then allows the main thread to print a list of things it's - waiting for by iterating `waiting_for()`. + waiting for by iterating `waiting_for()`. By default, the `HoldLock.AnonHolder` identifier is + used in all calls, allowing the identifier to be completely ignored if it's not useful. + + The HoldLock object itself can be used as a context manager in `with` statements, and functions + the same as calling `hold()` with defaults. """ class AnonHolder(): @@ -68,6 +73,7 @@ class HoldLock(contextlib.AbstractContextManager): can be supplied as any function that returns a current absolute time in seconds as a float. """ self._holders = [] + self._expired_holders = [] self._cv = threading.Condition() self.time_func = time_func self._closed = False @@ -76,21 +82,25 @@ class HoldLock(contextlib.AbstractContextManager): """ Acquire a hold on this HoldLock, blocking any `wait()` call until all holds are released. Multiple threads may acquire a hold simultaneously, and an identifier may be used more than - once. + once. A hold must later be released with `release()`, providing the same identifier. The default `None` identifier works like any other, but will result in calls to `holders` or `waiting_for()` to return a tuple containing None values. - Can either be called directly or used as a context manager - `with holdlock.hold():` + Can either be called directly or used as a context manager - `with holdlock.hold():`. The + returned Holder object also provides a way to see if the hold has expired + (`holder.expired()`) and also provides an alternate way to release it without having to + pass the identifier again (`holder.release()`). - The returned object is a context manager, but a bool comparison with it will return False - if the timeout has expired: + holder1 = holdlock.hold("annoying to reference identifier") + holder1.release() - with holdlock.hold(timeout=5) as hold: + with holdlock.hold(timeout=5) as holder2: while True: time.sleep(1) - if not hold: - print("Timeout has expired) + if holder2.expired(): + print("Timeout has expired") + """ with self._cv: if self._closed: @@ -111,21 +121,27 @@ class HoldLock(contextlib.AbstractContextManager): """ Release a hold on this HoldLock. If there are mutiple holders with the supplied identifier, the one with the earliest timeout will be released. + + Returns False if the hold had expired (technically holds only expire _if_ someone was + waiting for it when the timeout was hit), otherwise returns True. """ with self._cv: # _holders is already sorted for us - for holder in self._holders: + for holder in itertools.chain(self._expired_holders, self._holders): if holder.identifier == identifier: matching_holder = holder break else: raise Exception(F"Release identifier '{identifier}' is not currently held") - self._release(matching_holder) + return self._release(matching_holder) def _release(self, holder): with self._cv: - self._holders.remove(holder) + if holder in self._expired_holders: + self._expired_holders.remove(holder) + else: + self._holders.remove(holder) self._cv.notify_all() def close(self): @@ -181,7 +197,7 @@ class HoldLock(contextlib.AbstractContextManager): # Pull out any holders that have expired while (self._holders[0].expiry is not None): if self._holders[0].expiry <= now: - self._holders.pop(0) + self._expired_holders.append(self._holders.pop(0)) if len(self._holders) == 0: return True else: diff --git a/test_hold_lock.py b/test_hold_lock.py index 19ec749..f4fb554 100644 --- a/test_hold_lock.py +++ b/test_hold_lock.py @@ -58,6 +58,17 @@ def test_hold_timeout(): assert lock.wait() +def test_release_expired_holder(): + lock = HoldLock() + lock.hold(timeout=0.1) + assert lock.wait() + lock.release() + assert lock.wait() + + with lock.hold(timeout=0.1): + assert lock.wait() + + def test_waiting_for(): lock = HoldLock()