You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2018/01/25 22:25:06 UTC
[sling-org-apache-sling-commons-threads] 01/02: SLING-7447 - Race
condition in ThreadLocalCleaner initialization code
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
commit c472954b67524042618b59bb0e45f180bcea146f
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jan 26 00:07:47 2018 +0200
SLING-7447 - Race condition in ThreadLocalCleaner initialization code
Ensure that the reflection-obtained class fields are initialized eagerly
when the class is loaded. Since the class will be accessed only when the
thread pool is instantiated, the performance impact is minimal.
Moreover, this saves a synchronized check for each Runnable execution.
The gains are minimal, but there is less mutable state to reason about.
---
.../commons/threads/impl/ThreadLocalCleaner.java | 68 +++++++++-------------
1 file changed, 28 insertions(+), 40 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 5a85f72..3ac4b52 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
@@ -32,48 +32,38 @@ public class ThreadLocalCleaner {
private static final Logger LOG = LoggerFactory.getLogger(ThreadLocalCleaner.class);
- private final ThreadLocalChangeListener listener;
-
/* Reflection fields */
/** this field is in class {@link ThreadLocal} and is of type {@code ThreadLocal.ThreadLocalMap} */
- private static volatile Field threadLocalsField;
+ private static final Field threadLocalsField;
/** this field is in class {@link ThreadLocal} and is of type {@code ThreadLocal.ThreadLocalMap} */
- private static Field inheritableThreadLocalsField;
- private static Class<?> threadLocalMapClass;
+ private static final Field inheritableThreadLocalsField;
+ private static final Class<?> threadLocalMapClass;
/** this field is in class {@code ThreadLocal.ThreadLocalMap} and contains an array of {@code ThreadLocal.ThreadLocalMap.Entry's} */
- private static Field tableField;
- private static Class<?> threadLocalMapEntryClass;
+ private static final Field tableField;
+ private static final Class<?> threadLocalMapEntryClass;
/** this field is in class {@code ThreadLocal.ThreadLocalMap.Entry} and contains an object referencing the actual thread local
* variable */
- private static Field threadLocalEntryValueField;
+ private static final Field threadLocalEntryValueField;
/** this field is in the class {@code ThreadLocal.ThreadLocalMap} and contains the number of the entries */
- private static Field threadLocalMapSizeField;
+ private static final Field threadLocalMapSizeField;
/** this field is in the class {@code ThreadLocal.ThreadLocalMap} and next resize threshold */
- private static Field threadLocalMapThresholdField;
- private static volatile IllegalStateException reflectionException;
+ private static final Field threadLocalMapThresholdField;
- private static synchronized void initReflectionFields() throws IllegalStateException {
- // check if previous initialization lead to an exception
- if (reflectionException != null) {
- throw reflectionException;
- }
- // check if initialized
- if (threadLocalsField == null) {
- try {
- threadLocalsField = field(Thread.class, "threadLocals");
- inheritableThreadLocalsField = field(Thread.class, "inheritableThreadLocals");
- threadLocalMapClass = inner(ThreadLocal.class, "ThreadLocalMap");
- 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. " +
- "Will not be able to clear thread locals: " + e, e);
- throw reflectionException;
- }
+ static {
+ try {
+ threadLocalsField = field(Thread.class, "threadLocals");
+ inheritableThreadLocalsField = field(Thread.class, "inheritableThreadLocals");
+ threadLocalMapClass = inner(ThreadLocal.class, "ThreadLocalMap");
+ tableField = field(threadLocalMapClass, "table");
+ threadLocalMapEntryClass = inner(threadLocalMapClass, "Entry");
+ threadLocalEntryValueField = field(threadLocalMapEntryClass, "value");
+ threadLocalMapSizeField = field(threadLocalMapClass, "size");
+ threadLocalMapThresholdField = field(threadLocalMapClass, "threshold");
+ } catch (NoSuchFieldException e) {
+ ExceptionInInitializerError error = new ExceptionInInitializerError(
+ "Unable to access ThreadLocal class information using reflection");
+ error.initCause(e);
+ throw error;
}
}
@@ -125,10 +115,7 @@ public class ThreadLocalCleaner {
throw new IllegalStateException("Access denied", e);
}
}
-
- private ThreadLocalMapCopy threadLocalsCopy;
- private ThreadLocalMapCopy inheritableThreadLocalsCopy;
-
+
private static void restore(Field field, Object[] value, Integer size, Integer threshold) {
try {
Thread thread = Thread.currentThread();
@@ -146,11 +133,12 @@ public class ThreadLocalCleaner {
throw new IllegalStateException("Access denied", e);
}
}
+
+ private final ThreadLocalChangeListener listener;
+ private ThreadLocalMapCopy threadLocalsCopy;
+ private ThreadLocalMapCopy inheritableThreadLocalsCopy;
public ThreadLocalCleaner(ThreadLocalChangeListener listener) {
- if (threadLocalsField == null || reflectionException != null) {
- initReflectionFields();
- }
this.listener = listener;
saveOldThreadLocals();
}
--
To stop receiving notification emails like this one, please contact
rombert@apache.org.