From 7a5fa6bf8d3e58910e7c196cb2a281c9b58faac3 Mon Sep 17 00:00:00 2001
From: daguimu <daguimu.geek@gmail.com>
Date: Wed, 25 Mar 2026 19:28:28 +0800
Subject: [PATCH] fix: make LRU2Cache.computeIfAbsent thread-safe by holding
 lock across entire operation

LRU2Cache.computeIfAbsent() released the lock between get() and put(),
breaking the atomicity that all other methods in this class maintain.
This caused duplicate function computation under concurrency and could
defeat the LRU-2 promotion logic.

The fix holds the lock across the entire computeIfAbsent operation,
inlining the LRU-2 promotion logic to avoid nested lock acquisition.
---
 .../apache/dubbo/common/utils/LRU2Cache.java  | 21 ++++--
 .../dubbo/common/utils/LRU2CacheTest.java     | 67 +++++++++++++++++++
 2 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/LRU2Cache.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/LRU2Cache.java
index 9f52c0b9ed5..c0a155de0c6 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/LRU2Cache.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/LRU2Cache.java
@@ -99,12 +99,23 @@ public V put(K key, V value) {
 
     @Override
     public V computeIfAbsent(K key, Function<? super K, ? extends V> fn) {
-        V value = get(key);
-        if (value == null) {
-            value = fn.apply(key);
-            put(key, value);
+        lock.lock();
+        try {
+            V value = super.get(key);
+            if (value == null) {
+                value = fn.apply(key);
+                // Inline the LRU-2 promotion logic from put() to avoid releasing the lock
+                if (preCache.containsKey(key)) {
+                    preCache.remove(key);
+                    super.put(key, value);
+                } else {
+                    preCache.put(key, true);
+                }
+            }
+            return value;
+        } finally {
+            lock.unlock();
         }
-        return value;
     }
 
     @Override
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/LRU2CacheTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/LRU2CacheTest.java
index 96b47e01732..45db2dc1547 100644
--- a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/LRU2CacheTest.java
+++ b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/LRU2CacheTest.java
@@ -16,10 +16,14 @@
  */
 package org.apache.dubbo.common.utils;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -101,6 +105,69 @@ void testTrimWhenCapacityReduced() {
         assertFalse(cache.containsKey("2"));
     }
 
+    @Test
+    void testComputeIfAbsentBasic() {
+        LRU2Cache<String, String> cache = new LRU2Cache<>(10);
+        // First call: goes to preCache, function is called
+        String result1 = cache.computeIfAbsent("key", k -> k.toUpperCase());
+        assertEquals("KEY", result1);
+
+        // Second call: promotes from preCache to main cache, function is called again (LRU-2 semantics)
+        String result2 = cache.computeIfAbsent("key", k -> k.toUpperCase());
+        assertEquals("KEY", result2);
+
+        // Third call: found in main cache, function should NOT be called
+        AtomicInteger callCount = new AtomicInteger(0);
+        String result3 = cache.computeIfAbsent("key", k -> {
+            callCount.incrementAndGet();
+            return k.toUpperCase();
+        });
+        assertEquals("KEY", result3);
+        assertEquals(0, callCount.get(), "Function should not be called when value is in main cache");
+    }
+
+    @Test
+    void testComputeIfAbsentConcurrency() throws InterruptedException {
+        LRU2Cache<String, String> cache = new LRU2Cache<>(1000);
+        int threadCount = 10;
+        int iterations = 1000;
+
+        // Pre-warm: put keys twice to promote them into main cache
+        for (int i = 0; i < 100; i++) {
+            String key = "key-" + i;
+            cache.put(key, key.toUpperCase());
+            cache.put(key, key.toUpperCase());
+        }
+
+        CountDownLatch startLatch = new CountDownLatch(1);
+        CountDownLatch doneLatch = new CountDownLatch(threadCount);
+        AtomicInteger errors = new AtomicInteger(0);
+
+        for (int t = 0; t < threadCount; t++) {
+            new Thread(() -> {
+                        try {
+                            startLatch.await();
+                            for (int i = 0; i < iterations; i++) {
+                                String key = "key-" + (i % 100);
+                                String value = cache.computeIfAbsent(key, k -> k.toUpperCase());
+                                if (value == null || !value.equals(key.toUpperCase())) {
+                                    errors.incrementAndGet();
+                                }
+                            }
+                        } catch (Exception e) {
+                            errors.incrementAndGet();
+                        } finally {
+                            doneLatch.countDown();
+                        }
+                    })
+                    .start();
+        }
+
+        startLatch.countDown();
+        doneLatch.await();
+        assertEquals(0, errors.get(), "Concurrent computeIfAbsent should not produce errors");
+    }
+
     @Test
     void testPreCacheTrimWhenCapacityReduced() {
         LRU2Cache<String, Integer> cache = new LRU2Cache<>(5);