Skip to content

Commit f2c1a96

Browse files
authored
Merge branch 'master' into locking
2 parents 6e3a0b2 + 334cb3b commit f2c1a96

File tree

4 files changed

+54
-7
lines changed

4 files changed

+54
-7
lines changed

redis/_parsers/base.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
NoScriptError,
3636
OutOfMemoryError,
3737
ReadOnlyError,
38-
RedisError,
3938
ResponseError,
4039
TryAgainError,
4140
)
@@ -415,7 +414,7 @@ def on_connect(self, connection):
415414
"""Called when the stream connects"""
416415
self._stream = connection._reader
417416
if self._stream is None:
418-
raise RedisError("Buffer is closed.")
417+
raise ConnectionError(SERVER_CLOSED_CONNECTION_ERROR)
419418
self.encoder = connection.encoder
420419
self._clear()
421420
self._connected = True
@@ -426,7 +425,7 @@ def on_disconnect(self):
426425

427426
async def can_read_destructive(self) -> bool:
428427
if not self._connected:
429-
raise RedisError("Buffer is closed.")
428+
raise OSError("Buffer is closed.")
430429
if self._buffer:
431430
return True
432431
try:

redis/_parsers/hiredis.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ def on_disconnect(self):
238238

239239
async def can_read_destructive(self):
240240
if not self._connected:
241-
raise ConnectionError(SERVER_CLOSED_CONNECTION_ERROR)
241+
raise OSError("Buffer is closed.")
242242
if self._reader.gets() is not NOT_ENOUGH_DATA:
243243
return True
244244
try:

redis/asyncio/lock.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,16 +263,27 @@ async def owned(self) -> bool:
263263
stored_token = encoder.encode(stored_token)
264264
return self.local.token is not None and stored_token == self.local.token
265265

266-
def release(self) -> Awaitable[None]:
267-
"""Releases the already acquired lock"""
266+
async def release(self) -> None:
267+
"""Releases the already acquired lock.
268+
269+
The token is only cleared after the Redis release operation completes
270+
successfully. This ensures that if the release is cancelled mid-operation,
271+
the lock state remains consistent and can be retried.
272+
"""
268273
expected_token = self.local.token
269274
if expected_token is None:
270275
raise LockError(
271276
"Cannot release a lock that's not owned or is already unlocked.",
272277
lock_name=self.name,
273278
)
279+
try:
280+
await self.do_release(expected_token)
281+
except LockNotOwnedError:
282+
# Lock doesn't exist in Redis, safe to clear token
283+
self.local.token = None
284+
raise
285+
# Only clear token after successful release
274286
self.local.token = None
275-
return self.do_release(expected_token)
276287

277288
async def do_release(self, expected_token: bytes) -> None:
278289
if not bool(

tests/test_asyncio/test_lock.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,43 @@ async def test_reacquiring_lock_no_longer_owned_raises_error(self, r):
259259
with pytest.raises(LockNotOwnedError):
260260
await lock.reacquire()
261261

262+
async def test_release_cancellation_preserves_lock_state(self, r):
263+
"""
264+
Test that cancelling release() doesn't leave lock in inconsistent state.
265+
266+
Regression test for GitHub issue #3847. Before the fix, if release()
267+
was cancelled during execution, the token would be cleared but the
268+
Redis key would remain, causing a permanent deadlock.
269+
"""
270+
lock = self.get_lock(r, "foo")
271+
await lock.acquire(blocking=False)
272+
273+
# Verify lock is owned
274+
original_token = lock.local.token
275+
assert original_token is not None
276+
assert await lock.owned()
277+
278+
# Create release task and cancel it immediately
279+
release_task = asyncio.create_task(lock.release())
280+
release_task.cancel()
281+
282+
try:
283+
await release_task
284+
except asyncio.CancelledError:
285+
# Expected: the release task was deliberately cancelled to test lock state.
286+
pass
287+
288+
# Check the lock state after cancellation
289+
if lock.local.token is not None:
290+
# Release was cancelled before completion - token preserved
291+
# This is the fix: we can now retry the release
292+
assert lock.local.token == original_token
293+
await lock.release()
294+
assert await lock.locked() is False
295+
else:
296+
# Release completed before cancel took effect
297+
assert await lock.locked() is False
298+
262299

263300
@pytest.mark.onlynoncluster
264301
class TestLockClassSelection:

0 commit comments

Comments
 (0)