Fixed release of expired holders

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:

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()