This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch bugfix/SLING-7432
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git
The following commit(s) were added to refs/heads/bugfix/SLING-7432 by this push:
new 6020992 SLING-7432 no longer rely on thread locals themselves to
store old thread local state
6020992 is described below
commit 60209922e77383f71318d5249fba32bd6b21fd3a
Author: Konrad Windszus <[email protected]>
AuthorDate: Wed Jan 24 17:02:06 2018 +0100
SLING-7432 no longer rely on thread locals themselves to store old
thread local state
add dump method for debugging purposes
---
.../commons/threads/impl/ThreadLocalCleaner.java | 76 +++++++++++++++-------
.../ThreadPoolExecutorCleaningThreadLocals.java | 6 ++
...ThreadPoolExecutorCleaningThreadLocalsTest.java | 7 +-
3 files changed, 61 insertions(+), 28 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 b755034..b3ecb9e 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.lang.reflect.Field;
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 @@ public class ThreadLocalCleaner {
private static Field threadLocalMapThresholdField;
private static volatile IllegalStateException reflectionException;
-
public ThreadLocalCleaner(ThreadLocalChangeListener listener) {
if (threadLocalsField == null || reflectionException != null) {
initReflectionFields();
@@ -75,16 +75,40 @@ public class ThreadLocalCleaner {
} 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,23 @@ public class ThreadLocalCleaner {
"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 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 Reference<?>[] copyOfThreadLocals;
+ private Integer copyOfThreadLocalsSize;
+ private Integer copyOfThreadLocalsThreshold;
+ private Reference<?>[] copyOfInheritableThreadLocals;
+ private Integer copyOfInheritableThreadLocalsSize;
+ private Integer copyOfInheritableThreadLocalsThreshold;
+
+ 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);
+
+ System.out.println(
+ "saveOldThreadLocals: Found " + copyOfThreadLocalsSize + "
thread locals bound to thread " + Thread.currentThread());
}
private static Reference<?>[] copy(Field field) {
@@ -216,15 +243,14 @@ public class ThreadLocalCleaner {
}
}
- 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 @@ public class ThreadPoolExecutorCleaningThreadLocals extends
ThreadPoolExecutor {
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 @@ public class ThreadPoolExecutorCleaningThreadLocals extends
ThreadPoolExecutor {
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.ExecutionException;
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 @@ public class ThreadPoolExecutorCleaningThreadLocalsTest {
1, 1, 100, TimeUnit.MILLISECONDS,
queue, Executors.defaultThreadFactory(), rejectionHandler,
listener);
}
-
+
@Test(timeout = 10000)
public void threadLocalCleanupWorksWithResize() throws Exception {
@@ -82,6 +81,8 @@ public class ThreadPoolExecutorCleaningThreadLocalsTest {
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());
}
--
To stop receiving notification emails like this one, please contact
[email protected].
