Skip to content

Watchdog renewal can outlive unlock(): cancelLockRenewal races the acquire's async scheduleExpirationRenewal #7272

Description

@ptanov

Redis version

8.2.2

Redisson version

4.4.0 (also present in 3.51.0 - same renewal design)

Redisson configuration

Single server, watchdog enabled (lockWatchdogTimeout default 30000; the reproducer lowers it to 1000 to see the leak faster), default codec, JDK 21.

What is the Expected behavior?

Once a lock is released, its watchdog must stop renewing the key. An acquire followed by a release must never leave a key whose TTL keeps getting refreshed.

What is the Actual behavior?

A watchdog lock sometimes stays in Redis with its TTL refreshed indefinitely (PTTL keeps jumping back up to lockWatchdogTimeout) after the holder released it and its thread is gone. It is intermittent.

Additional information

Root cause

scheduleExpirationRenewal runs from an async callback on the acquire future, and cancelExpirationRenewal runs from an async callback on the unlock future. Nothing orders the two, so the cancel can run before the schedule.

RedissonLock.tryAcquireAsync schedules renewal from a thenApply on the acquire future (and only for the watchdog case, leaseTime <= 0):

ttlRemainingFuture = handleNoSync(threadId, ttlRemainingFuture);
CompletionStage<Long> f = ttlRemainingFuture.thenApply(ttlRemaining -> {
    if (ttlRemaining == null) {
        if (leaseTime > 0) {
            internalLockLeaseTime = unit.toMillis(leaseTime);
        } else {
            scheduleExpirationRenewal(threadId);
        }
    }
    return ttlRemaining;
});

RedissonBaseLock.unlockAsync0 cancels renewal from the unlock's handle:

CompletionStage<Void> f = future.handle((res, e) -> {
    cancelExpirationRenewal(threadId, res);
    ...
});

Both go through org.redisson.renewal.LockRenewalScheduler:

protected void scheduleExpirationRenewal(long threadId) {
    renewalScheduler.renewLock(getRawName(), threadId, getLockName(threadId));
}
protected void cancelExpirationRenewal(Long threadId, Boolean unlockResult) {
    renewalScheduler.cancelLockRenewal(getRawName(), threadId);
}

renewLock adds the lock to RenewalTask.name2entry and arms renewal; cancelLockRenewal removes it. Renewal continues while the lock is in name2entry.

The bad interleaving:

  1. The acquire SET is dispatched. The thenApply has not run yet, so the lock is not in name2entry.
  2. The unlock runs cancelLockRenewal, which finds the lock is not registered and does nothing. (The unlock DEL may also reach Redis before the acquire SET - they can use different pooled connections - so it finds nothing to delete.)
  3. The acquire's thenApply runs scheduleExpirationRenewal -> renewLock, registering the lock and arming renewal.
  4. No cancel is left to undo it, so the key is renewed indefinitely.

The cancel is lost because it runs before the renewLock it was meant to undo.

How we hit it

We use the blocking lockInterruptibly() and release in a finally. Normally the synchronous acquire only returns after the thenApply has run, so the release's cancel is safely ordered after the schedule and there is no race. But when the acquiring thread is interrupted, the acquire returns before the thenApply runs, and the release in the finally then races the still-pending acquire. Interrupting a blocking lockInterruptibly() and releasing in a finally is ordinary, correct code, and it leaks the watchdog.

Interruption is only how we trigger it. The underlying problem is that renewLock and cancelLockRenewal are not ordered.

Acquiring with an explicit positive lease (lockInterruptibly(leaseTime, unit)) never leaks, because scheduleExpirationRenewal is not called for a fixed lease. That confirms the diagnosis.

Reproducer

Needs a running Redis, plus redisson 4.4.0 and junit-jupiter. One thread acquires and releases the same lock in a loop while the test thread interrupts it. A short lockWatchdogTimeout makes a leak visible quickly. The race is timing-dependent; increase the duration or run a few threads if it does not reproduce on the first try.

import org.junit.jupiter.api.Test;
import org.redisson.Redisson;
import org.redisson.api.RLock;
import org.redisson.api.RedissonClient;
import org.redisson.config.Config;

import java.util.concurrent.atomic.AtomicBoolean;

import static org.junit.jupiter.api.Assertions.assertTrue;

class WatchdogRenewalLeakTest {

    @Test
    void interruptedAcquireCanLeakTheWatchdog() throws Exception {
        Config config = new Config();
        config.useSingleServer().setAddress("redis://127.0.0.1:6379");
        config.setLockWatchdogTimeout(1000);
        RedissonClient redisson = Redisson.create(config);
        String name = "test:watchdog:leak";
        redisson.getKeys().delete(name);

        AtomicBoolean stop = new AtomicBoolean();
        Thread worker = new Thread(() -> {
            RLock lock = redisson.getLock(name);
            while (!stop.get()) {
                Thread.interrupted();               // clear so the next interrupt lands during the acquire
                try {
                    lock.lockInterruptibly();
                } catch (InterruptedException | RuntimeException e) {
                    // acquire was interrupted (Redisson may wrap it as RedisException)
                } finally {
                    try {
                        lock.unlock();              // release whether or not we got it
                    } catch (RuntimeException ignored) {
                        // not held, or interrupted again during unlock
                    }
                }
            }
        });
        worker.start();

        long until = System.currentTimeMillis() + 20_000;
        while (System.currentTimeMillis() < until) {
            worker.interrupt();
        }
        stop.set(true);
        worker.interrupt();
        worker.join();

        Thread.sleep(3000);                         // longer than the watchdog TTL; a released key is gone by now
        long ttl = redisson.getMap(name).remainTimeToLive();
        redisson.getKeys().delete(name);
        redisson.shutdown();

        assertTrue(ttl <= 0,
                "lock key survived with ttl=" + ttl + "ms after the worker stopped; the watchdog is renewing an orphan");
    }
}

Confirmed reproduced with Redisson 4.4.0 against a stock Redis started with docker run --rm -p 6379:6379 redis:8.2.2: after the worker stops and 3 s pass, the lock key is still present with its TTL refreshed to ~1000 ms (the configured lockWatchdogTimeout) even though no thread holds it — the watchdog is renewing an orphan.

Suggested fix

Order or reconcile renewLock and cancelLockRenewal so a cancel issued before its renewLock is not lost. For example, a cancel could leave a short-lived tombstone that suppresses a following renewLock for the same thread, or the acquire's thenApply could skip or undo renewal when the acquire has already been cancelled.

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