Skip to content

Lock(timeout=0): reacquire()/extend(replace_ttl=True) silently DELETE the lock and return True #4279

Description

@Mukller

With Lock(timeout=0) (i.e. "hold until explicitly released"), calling reacquire() or extend(replace_ttl=True) deletes the lock key silently and returns True. The owner believes it renewed its lock; in reality the lock no longer exists and another process can acquire it immediately.

Version: redis-py 8.1.0 (also present on current master, redis/lock.py)
Python: 3.13.14

Root cause

  1. Lock.do_acquire() maps a falsy timeout to SET key token NX without PX — the key never expires (documented: "By default, it will remain locked until release() is called"). Good so far.
  2. But LUA_REACQUIRE_SCRIPT / LUA_EXTEND_SCRIPT unconditionally send PEXPIRE KEYS[1], ARGV[2] with ARGV[2] = int(timeout * 1000) = 0. Per PEXPIRE semantics: "...if the timeout is non-positive, the key will be deleted rather than expired."
  3. The script then returns 1 (the token matched), so reacquire() / extend() return True — success — while the lock was destroyed.

There is no guard anywhere that ttl > 0 in the Lua scripts or their Python wrappers.

Repro (client-side, exact Lua args captured)

from redis.lock import Lock

class FakeScript:
    def __init__(self, store, src): self.store, self.src = store, src
    def __call__(self, keys=None, args=None, client=None):
        self.store.append(args); return 1

class FakeClient:
    def __init__(self):
        self.calls = []
        self.set = lambda name, value, nx=False, px=None: True
    def register_script(self, script):
        return FakeScript(self.calls, script)
    def get_encoder(self):
        class E:
            def encode(self, v): return v.encode() if isinstance(v, str) else v
        return E()

client = FakeClient()
lk = Lock(client, "mylock", timeout=0)

# acquire path: px=None -> SET mylock tok NX (no PX) => never expires, correct per docs
assert lk.do_acquire(b"tok") is True

# renewal path:
print(lk.reacquire())                    # True  <-- "success"
print(client.calls[-1])                  # [b'tok', 0]  <-- PEXPIRE mylock 0 => DELETES the key
print(lk.extend(0, replace_ttl=True))    # True; args [b'tok', 0, '1']

On a real server the sequence is: lock exists with no TTL → reacquire()PEXPIRE mylock 0 → key deleted → method returns True.

Expected behavior

Per docstring — reacquire(): "Resets a TTL of an already acquired lock back to a timeout value" — for timeout=0 there is no TTL to reset, so either:

  1. keep the no-expiry semantics: scripts should skip PEXPIRE when the target ttl is <= 0 and just verify the token, returning True; or
  2. raise LockError("Lock is not acquired" / invalid timeout) instead of reporting success.

Option 1 preserves the documented invariant "timeout=0 ⇒ locked until release()", which is currently broken by any renewal call.

I can prepare a PR (guard inside both Lua scripts + tests covering the timeout=0 renewal path).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions