diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 828b8b76..21099216 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/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) diff --git a/tests/test_lock_decorator.py b/tests/test_lock_decorator.py index 42528567..a34a0a10 100644 --- a/tests/test_lock_decorator.py +++ b/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