Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds option not to raise exception when leaving context manager after lock expiration #3531

Merged
merged 11 commits into from
Mar 10, 2025
7 changes: 7 additions & 0 deletions redis/asyncio/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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":
Expand Down
7 changes: 7 additions & 0 deletions redis/asyncio/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,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
Expand Down Expand Up @@ -885,6 +886,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
Expand All @@ -902,6 +908,7 @@ def lock(
blocking=blocking,
blocking_timeout=blocking_timeout,
thread_local=thread_local,
raise_on_release_error=raise_on_release_error,
)


Expand Down
19 changes: 18 additions & 1 deletion redis/asyncio/lock.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import logging
import threading
import uuid
from types import SimpleNamespace
Expand All @@ -10,6 +11,8 @@
if TYPE_CHECKING:
from redis.asyncio import Redis, RedisCluster

logger = logging.getLogger(__name__)


class Lock:
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -128,6 +132,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
Expand All @@ -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()

Expand All @@ -163,7 +173,14 @@ 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 LockError:
if self.raise_on_release_error:
raise
logger.warning(
"Lock was unlocked or no longer owned when exiting context manager."
)

async def acquire(
self,
Expand Down
7 changes: 7 additions & 0 deletions redis/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down
7 changes: 7 additions & 0 deletions redis/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ def lock(
blocking_timeout=None,
lock_class=None,
thread_local=True,
raise_on_release_error: bool = True,
):
"""
Return a new Lock object using key ``name`` that mimics
Expand Down Expand Up @@ -868,6 +869,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
Expand All @@ -885,6 +891,7 @@ def lock(
blocking=blocking,
blocking_timeout=blocking_timeout,
thread_local=thread_local,
raise_on_release_error=raise_on_release_error,
)

def set_response_callback(self, command, callback):
Expand Down
2 changes: 1 addition & 1 deletion redis/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
19 changes: 18 additions & 1 deletion redis/lock.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import threading
import time as mod_time
import uuid
Expand All @@ -7,6 +8,8 @@
from redis.exceptions import LockError, LockNotOwnedError
from redis.typing import Number

logger = logging.getLogger(__name__)


class Lock:
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -168,7 +178,14 @@ def __exit__(
exc_value: Optional[BaseException],
traceback: Optional[TracebackType],
) -> None:
self.release()
try:
self.release()
except LockError:
if self.raise_on_release_error:
raise
logger.warning(
"Lock was unlocked or no longer owned when exiting context manager."
)

def acquire(
self,
Expand Down
30 changes: 30 additions & 0 deletions tests/test_asyncio/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,36 @@ 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_lock_not_owned_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_context_manager_not_raise_on_release_lock_error(self, r):
try:
async with self.get_lock(
r, "foo", timeout=0.1, raise_on_release_error=False
) as lock:
lock.release()
except LockError:
pytest.fail("LockError should not have been raised")

with pytest.raises(LockError):
async with self.get_lock(
r, "foo", timeout=0.1, raise_on_release_error=True
) as lock:
lock.release()

async def test_high_sleep_small_blocking_timeout(self, r):
lock1 = self.get_lock(r, "foo")
assert await lock1.acquire(blocking=False)
Expand Down
26 changes: 26 additions & 0 deletions tests/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,32 @@ 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_lock_not_owned_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_context_manager_not_raise_on_release_lock_error(self, r):
try:
with self.get_lock(
r, "foo", timeout=0.1, raise_on_release_error=False
) as lock:
lock.release()
except LockError:
pytest.fail("LockError should not have been raised")

with pytest.raises(LockError):
with self.get_lock(
r, "foo", timeout=0.1, raise_on_release_error=True
) as lock:
lock.release()

def test_high_sleep_small_blocking_timeout(self, r):
lock1 = self.get_lock(r, "foo")
assert lock1.acquire(blocking=False)
Expand Down
Loading