Fixed - IndexOutOfBoundsException in ContainsDecoder.decode during lock watchdog renewal - #7403

Summary

ContainsDecoder kept a live reference to the caller-supplied Collection whenever it happened to be a List:

public ContainsDecoder(Collection<T> args) {
    if (args instanceof List) {
        this.args = (List<T>) args;   // aliases the caller's list
    } else {
        this.args = new ArrayList<>(args);
    }
}

LockTask.renew() and ReadLockTask.renew() both pass their mutable keys list to the decoder and then shrink that same list inside the completion callback:

new RedisCommand<>("EVAL", new ContainsDecoder<>(keys)),
...
return f.thenCompose(existingNames -> {
    keys.removeAll(existingNames);      // mutates the list the decoder holds
    ...
});

If the reply is decoded after that callback has already run — delayed Redis response, command retry, connection switch — decode() iterates over parts (whose size always equals the number of KEYS sent, since the Lua script returns one element per key) while reading from the now-shorter args list, and throws IndexOutOfBoundsException:

java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
    at java.util.ArrayList.get(ArrayList.java:428)
    at org.redisson.client.protocol.decoder.ContainsDecoder.decode(ContainsDecoder.java:50)
    at org.redisson.client.handler.CommandDecoder.decodeList(CommandDecoder.java:...)
    at org.redisson.renewal.LockTask.renew(LockTask.java:78)

Because the exception is thrown while decoding, the EVAL future is never completed. RenewalTask schedules the next round only from the whenComplete callback, so the watchdog renewal chain for that RedissonClient stalls until the command itself times out. Held locks can then expire while still in use, which silently breaks mutual exclusion.

Note that the existing parts.isEmpty() guard does not help here: this code path always sends at least one key, so parts is never empty. The keys.isEmpty() guard added in #6506 does not help either, because keys is non-empty when the command is fired — it is only shrunk afterwards.

Fix

Snapshot the incoming collection in the constructor:

public ContainsDecoder(Collection<T> args) {
    this.args = new ArrayList<>(args);
}

This makes the decoder immune to later caller-side mutations, preserves decoding semantics for every existing caller (only LockTask, ReadLockTask and RedissonSet construct it, none of which relies on the aliasing), and removes the instanceof List special case.

Reproduction

The failure is deterministic once the ordering is forced. With the unpatched ContainsDecoder from 3.52.0:

All three throw exactly the signatures reported from production. With the patch, all pass.

Tests

Added ContainsDecoderTest (JUnit 5, 6 cases) covering the plain decode paths, empty reply, full argument removal, partial argument removal, and decoding concurrent with caller-side mutation.

6 tests found
6 tests successful
0 tests failed

Against the unpatched 3.52.0 class the same suite reports 3 failures — the three mutation cases above.

On the remaining bounds behaviour

Worth being explicit about scope: this patch removes the only reachable cause of the exception, it does not turn decode() into code that cannot throw. The loop still reads args.get(index) without checking index < args.size(), so the following remain true after the patch:

Neither of the two remaining throwing cases is reachable from the renewal path:

• empty args cannot occur, because LockTask.renew() and ReadLockTask.renew() both return early when keys.isEmpty() (the guard added in IndexOutOfBoundsException from RenewalTask #6506), so the command is never dispatched with zero keys.
• parts.size() > args.size() cannot occur, because the returned parts size is fixed by the Lua script embedded in LockTask.renew() / ReadLockTask.renew(): it appends exactly one element per KEYS entry, and KEYS is built from the same args list. The two lengths are equal by construction, and after this patch args can no longer be shortened behind the decoder's back.

I deliberately kept the change to the constructor rather than adding a bounds check inside the loop. Silently skipping mismatched indices would mask a genuine protocol anomaly — if a future change to the Lua script or a decode ordering issue ever produced more elements than keys, the current behaviour surfaces it loudly instead of quietly renewing a subset of locks. If you would prefer defence in depth here anyway, the minimal version is:

for (int index = 0; index < parts.size() && index + shiftIndex < args.size(); index++) {

Happy to add that as a follow-up if you consider it preferable.

Related

• IndexOutOfBoundsException in ContainsDecoder.decode from lock watchdog renewal (mutable args list aliased) #7402 — the issue this PR fixes
• IndexOutOfBoundsException from RenewalTask #6506 / java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0 #6519 — different defect (LockTask.renew() dereferencing keys.get(0) when keys is empty); fixed in 3.46.1 and already present in 3.52.0
• Performance Issue with Redisson Lock Watchdog Mechanism When Handling a Large Number of Locks #6303 — the performance change that introduced batched renewal (597d604), which is what made the shared-list aliasing reachable