You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2018/01/24 16:03:35 UTC
[sling-org-apache-sling-commons-threads] branch bugfix/SLING-7432
updated (6020992 -> 0e0babe)
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a change to branch bugfix/SLING-7432
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git.
discard 6020992 SLING-7432 no longer rely on thread locals themselves to store old thread local state
new 0e0babe SLING-7432 no longer rely on thread locals themselves to store old thread local state
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (6020992)
\
N -- N -- N refs/heads/bugfix/SLING-7432 (0e0babe)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
--
To stop receiving notification emails like this one, please contact
kwin@apache.org.
[sling-org-apache-sling-commons-threads] 01/01: SLING-7432 no
longer rely on thread locals themselves to store old thread local state
Posted by kw...@apache.org.
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
commit 0e0babe582755402fcc3464edc8cc4cd90a0ae30
Author: Konrad Windszus <kw...@apache.org>
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
kwin@apache.org.