SLING-7432 no longer rely on thread locals themselves to store old
thread local state
add dump method for debugging purposes
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 b755034..ab72c9f 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
@@ -21,6 +21,7 @@
import java.util.Arrays;
import org.apache.sling.commons.threads.impl.ThreadLocalChangeListener.Mode;
+import org.slf4j.Logger;
/** Notifies a {@link ThreadLocalChangeListener} about changes on a thread local storage. In addition it removes all references to variables
* being added to the thread local storage while the cleaner was running with its {@link cleanup} method.
@@ -47,7 +48,6 @@
private static Field threadLocalMapThresholdField;
private static volatile IllegalStateException reflectionException;
-
public ThreadLocalCleaner(ThreadLocalChangeListener listener) {
if (threadLocalsField == null || reflectionException != null) {
initReflectionFields();
@@ -75,16 +75,40 @@
} catch (NoSuchFieldException e) {
reflectionException = new IllegalStateException(
"Could not locate threadLocals field in class Thread. " +
- "Will not be able to clear thread locals: " + e, e);
+ "Will not be able to clear thread locals: " + e,
+ e);
throw reflectionException;
}
}
}
+ /** This is only for debugging purposes. Gives out all thread locals bound to the current thread to the logger.
+ *
+ * @param log
+ * @throws IllegalArgumentException
+ * @throws IllegalAccessException */
+ void dump(Logger log) {
+ Thread thread = Thread.currentThread();
+ Object threadLocals;
+ try {
+ threadLocals = threadLocalsField.get(thread);
+ Reference<?>[] currentReferences = (Reference<?>[]) tableField.get(threadLocals);
+ int size = (int) threadLocalMapSizeField.get(threadLocals);
+ log.info("Found {} thread locals bound to thread {}", size, thread);
+ for (Reference<?> curRef : currentReferences) {
+ if (curRef != null) {
+ log.info("Found reference {} with value {}", (ThreadLocal<?>) curRef.get(), threadLocalEntryValueField.get(curRef));
+ }
+ }
+ } catch (IllegalArgumentException | IllegalAccessException e) {
+ log.error("Can not dump thread locals for thread {}: {}", thread, e, e);
+ }
+ }
+
public void cleanup() {
// the first two diff calls are only to notify the listener, the actual cleanup is done by restoreOldThreadLocals
- diff(threadLocalsField, copyOfThreadLocals.get());
- diff(inheritableThreadLocalsField, copyOfInheritableThreadLocals.get());
+ diff(threadLocalsField, copyOfThreadLocals);
+ diff(inheritableThreadLocalsField, copyOfInheritableThreadLocals);
restoreOldThreadLocals();
}
@@ -175,20 +199,20 @@
"Could not find inner class " + name + " in " + clazz);
}
- 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 Reference<?>[] copyOfThreadLocals;
+ private Integer copyOfThreadLocalsSize;
+ private Integer copyOfThreadLocalsThreshold;
+ private Reference<?>[] copyOfInheritableThreadLocals;
+ private Integer copyOfInheritableThreadLocalsSize;
+ private Integer copyOfInheritableThreadLocalsThreshold;
- 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 void saveOldThreadLocals() {
+ copyOfThreadLocals = copy(threadLocalsField);
+ copyOfThreadLocalsSize = size(threadLocalsField, threadLocalMapSizeField);
+ copyOfThreadLocalsThreshold = size(threadLocalsField, threadLocalMapThresholdField);
+ copyOfInheritableThreadLocals = copy(inheritableThreadLocalsField);
+ copyOfInheritableThreadLocalsSize = size(inheritableThreadLocalsField, threadLocalMapSizeField);
+ copyOfInheritableThreadLocalsThreshold = size(inheritableThreadLocalsField, threadLocalMapThresholdField);
}
private static Reference<?>[] copy(Field field) {
@@ -216,15 +240,14 @@
}
}
- private static void restoreOldThreadLocals() {
+ private void restoreOldThreadLocals() {
try {
- restore(inheritableThreadLocalsField, copyOfInheritableThreadLocals.get(),
- copyOfInheritableThreadLocalsSize.get(), copyOfInheritableThreadLocalsThreshold.get());
- restore(threadLocalsField, copyOfThreadLocals.get(),
- copyOfThreadLocalsSize.get(), copyOfThreadLocalsThreshold.get());
+ restore(inheritableThreadLocalsField, copyOfInheritableThreadLocals,
+ copyOfInheritableThreadLocalsSize, copyOfInheritableThreadLocalsThreshold);
+ restore(threadLocalsField, copyOfThreadLocals,
+ copyOfThreadLocalsSize, copyOfThreadLocalsThreshold);
} finally {
- copyOfThreadLocals.remove();
- copyOfInheritableThreadLocals.remove();
+
}
}
diff --git a/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java b/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
index 922f18b..29dfde4 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
@@ -54,6 +54,8 @@
LOGGER.debug("Collecting changes to ThreadLocal for thread {} from now on...", t);
try {
ThreadLocalCleaner cleaner = new ThreadLocalCleaner(listener);
+ LOGGER.info("Before thread execution");
+ cleaner.dump(LOGGER);
local.set(cleaner);
} catch (Throwable e) {
LOGGER.warn("Could not set up thread local cleaner (most probably not a compliant JRE): {}", e, e);
@@ -64,7 +66,11 @@
LOGGER.debug("Cleaning up thread locals for thread {}...", Thread.currentThread());
ThreadLocalCleaner cleaner = local.get();
if (cleaner != null) {
+ LOGGER.info("After thread execution before cleanup");
+ cleaner.dump(LOGGER);
cleaner.cleanup();
+ LOGGER.info("After thread execution after cleanup");
+ cleaner.dump(LOGGER);
} else {
LOGGER.warn("Could not clean up thread locals in thread {} as the cleaner was not set up correctly", Thread.currentThread());
}
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 0160e64..f56456e 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
@@ -24,13 +24,12 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionHandler;
- import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import org.apache.sling.commons.threads.impl.ThreadLocalChangeListener.Mode;
import org.junit.Assert;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentMatchers;
@@ -55,7 +54,7 @@
1, 1, 100, TimeUnit.MILLISECONDS,
queue, Executors.defaultThreadFactory(), rejectionHandler, listener);
}
-
+
@Test(timeout = 10000)
public void threadLocalCleanupWorksWithResize() throws Exception {
@@ -82,6 +81,8 @@
private void assertTaskDoesNotSeeOldThreadLocals(String value) throws InterruptedException, ExecutionException {
ThreadLocalTask task = new ThreadLocalTask(value);
pool.submit(task).get();
+ // when get returns the task may not have been cleaned up (i.e. afterExecute() was no necessarily executed), therefore wait a littlebit here
+ Thread.sleep(500);
Assert.assertNull(task.getOldValue());
}
