fix(lock): raise on reacquire() with timeout=0 instead of deleting the lock

[simpleqt]

Fixes #4279

The bug

Lock(timeout=0).reacquire() rejected timeout is None but let timeout=0 through. timeout=0 → PEXPIRE key 0, which deletes the key (Redis ≥ 7.0), and the Lua script returned 1 — so reacquire() returned True while the lock was gone: mutual exclusion silently destroyed, the caller told everything was fine, and any other client could immediately acquire the same lock.

Fix

do_reacquire() now raises the same LockError that reacquire() already raises for timeout=None when the timeout is ≤ 0.

Repro (executed with fakeredis + Lua)

lock = Lock(r, 'mylock', timeout=0)
lock.acquire()         # True
lock.reacquire()       # True   <-- reported success
r.exists('mylock')     # False  <-- key deleted

---

Note

Medium Risk
Changes distributed lock renewal behavior for the edge case timeout=0, but the new failure mode is safer than silently deleting the lock key.

Overview
Fixes a silent mutual-exclusion break when Lock(..., timeout=0).reacquire() ran after a successful acquire().

reacquire() already rejected timeout is None, but timeout=0 was allowed. That path converted the timeout to PEXPIRE key 0, which on Redis ≥ 7.0 removes the lock key while the Lua reacquire script still returned success—so callers got True even though the lock no longer existed.

do_reacquire() now raises LockError (same pattern as the no-timeout case) when timeout <= 0, before touching Redis, so reacquire cannot report success after wiping the lock.

^{Reviewed by Cursor Bugbot for commit ba7657d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit ba7657d. Configure here.}

[cursor]

Async lock omits timeout guard

High Severity

The timeout <= 0 guard was added only on the sync Lock. Async do_reacquire still turns timeout=0 into PEXPIRE with 0 milliseconds, so reacquire() can delete the key, return success, and drop mutual exclusion for asyncio callers.

 

^{Reviewed by Cursor Bugbot for commit ba7657d. Configure here.}

[petyaslavova]

Hey @simpleqt, thank you for your contribution!

The underlying bug in #4279 is real, but it is already being fixed in #4282, which enforces the invariant inside the Lua scripts instead: it covers reacquire() and the extend() paths where the computed TTL lands at or below zero, negative and computed values rather than only timeout=0, and both the sync and async stacks. #4286 was closed for the same reason earlier.

Raising here would also change behavior in the other direction. Lock(timeout=0) acquires with no expiry on purpose, so reacquire() should keep it persistent rather than become a hard error, and turning a call that returns True today into a LockError is a released-error-semantics change we are not taking right now. This change is also sync-only, so redis/asyncio/lock.py would keep the defect.

Closing this one in favor of #4282. Your docs fixes were very welcome, and if you would like to help here, an extra regression test on #4282 for extend(-20) on a timeout=10 lock would be genuinely useful.