You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2022/02/06 04:09:56 UTC

[dubbo] branch 3.0 updated: Fix InternalThreadLocalMap capacity expands overflow; Change access level of field InternalThreadLocalMap.UNSET to package private (#9564)

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

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


The following commit(s) were added to refs/heads/3.0 by this push:
     new 3444b9a  Fix InternalThreadLocalMap capacity expands overflow; Change access level of field InternalThreadLocalMap.UNSET to package private (#9564)
3444b9a is described below

commit 3444b9a697b683b9dad4cd0dbdb44f7c35d75894
Author: old driver <la...@gmail.com>
AuthorDate: Sun Feb 6 12:09:42 2022 +0800

    Fix InternalThreadLocalMap capacity expands overflow; Change access level of field InternalThreadLocalMap.UNSET to package private (#9564)
    
    Co-authored-by: laosijikaichele <laosijikaichele>
---
 .../common/threadlocal/InternalThreadLocalMap.java | 31 ++++++++----
 .../threadlocal/InternalThreadLocalTest.java       | 58 ++++++++++++++++++++++
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalMap.java b/dubbo-common/src/main/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalMap.java
index 54c3e60..e683b28 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalMap.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalMap.java
@@ -32,7 +32,12 @@ public final class InternalThreadLocalMap {
 
     private static final AtomicInteger NEXT_INDEX = new AtomicInteger();
 
-    public static final Object UNSET = new Object();
+    static final Object UNSET = new Object();
+
+    // Reference: https://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/util/ArrayList.java#l229
+    private static final int ARRAY_LIST_CAPACITY_MAX_SIZE = Integer.MAX_VALUE - 8;
+
+    private static final int ARRAY_LIST_CAPACITY_EXPAND_THRESHOLD = 1 << 30;
 
     public static InternalThreadLocalMap getIfSet() {
         Thread thread = Thread.currentThread();
@@ -65,8 +70,8 @@ public final class InternalThreadLocalMap {
 
     public static int nextVariableIndex() {
         int index = NEXT_INDEX.getAndIncrement();
-        if (index < 0) {
-            NEXT_INDEX.decrementAndGet();
+        if (index >= ARRAY_LIST_CAPACITY_MAX_SIZE || index < 0) {
+            NEXT_INDEX.set(ARRAY_LIST_CAPACITY_MAX_SIZE);
             throw new IllegalStateException("Too many thread-local indexed variables");
         }
         return index;
@@ -133,13 +138,19 @@ public final class InternalThreadLocalMap {
     }
 
     private static int newCapacity(int index) {
-        int newCapacity = index;
-        newCapacity |= newCapacity >>> 1;
-        newCapacity |= newCapacity >>> 2;
-        newCapacity |= newCapacity >>> 4;
-        newCapacity |= newCapacity >>> 8;
-        newCapacity |= newCapacity >>> 16;
-        return ++newCapacity;
+        int newCapacity;
+        if (index < ARRAY_LIST_CAPACITY_EXPAND_THRESHOLD) {
+            newCapacity = index;
+            newCapacity |= newCapacity >>>  1;
+            newCapacity |= newCapacity >>>  2;
+            newCapacity |= newCapacity >>>  4;
+            newCapacity |= newCapacity >>>  8;
+            newCapacity |= newCapacity >>> 16;
+            newCapacity ++;
+        } else {
+            newCapacity = ARRAY_LIST_CAPACITY_MAX_SIZE;
+        }
+        return newCapacity;
     }
 
     private static InternalThreadLocalMap fastGet(InternalThread thread) {
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalTest.java
index ad88c3d..4f243c4 100644
--- a/dubbo-common/src/test/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalTest.java
+++ b/dubbo-common/src/test/java/org/apache/dubbo/common/threadlocal/InternalThreadLocalTest.java
@@ -21,11 +21,19 @@ import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.lang.reflect.Field;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.LockSupport;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 public class InternalThreadLocalTest {
 
     private static final int THREADS = 10;
@@ -221,4 +229,54 @@ public class InternalThreadLocalTest {
         t.start();
         LockSupport.park(mainThread);
     }
+
+    @Test
+    public void testConstructionWithIndex() throws Exception {
+        int ARRAY_LIST_CAPACITY_MAX_SIZE = Integer.MAX_VALUE - 8;
+        Field nextIndexField =
+            InternalThreadLocalMap.class.getDeclaredField("NEXT_INDEX");
+        nextIndexField.setAccessible(true);
+        AtomicInteger nextIndex = (AtomicInteger) nextIndexField.get(AtomicInteger.class);
+        int nextIndex_before = nextIndex.get();
+        final AtomicReference<Throwable> throwable = new AtomicReference<Throwable>();
+        try {
+            while (nextIndex.get() < ARRAY_LIST_CAPACITY_MAX_SIZE) {
+                new InternalThreadLocal<Boolean>();
+            }
+            assertEquals(ARRAY_LIST_CAPACITY_MAX_SIZE - 1, InternalThreadLocalMap.lastVariableIndex());
+            try {
+                new InternalThreadLocal<Boolean>();
+            } catch (Throwable t) {
+                throwable.set(t);
+            }
+            // Assert the max index cannot greater than (ARRAY_LIST_CAPACITY_MAX_SIZE - 1)
+            assertThat(throwable.get(), is(instanceOf(IllegalStateException.class)));
+            // Assert the index was reset to ARRAY_LIST_CAPACITY_MAX_SIZE after it reaches ARRAY_LIST_CAPACITY_MAX_SIZE
+            assertEquals(ARRAY_LIST_CAPACITY_MAX_SIZE - 1, InternalThreadLocalMap.lastVariableIndex());
+        } finally {
+            // Restore the index
+            nextIndex.set(nextIndex_before);
+        }
+    }
+
+    @Test
+    public void testInternalThreadLocalMapExpand() throws Exception {
+        final AtomicReference<Throwable> throwable = new AtomicReference<Throwable>();
+        Runnable runnable = new Runnable() {
+            @Override
+            public void run() {
+                int expand_threshold = 1 << 30;
+                try {
+                    InternalThreadLocalMap.get().setIndexedVariable(expand_threshold, null);
+                } catch (Throwable t) {
+                    throwable.set(t);
+                }
+            }
+        };
+        InternalThread internalThread = new InternalThread(runnable);
+        internalThread.start();
+        internalThread.join();
+        // Assert the expanded size is not overflowed to negative value
+        assertThat(throwable.get(), is(not(instanceOf(NegativeArraySizeException.class))));
+    }
 }