diff --git a/build-tools/scripts/src/main/groovy/geode-test.gradle b/build-tools/scripts/src/main/groovy/geode-test.gradle index 93488986e512..602f0b731651 100644 --- a/build-tools/scripts/src/main/groovy/geode-test.gradle +++ b/build-tools/scripts/src/main/groovy/geode-test.gradle @@ -182,7 +182,6 @@ gradle.taskGraph.whenReady({ graph -> if (project.hasProperty('testJVMVer') && testJVMVer.toInteger() >= 9) { jvmArgs += [ "--add-opens=java.base/java.io=ALL-UNNAMED", - "--add-opens=java.base/java.lang=ALL-UNNAMED", "--add-opens=java.base/java.lang.annotation=ALL-UNNAMED", "--add-opens=java.base/java.lang.module=ALL-UNNAMED", "--add-opens=java.base/java.lang.ref=ALL-UNNAMED", diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/deadlock/UnsafeThreadLocal.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/deadlock/UnsafeThreadLocal.java index 17872c29cb65..afabb84722d2 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/deadlock/UnsafeThreadLocal.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/deadlock/UnsafeThreadLocal.java @@ -14,72 +14,68 @@ */ package org.apache.geode.distributed.internal.deadlock; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; +import java.util.Map; +import java.util.WeakHashMap; /** - * Most of this thread local is safe to use, except for the getValue(Thread) method. That is not - * guaranteed to be correct. But for our deadlock detection tool I think it's good enough, and this - * class provides a very low overhead way for us to record what thread holds a particular resource. + * A ThreadLocal implementation that allows reading values from arbitrary threads, useful for + * deadlock detection. This implementation uses a WeakHashMap to track values per thread without + * requiring reflection or JVM internal access. * + *

+ * Unlike standard ThreadLocal, this class maintains an additional mapping that allows querying the + * value for any thread, not just the current thread. This is useful for deadlock detection where + * we need to inspect what resources other threads are holding. + *

* + *

+ * The implementation uses WeakHashMap with Thread keys to ensure threads can be garbage collected + * when they terminate, preventing memory leaks. + *

*/ public class UnsafeThreadLocal extends ThreadLocal { /** - * Dangerous method. Uses reflection to extract the thread local for a given thread. - * - * Unlike get(), this method does not set the initial value if none is found - * + * Maps threads to their values. Uses WeakHashMap so terminated threads can be GC'd. Synchronized + * to ensure thread-safe access. */ - public T get(Thread thread) { - return (T) get(this, thread); - } + private final Map threadValues = + java.util.Collections.synchronizedMap(new WeakHashMap<>()); - private static Object get(ThreadLocal threadLocal, Thread thread) { - try { - Object threadLocalMap = - invokePrivate(threadLocal, "getMap", new Class[] {Thread.class}, new Object[] {thread}); - - if (threadLocalMap != null) { - Object entry = invokePrivate(threadLocalMap, "getEntry", new Class[] {ThreadLocal.class}, - new Object[] {threadLocal}); - if (entry != null) { - return getPrivate(entry, "value"); - } - } - return null; - } catch (Exception e) { - throw new RuntimeException("Unable to extract thread local", e); + /** + * Sets the value for the current thread and records it in the cross-thread map. + */ + @Override + public void set(T value) { + super.set(value); + if (value != null) { + threadValues.put(Thread.currentThread(), value); + } else { + threadValues.remove(Thread.currentThread()); } } - private static Object getPrivate(Object object, String fieldName) throws SecurityException, - NoSuchFieldException, IllegalArgumentException, IllegalAccessException { - Field field = object.getClass().getDeclaredField(fieldName); - field.setAccessible(true); - return field.get(object); + /** + * Removes the value for the current thread from both the ThreadLocal and the cross-thread map. + */ + @Override + public void remove() { + super.remove(); + threadValues.remove(Thread.currentThread()); } - private static Object invokePrivate(Object object, String methodName, Class[] argTypes, - Object[] args) throws SecurityException, NoSuchMethodException, IllegalArgumentException, - IllegalAccessException, InvocationTargetException { - - Method method = null; - Class clazz = object.getClass(); - while (method == null) { - try { - method = clazz.getDeclaredMethod(methodName, argTypes); - } catch (NoSuchMethodException e) { - clazz = clazz.getSuperclass(); - if (clazz == null) { - throw e; - } - } - } - method.setAccessible(true); - Object result = method.invoke(object, args); - return result; + /** + * Gets the value for an arbitrary thread, useful for deadlock detection. + * + *

+ * Unlike get(), this method does not set the initial value if none is found. Returns null if the + * specified thread has no value set. + *

+ * + * @param thread the thread whose value to retrieve + * @return the value for the specified thread, or null if none exists + */ + public T get(Thread thread) { + return threadValues.get(thread); } } diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java index d0fa681f47f2..4021db507d62 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java @@ -26,7 +26,7 @@ import java.util.Collections; import java.util.List; -import org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal; +import org.apache.geode.internal.offheap.AddressableMemoryManager; import org.apache.geode.internal.stats50.VMStats50; import org.apache.geode.unsafe.internal.com.sun.jmx.remote.security.MBeanServerAccessController; @@ -38,9 +38,9 @@ public class MemberJvmOptions { private static final String COM_SUN_JMX_REMOTE_SECURITY_EXPORT = "--add-exports=java.management/com.sun.jmx.remote.security=ALL-UNNAMED"; /** - * open needed by {@link UnsafeThreadLocal} + * open needed by {@link AddressableMemoryManager} */ - private static final String JAVA_LANG_OPEN = "--add-opens=java.base/java.lang=ALL-UNNAMED"; + private static final String JAVA_NIO_OPEN = "--add-opens=java.base/java.nio=ALL-UNNAMED"; /** * open needed by {@link VMStats50} */ @@ -50,7 +50,7 @@ public class MemberJvmOptions { static final List JAVA_11_OPTIONS = Arrays.asList( COM_SUN_JMX_REMOTE_SECURITY_EXPORT, COM_SUN_MANAGEMENT_INTERNAL_OPEN, - JAVA_LANG_OPEN); + JAVA_NIO_OPEN); public static List getMemberJvmOptions() { if (isJavaVersionAtLeast(JAVA_11)) {