Keep track of pending Redis get calls in client side caching to avoid race with invalidations. #3481

[IvarHenckel]

Closes #3481

Make sure that:

• You have read the contribution guidelines.
• You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
• You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
• You submit test cases (unit or integration tests) that back your changes.

---

Note

Medium Risk
Changes client-side caching semantics during cache misses by introducing a pending marker and conditional writes; concurrency/race behavior is subtle and could impact cache correctness under load.

Overview
Prevents a race where a cache miss triggers a Redis read that later overwrites a concurrent invalidation by introducing a pending state for in-flight Redis get calls.

CacheAccessor gains a new setPending hook (no-op by default), ClientSideCaching marks keys pending before Redis lookups, and the default Map accessor is replaced with PendingAwareCacheAccessor which uses a sentinel and map.replace(...) to only write results if the key is still pending.

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

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[IvarHenckel]

So far didn't add any tests. I felt I was only changing internal parts and there is already tests against the end behavior. Testing the race itself, i.e. somehow mocking the order of threads, is tricky. But let me know if you prefer more tests.

[IvarHenckel]

FYI I have been applying the same fix in my production code, it has been running there for weeks successfully. Would be nice to clean up the code by getting this fix into lettuce (currently have the implementation of CacheAccessor in our code base). And of course, to fix the issue for others.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

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

[cursor]

Sentinel leaks into map when Redis returns null

Medium Severity

When redisCache.get(key) returns null in the get(K key) method, setPending has already placed the REDIS_IN_PROGRESS sentinel into the user's Map<K, V>, but no subsequent put or evict call is made to clean it up. This leaves a type-unsafe Object in the user-owned typed map indefinitely. The sentinel accumulates for every queried-but-nonexistent key, causing a slow memory leak and map pollution (e.g., map.size() reports phantom entries, iterating values yields ClassCastException). The same issue applies in the get(K key, Callable<V> valueLoader) exception paths.

Additional Locations (1)

• src/main/java/io/lettuce/core/support/caching/ClientSideCaching.java#L142-L166

 

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

[cursor]

put silently drops updates violating public API contract

Medium Severity

The put method now uses map.replace(key, (V) REDIS_IN_PROGRESS, value), which only stores the value when the current entry is the sentinel. The old MapCacheAccessor used unconditional map.put(key, value). This silently breaks the CacheAccessor public interface contract, which documents that put will replace any previously contained mapping. Any external caller using CacheAccessor.forMap() and invoking put directly (e.g., to pre-populate or manually update the cache) will have their writes silently discarded.

 

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

[IvarHenckel]

Rebased to latest main to clean up diff. @tishun , pinging to see if there is any interest in this one?

Happy to add tests / edit based on review, anything else, but of course will only do so if there is interest. I personally think this is a crucial bug that can't be ignored.

You mentioned in 3481 that there are plans to migrate the code in question anyways. But since it's been some time I'm just looking for an update.