adds option not to raise when leaving context manager after lock expiration

Author: julianolm

redis/asyncio/client.py

@@ -478,6 +478,7 @@ def lock(
         blocking_timeout: Optional[float] = None,
         lock_class: Optional[Type[Lock]] = None,
         thread_local: bool = True,
+        raise_on_release_error: bool = True,
     ) -> Lock:
         """
         Return a new Lock object using key ``name`` that mimics
@@ -524,6 +525,11 @@ def lock(
                      thread-1 would see the token value as "xyz" and would be
                      able to successfully release the thread-2's lock.
 
+        ``raise_on_release_error`` indicates whether to raise an exception when
+        the lock is no longer owned when exiting the context manager. By default,
+        this is True, meaning an exception will be raised. If False, the warning
+        will be logged and the exception will be suppressed.
+
         In some use cases it's necessary to disable thread local storage. For
         example, if you have code where one thread acquires a lock and passes
         that lock instance to a worker thread to release later. If thread
@@ -541,6 +547,7 @@ def lock(
             blocking=blocking,
             blocking_timeout=blocking_timeout,
             thread_local=thread_local,
+            raise_on_release_error=raise_on_release_error,
         )
 
     def pubsub(self, **kwargs) -> "PubSub":

redis/asyncio/lock.py

@@ -1,6 +1,7 @@
 import asyncio
 import threading
 import uuid
+import logging
 from types import SimpleNamespace
 from typing import TYPE_CHECKING, Awaitable, Optional, Union
 
@@ -10,6 +11,8 @@
 if TYPE_CHECKING:
     from redis.asyncio import Redis, RedisCluster
 
+logger = logging.getLogger(__name__)
+
 
 class Lock:
     """
@@ -85,6 +88,7 @@ def __init__(
         blocking: bool = True,
         blocking_timeout: Optional[Number] = None,
         thread_local: bool = True,
+        raise_on_release_error: bool = True,
     ):
         """
         Create a new Lock instance named ``name`` using the Redis client
@@ -127,6 +131,11 @@ def __init__(
                      token is *not* stored in thread local storage, then
                      thread-1 would see the token value as "xyz" and would be
                      able to successfully release the thread-2's lock.
+        
+        ``raise_on_release_error`` indicates whether to raise an exception when
+        the lock is no longer owned when exiting the context manager. By default,
+        this is True, meaning an exception will be raised. If False, the warning
+        will be logged and the exception will be suppressed.
 
         In some use cases it's necessary to disable thread local storage. For
         example, if you have code where one thread acquires a lock and passes
@@ -144,6 +153,7 @@ def __init__(
         self.blocking_timeout = blocking_timeout
         self.thread_local = bool(thread_local)
         self.local = threading.local() if self.thread_local else SimpleNamespace()
+        self.raise_on_release_error = raise_on_release_error
         self.local.token = None
         self.register_scripts()
 
@@ -163,7 +173,13 @@ async def __aenter__(self):
         raise LockError("Unable to acquire lock within the time specified")
 
     async def __aexit__(self, exc_type, exc_value, traceback):
-        await self.release()
+        try:
+            await self.release()
+        except LockNotOwnedError as e:
+            if self.raise_on_release_error:
+                raise e
+            logger.warning("Lock was no longer owned when exiting context manager.")
+
 
     async def acquire(
         self,

redis/client.py

@@ -473,6 +473,7 @@ def lock(
         blocking_timeout: Optional[float] = None,
         lock_class: Union[None, Any] = None,
         thread_local: bool = True,
+        raise_on_release_error: bool = True,
     ):
         """
         Return a new Lock object using key ``name`` that mimics
