sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From romb...@apache.org
Subject [sling-org-apache-sling-commons-threads] branch master updated: SLING-7432 - Thread pool clean up code can lead to infinite loops in ThreadLocal.get
Date Tue, 23 Jan 2018 14:33:11 GMT
This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git


The following commit(s) were added to refs/heads/master by this push:
     new afa59d6  SLING-7432 - Thread pool clean up code can lead to infinite loops in ThreadLocal.get
afa59d6 is described below

commit afa59d668ac39004510fcd4366298f62179736c4
Author: Robert Munteanu <rombert@apache.org>
AuthorDate: Tue Jan 23 16:32:50 2018 +0200

    SLING-7432 - Thread pool clean up code can lead to infinite loops in
    ThreadLocal.get
    
    Save and restore the size and threshold fields of the ThreadLocalMap as
    well as the entries. Not saving them would lead to situations there the
    ThreadLocalMap would be full and not resized, since the initial size and
    threshold values were being kept.
---
 .../commons/threads/impl/ThreadLocalCleaner.java   | 46 +++++++++++++++++-----
 ...ThreadPoolExecutorCleaningThreadLocalsTest.java |  1 -
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
index 6a0a067..98f264f 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
@@ -41,8 +41,13 @@ public class ThreadLocalCleaner {
     /** this field is in class {@code ThreadLocal.ThreadLocalMap.Entry} and contains an object
referencing the actual thread local
      * variable */
     private static Field threadLocalEntryValueField;
+    /** this field is in the class {@code ThreadLocal.ThreadLocalMap} and contains the number
of the entries */
+    private static Field threadLocalMapSizeField;
+    /** this field is in the class {@code ThreadLocal.ThreadLocalMap} and next resize threshold
*/
+    private static Field threadLocalMapThresholdField;
     private static IllegalStateException reflectionException;
 
+
     public ThreadLocalCleaner(ThreadLocalChangeListener listener) {
         if (threadLocalsField == null) {
             initReflectionFields();
@@ -65,6 +70,8 @@ public class ThreadLocalCleaner {
                 tableField = field(threadLocalMapClass, "table");
                 threadLocalMapEntryClass = inner(threadLocalMapClass, "Entry");
                 threadLocalEntryValueField = field(threadLocalMapEntryClass, "value");
+                threadLocalMapSizeField = field(threadLocalMapClass, "size");
+                threadLocalMapThresholdField = field(threadLocalMapClass, "threshold");
             } catch (NoSuchFieldException e) {
                 reflectionException = new IllegalStateException(
                         "Could not locate threadLocals field in class Thread.  " +
@@ -169,12 +176,19 @@ public class ThreadLocalCleaner {
     }
 
     private static final ThreadLocal<Reference<?>[]> copyOfThreadLocals = new
ThreadLocal<>();
-
+    private static final ThreadLocal<Integer> copyOfThreadLocalsSize = new ThreadLocal<>();
+    private static final ThreadLocal<Integer> copyOfThreadLocalsThreshold = new ThreadLocal<>();
     private static final ThreadLocal<Reference<?>[]> copyOfInheritableThreadLocals
= new ThreadLocal<>();
+    private static final ThreadLocal<Integer> copyOfInheritableThreadLocalsSize = new
ThreadLocal<>();
+    private static final ThreadLocal<Integer> copyOfInheritableThreadLocalsThreshold
= new ThreadLocal<>();
 
     private static void saveOldThreadLocals() {
         copyOfThreadLocals.set(copy(threadLocalsField));
+        copyOfThreadLocalsSize.set(size(threadLocalsField, threadLocalMapSizeField));
+        copyOfThreadLocalsThreshold.set(size(threadLocalsField, threadLocalMapThresholdField));
         copyOfInheritableThreadLocals.set(copy(inheritableThreadLocalsField));
+        copyOfInheritableThreadLocalsSize.set(size(inheritableThreadLocalsField, threadLocalMapSizeField));
+        copyOfInheritableThreadLocalsThreshold.set(size(inheritableThreadLocalsField, threadLocalMapThresholdField));
     }
 
     private static Reference<?>[] copy(Field field) {
@@ -190,31 +204,43 @@ public class ThreadLocalCleaner {
         }
     }
 
+    private static Integer size(Field field, Field sizeField) {
+        try {
+            Thread thread = Thread.currentThread();
+            Object threadLocals = field.get(thread);
+            if (threadLocals == null)
+                return null;
+            return (Integer) sizeField.get(threadLocals);
+        } catch (IllegalAccessException e) {
+            throw new IllegalStateException("Access denied", e);
+        }
+    }
+
     private static void restoreOldThreadLocals() {
         try {
-            restore(inheritableThreadLocalsField, copyOfInheritableThreadLocals.get());
-            restore(threadLocalsField, copyOfThreadLocals.get());
+            restore(inheritableThreadLocalsField, copyOfInheritableThreadLocals.get(),
+                copyOfInheritableThreadLocalsSize.get(), copyOfInheritableThreadLocalsThreshold.get());
+            restore(threadLocalsField, copyOfThreadLocals.get(),
+                copyOfThreadLocalsSize.get(), copyOfThreadLocalsThreshold.get());
         } finally {
             copyOfThreadLocals.remove();
             copyOfInheritableThreadLocals.remove();
         }
     }
 
-    private static void restore(Field field, Object value) {
+    private static void restore(Field field, Object value, Integer size, Integer threshold)
{
         try {
             Thread thread = Thread.currentThread();
             if (value == null) {
                 field.set(thread, null);
             } else {
-                tableField.set(field.get(thread), value);
+                final Object threadLocals = field.get(thread);
+                tableField.set(threadLocals, value);
+                threadLocalMapSizeField.set(threadLocals, size);
+                threadLocalMapThresholdField.set(threadLocals, threshold);
             }
         } catch (IllegalAccessException e) {
             throw new IllegalStateException("Access denied", e);
         }
     }
-
-    static {
-        // TODO: move to a place where the exception can be caught!
-
-    }
 }
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
b/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
index 7e6b92a..0160e64 100644
--- a/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
+++ b/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
@@ -57,7 +57,6 @@ public class ThreadPoolExecutorCleaningThreadLocalsTest {
     }
     
     @Test(timeout = 10000)
-    @Ignore
     public void threadLocalCleanupWorksWithResize() throws Exception {
         
         // configure thread local counts to make sure that

-- 
To stop receiving notification emails like this one, please contact
rombert@apache.org.

Mime
View raw message