From c1aa1b7405afac5d29b260888b5980d1548bdcfd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=EA=B9=80=EC=A4=80=ED=98=95?=
 <158379358+junhyeong9812@users.noreply.github.com>
Date: Tue, 15 Sep 2026 01:06:47 +0900
Subject: [PATCH] Prevent double size decrement in ConcurrentLruCache

markAsRemoved() transitions a node to the removed state and decrements
the current size, but it did not check whether the node had already
been removed. The eviction path and an explicit removal can process
the same node in sequence: when a write drain runs a queued AddTask
whose eviction polls a node that a concurrent remove(K) has already
taken out of the cache, the eviction decrements the size, and the
queued RemovalTask for the same node decrements it again. The sibling
transition markForRemoval() guards against invalid transitions; this one
did not.

Each extra decrement makes currentSize permanently smaller than the
number of cached entries, so eviction stops triggering and the cache
exceeds its capacity for good, silently. A bounded two-thread stress
run accumulates the drift reliably: before the change the cache
stabilized far above its capacity in 20 out of 20 runs.

markAsRemoved() now returns without decrementing when the entry is
already in the removed state, mirroring the guard in markForRemoval().
The removed state is terminal, so each node is counted down exactly
once. The new test races explicit removals against eviction and then
verifies that the cache converges back to its capacity; it also
asserts that the racing thread ran and terminated cleanly.

Closes gh-37268

Signed-off-by: junhyeong9812 <pickjog@gmail.com>
---
 .../util/ConcurrentLruCache.java              |  6 ++-
 .../util/ConcurrentLruCacheTests.java         | 45 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java
index 279e75b0ebf..23052d7498b 100644
--- a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java
+++ b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java
@@ -199,11 +199,15 @@ public final class ConcurrentLruCache<K, V> {
 	}
 
 	/*
-	 * Transition the node to the {@code removed} state and decrement the current size of the cache.
+	 * Transition the node to the {@code removed} state and decrement the
+	 * current size of the cache, unless the node has already been removed.
 	 */
 	private void markAsRemoved(Node<K, V> node) {
 		for (; ; ) {
 			CacheEntry<V> current = node.get();
+			if (current.state == CacheEntryState.REMOVED) {
+				return;
+			}
 			CacheEntry<V> removed = new CacheEntry<>(current.value, CacheEntryState.REMOVED);
 			if (node.compareAndSet(current, removed)) {
 				this.currentSize.lazySet(this.currentSize.get() - 1);
diff --git a/spring-core/src/test/java/org/springframework/util/ConcurrentLruCacheTests.java b/spring-core/src/test/java/org/springframework/util/ConcurrentLruCacheTests.java
index 784bc97f376..b4dfadf08e0 100644
--- a/spring-core/src/test/java/org/springframework/util/ConcurrentLruCacheTests.java
+++ b/spring-core/src/test/java/org/springframework/util/ConcurrentLruCacheTests.java
@@ -16,6 +16,10 @@
 
 package org.springframework.util;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -108,4 +112,45 @@ class ConcurrentLruCacheTests {
 		assertThat(this.cache.contains("k3")).isTrue();
 	}
 
+	@Test
+	void removeRacingWithEvictionDoesNotExceedCapacity() throws Exception {
+		ConcurrentLruCache<Integer, String> cache = new ConcurrentLruCache<>(2, key -> "value" + key);
+		AtomicBoolean stop = new AtomicBoolean();
+		AtomicInteger removals = new AtomicInteger();
+		AtomicReference<Throwable> failure = new AtomicReference<>();
+		Thread remover = new Thread(() -> {
+			try {
+				while (!stop.get()) {
+					cache.get(0);
+					if (cache.remove(0)) {
+						removals.incrementAndGet();
+					}
+				}
+			}
+			catch (Throwable ex) {
+				failure.set(ex);
+			}
+		});
+		remover.start();
+		try {
+			for (int i = 1; i <= 50_000; i++) {
+				cache.get(i);
+			}
+		}
+		finally {
+			stop.set(true);
+			remover.join(5000);
+		}
+		int budget = 50_000;
+		int key = 100_000;
+		while (cache.size() > cache.capacity() && budget-- > 0) {
+			cache.get(key++);
+		}
+
+		assertThat(remover.isAlive()).isFalse();
+		assertThat(failure.get()).isNull();
+		assertThat(removals.get()).isGreaterThan(0);
+		assertThat(cache.size()).isLessThanOrEqualTo(cache.capacity());
+	}
+
 }