[dubbo] branch 3.2 updated: fix #11986 ConcurrentHashMapUtils lock bug (#11987)

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

albumenj pushed a commit to branch 3.2
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/3.2 by this push:
     new 0c11a0ddcf fix #11986 ConcurrentHashMapUtils lock bug (#11987)
0c11a0ddcf is described below

commit 0c11a0ddcfc607a1aa45ef2915479bc59f1e3e7d
Author: Golden Looly <lo...@aliyun.com>
AuthorDate: Wed May 10 13:30:36 2023 +0800

    fix #11986 ConcurrentHashMapUtils lock bug (#11987)
    
    * fix #11986
    
    * fix apache#11986
    
    * fix apache#11986
    
    * change test at jdk8
    
    * seprate test for jdk8 and jdk17
    
    * seprate jre version test
    
    * change comment to english
    
    * add null check for v
    
    ---------
    
    Co-authored-by: Albumen Kevin <jh...@gmail.com>
---
 .../dubbo/common/utils/ConcurrentHashMapUtils.java | 18 +++++++++++++-
 .../common/utils/ConcurrentHashMapUtilsTest.java   | 29 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtils.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtils.java
index 2574d10202..7e71a1bab0 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtils.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtils.java
@@ -17,6 +17,7 @@
 
 package org.apache.dubbo.common.utils;
 
+import java.util.Objects;
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Function;
 
@@ -32,10 +33,25 @@ public class ConcurrentHashMapUtils {
      *
      */
     public static <K, V> V computeIfAbsent(ConcurrentMap<K, V> map, K key, Function<? super K, ? extends V> func) {
+        Objects.requireNonNull(func);
         if (JRE.JAVA_8.isCurrentVersion()) {
             V v = map.get(key);
             if (null == v) {
-                v = map.computeIfAbsent(key, func);
+                // issue#11986 lock bug
+                // v = map.computeIfAbsent(key, func);
+
+                // this bug fix methods maybe cause `func.apply` multiple calls.
+                v = func.apply(key);
+                if(null == v){
+                    return null;
+                }
+                final V res = map.putIfAbsent(key, v);
+                if(null != res){
+                    // if pre value present, means other thread put value already, and putIfAbsent not effect
+                    // return exist value
+                    return res;
+                }
+                // if pre value is null, means putIfAbsent effected, return current value
             }
             return v;
         } else {
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtilsTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtilsTest.java
index 3fc895497f..66c1ae462d 100644
--- a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtilsTest.java
+++ b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/ConcurrentHashMapUtilsTest.java
@@ -18,10 +18,12 @@
 package org.apache.dubbo.common.utils;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledForJreRange;
 
 import java.util.concurrent.ConcurrentHashMap;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 
 class ConcurrentHashMapUtilsTest {
@@ -37,4 +39,31 @@ class ConcurrentHashMapUtilsTest {
         ifAbsent = ConcurrentHashMapUtils.computeIfAbsent(map, "mxsm", k -> "mxsm1");
         assertEquals("mxsm1", ifAbsent);
     }
+
+    @Test
+    @EnabledForJreRange(max = org.junit.jupiter.api.condition.JRE.JAVA_8)
+    public void issue11986ForJava8Test(){
+        // https://github.com/apache/dubbo/issues/11986
+        final ConcurrentHashMap<String,Integer> map=new ConcurrentHashMap<>();
+        // // map.computeIfAbsent("AaAa", key->map.computeIfAbsent("BBBB",key2->42));
+
+        // In JDK8，for the bug of JDK-8161372，may cause dead cycle when use computeIfAbsent
+        // ConcurrentHashMapUtils.computeIfAbsent method to resolve this bug
+        ConcurrentHashMapUtils.computeIfAbsent(map, "AaAa", key->map.computeIfAbsent("BBBB",key2->42));
+        assertEquals(2, map.size());
+        assertEquals(Integer.valueOf(42), map.get("AaAa"));
+        assertEquals(Integer.valueOf(42), map.get("BBBB"));
+    }
+
+    @Test
+    @EnabledForJreRange(min = org.junit.jupiter.api.condition.JRE.JAVA_9)
+    public void issue11986ForJava17Test(){
+        // https://github.com/apache/dubbo/issues/11986
+        final ConcurrentHashMap<String,Integer> map=new ConcurrentHashMap<>();
+
+        // JDK9+ has been resolved JDK-8161372 bug, when cause dead then throw IllegalStateException
+        assertThrows(IllegalStateException.class, ()->{
+            ConcurrentHashMapUtils.computeIfAbsent(map, "AaAa", key->map.computeIfAbsent("BBBB",key2->42));
+        });
+    }
 }