Race condition in ConcurrentMapCache [SPR-16533]

[spring-projects-issues]

William Hoyle opened SPR-16533 and commented

ConcurrentMapCache.get(Object, Callable) can throw a NullPointerException if the key is evicted by another thread during the get.

From the source:

if (this.store.containsKey(key)) {
	// get(key) may return null if another thread evicts the key  
	return (T) get(key).get();
}

This is fixed by #18383 in the 5.0 branch.

Test code:

import org.springframework.cache.concurrent.ConcurrentMapCache;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;

public class Scratch {
    public static void main(String[] args) {
        ConcurrentMapCache cache = new ConcurrentMapCache("foo");

        ExecutorService executor = Executors.newCachedThreadPool();
        AtomicInteger count = new AtomicInteger();
        int iterations = 100;
        for (int i = 0; i < iterations; i++) {
            executor.execute(() -> {
                for (int j = 0; j < iterations; j++) {
                    int k = j;
                    try {
                        if (cache.get("test", () -> k) != null) {
                            count.incrementAndGet();
                        }
                    } catch (Throwable t) {
                        // ignore problems
                    }
                    cache.evict("test");
                }
            });
        }
        executor.shutdown();

        int expected = iterations * iterations;
        if (count.get() != expected) {
            throw new AssertionError("Expected " + expected + ", got " + count);
        }
    }
}

---

Affects: 4.3.14

Issue Links:

• Better synchronization in ConcurrentMapCache [SPR-13810] #18383 Better synchronization in ConcurrentMapCache

Referenced from: commits 06e2bad

[spring-projects-issues]

Juergen Hoeller commented

The containsKey check isn't necessary anyway since we can simply check the get(key) result for null, differentiating between a non-existing key and a null value that way. Fixed for 4.3.15 now.