[GEODE-10519] Security : Remove Unsafe Reflection Breaking Java Module System Encapsulation

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",

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.
  *
+ * <p>
+ * 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.
+ * </p>
  *
+ * <p>
+ * The implementation uses WeakHashMap with Thread keys to ensure threads can be garbage collected
+ * when they terminate, preventing memory leaks.
+ * </p>
  */
 public class UnsafeThreadLocal<T> extends ThreadLocal<T> {
   /**
-   * 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<Thread, T> 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.
+   *
+   * <p>
+   * Unlike get(), this method does not set the initial value if none is found. Returns null if the
+   * specified thread has no value set.
+   * </p>
+   *
+   * @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);
   }
 
 }

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<String> 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<String> getMemberJvmOptions() {
     if (isJavaVersionAtLeast(JAVA_11)) {