@@ -519,6 +520,11 @@ def lock(
                      thread-1 would see the token value as "xyz" and would be
                      able to successfully release the thread-2's lock.
 
+        ``raise_on_release_error`` indicates whether to raise an exception when
+        the lock is no longer owned when exiting the context manager. By default,
+        this is True, meaning an exception will be raised. If False, the warning
+        will be logged and the exception will be suppressed.
+
         In some use cases it's necessary to disable thread local storage. For
         example, if you have code where one thread acquires a lock and passes
         that lock instance to a worker thread to release later. If thread
@@ -536,6 +542,7 @@ def lock(
             blocking=blocking,
             blocking_timeout=blocking_timeout,
             thread_local=thread_local,
+            raise_on_release_error=raise_on_release_error,
         )
 
     def pubsub(self, **kwargs):

redis/exceptions.py

@@ -89,7 +89,7 @@ def __init__(self, message=None, lock_name=None):
 
 
 class LockNotOwnedError(LockError):
-    "Error trying to extend or release a lock that is (no longer) owned"
+    "Error trying to extend or release a lock that is not owned (anymore)"
 
     pass
 

redis/lock.py

@@ -1,12 +1,15 @@
 import threading
 import time as mod_time
 import uuid
+import logging
 from types import SimpleNamespace, TracebackType
 from typing import Optional, Type
 
 from redis.exceptions import LockError, LockNotOwnedError
 from redis.typing import Number
 
+logger = logging.getLogger(__name__)
+
 
 class Lock:
     """
@@ -82,6 +85,7 @@ def __init__(
         blocking: bool = True,
         blocking_timeout: Optional[Number] = None,
         thread_local: bool = True,
+        raise_on_release_error: bool = True,
     ):
         """
         Create a new Lock instance named ``name`` using the Redis client
@@ -125,6 +129,11 @@ def __init__(
                      thread-1 would see the token value as "xyz" and would be
                      able to successfully release the thread-2's lock.
 
+        ``raise_on_release_error`` indicates whether to raise an exception when
+        the lock is no longer owned when exiting the context manager. By default,
+        this is True, meaning an exception will be raised. If False, the warning
+        will be logged and the exception will be suppressed.
+
         In some use cases it's necessary to disable thread local storage. For
         example, if you have code where one thread acquires a lock and passes
         that lock instance to a worker thread to release later. If thread
@@ -140,6 +149,7 @@ def __init__(
         self.blocking = blocking
         self.blocking_timeout = blocking_timeout
         self.thread_local = bool(thread_local)
+        self.raise_on_release_error = raise_on_release_error
         self.local = threading.local() if self.thread_local else SimpleNamespace()
         self.local.token = None
         self.register_scripts()
@@ -168,7 +178,12 @@ def __exit__(
         exc_value: Optional[BaseException],
         traceback: Optional[TracebackType],
     ) -> None:
-        self.release()
+        try:
+            self.release()
+        except LockNotOwnedError as e:
+            if self.raise_on_release_error:
+                raise e
+            logger.warning("Lock was no longer owned when exiting context manager.")
 
     def acquire(
         self,

tests/test_asyncio/test_lock.py

@@ -129,6 +129,21 @@ async def test_context_manager_raises_when_locked_not_acquired(self, r):
             async with self.get_lock(r, "foo", blocking_timeout=0.1):
                 pass
 
+    async def test_context_manager_not_raise_on_release_error(self, r):
+        try:
+            async with self.get_lock(
+                r, "foo", timeout=0.1, raise_on_release_error=False
+            ):
+                await asyncio.sleep(0.15)
+        except LockNotOwnedError:
+            pytest.fail("LockNotOwnedError should not have been raised")
+
+        with pytest.raises(LockNotOwnedError):
+            async with self.get_lock(
+                r, "foo", timeout=0.1, raise_on_release_error=True
+            ):
+                await asyncio.sleep(0.15)
+
     async def test_high_sleep_small_blocking_timeout(self, r):
         lock1 = self.get_lock(r, "foo")
         assert await lock1.acquire(blocking=False)

tests/test_lock.py

@@ -133,6 +133,21 @@ def test_context_manager_raises_when_locked_not_acquired(self, r):
             with self.get_lock(r, "foo", blocking_timeout=0.1):
                 pass
 
+    def test_context_manager_not_raise_on_release_error(self, r):
+        try:
+            with self.get_lock(
+                r, "foo", timeout=0.1, raise_on_release_error=False
+            ):
+                time.sleep(0.15)
+        except LockNotOwnedError:
+            pytest.fail("LockNotOwnedError should not have been raised")
+
+        with pytest.raises(LockNotOwnedError):
+            with self.get_lock(
+                r, "foo", timeout=0.1, raise_on_release_error=True
+            ):
+                time.sleep(0.15)
+
     def test_high_sleep_small_blocking_timeout(self, r):
         lock1 = self.get_lock(r, "foo")
         assert lock1.acquire(blocking=False)