Skip to content

Commit

Permalink
SLING-7432 - Thread pool clean up code can lead to infinite loops in
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rombert committed Jan 23, 2018
1 parent 21a553d commit afa59d6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
Expand Up @@ -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();
Expand All @@ -65,6 +70,8 @@ private static synchronized void initReflectionFields() throws IllegalStateExcep
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. " +
Expand Down Expand Up @@ -169,12 +176,19 @@ private static Class<?> inner(Class<?> clazz, String name) {
}

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) {
Expand All @@ -190,31 +204,43 @@ private static Reference<?>[] copy(Field field) {
}
}

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!

}
}
Expand Up @@ -57,7 +57,6 @@ public void setUp() {
}

@Test(timeout = 10000)
@Ignore
public void threadLocalCleanupWorksWithResize() throws Exception {

// configure thread local counts to make sure that
Expand Down

0 comments on commit afa59d6

Please sign in to comment.