Add check that _lock is a lock in requires_lock

src/openflexure_microscope_server/utilities.py

@@ -23,6 +23,16 @@ COMMIT_REGEX = re.compile(r"[0-9a-f]{40}")
 REF_REGEX = re.compile(r"^ref:\s(.*)$")
 
 
+def _is_lock_like(obj):
+    """Check if an object is a lock.
+
+    Cannot use ``isinstance(obj, threading.RLock)``, may be possible to use
+    ``isinstance(obj, threading._RLock)``. But this uses a private method and may
+    break. Instead making the check that both "acquire" and "release" exist.
+    """
+    return hasattr(obj, "acquire") and hasattr(obj, "release")
+
+
 def requires_lock(method: Callable[P, T]) -> Callable[P, T]:
     """Decorate a class method so that it requires the class lock to run.
 
@@ -31,8 +41,12 @@ def requires_lock(method: Callable[P, T]) -> Callable[P, T]:
 
     @wraps(method)
     def wrapper(self, *args: P.args, **kwargs: P.kwargs) -> T:
+        # Confirm the object the method is attached to has a self._lock property
         if not hasattr(self, "_lock"):
             raise AttributeError(f"{self.__class__.__name__} has no '_lock' attribute")
+        # And that it is a reentrant lock.
+        if not _is_lock_like(self._lock):
+            raise TypeError("requires_lock requires self._lock to be a lock")
         with self._lock:
             return method(self, *args, **kwargs)
 

tests/test_lock_decorator.py

@@ -50,7 +50,7 @@ def lockable_obj() -> LockableClass:
 
 
 def test_error_if_no_lock(lockable_obj):
-    """Test there is an Attribute error if the object has no lock attribute."""
+    """Test there is an AttributeError if the object has no _lock attribute."""
     # Delete the lock!
     del lockable_obj._lock
     with pytest.raises(AttributeError) as exec_info:
@@ -58,6 +58,16 @@ def test_error_if_no_lock(lockable_obj):
     assert str(exec_info.value) == "LockableClass has no '_lock' attribute"
 
 
+def test_error_if_lock_of_wrong_type(lockable_obj):
+    """Test there is an TypeError if the object's _lock attribute isn't a lock."""
+    # Delete the lock!
+    lockable_obj._lock = "I'm a mock, not a lock!"
+    with pytest.raises(TypeError) as exec_info:
+        lockable_obj.test_property
+    expected_msg = "requires_lock requires self._lock to be a lock"
+    assert str(exec_info.value) == expected_msg
+
+
 def test_functionality(lockable_obj):
     """Check that this doesn't affect the most basic functionality of the class."""
     assert lockable_obj.test_property == 1