From a26e4c0e06107f290e573f51ef46f374cea8f826 Mon Sep 17 00:00:00 2001
From: Akshat Jain <akjn11@gmail.com>
Date: Fri, 13 Dec 2024 19:54:21 +0530
Subject: [PATCH] Cleanup unreachable Java 8 code flows (#17559)

---
 .../indexing/common/task/HadoopTask.java      | 11 +--
 .../indexing/overlord/ForkingTaskRunner.java  |  4 +-
 .../indexing/common/task/HadoopTaskTest.java  | 10 +-
 pom.xml                                       |  3 +
 .../java/util/common/ByteBufferUtils.java     | 59 ++---------
 .../druid/java/util/common/Cleaners.java      | 23 +----
 .../java/util/common/DefineClassUtils.java    | 99 +------------------
 .../java/org/apache/druid/utils/JvmUtils.java |  5 -
 .../org/apache/druid/utils/RuntimeInfo.java   |  2 +-
 9 files changed, 18 insertions(+), 198 deletions(-)

diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
index 201024540e16..a15b77b90d18 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
@@ -173,16 +173,7 @@ public static ClassLoader buildClassLoader(final List<String> hadoopDependencyCo
       localClassLoaderURLs.addAll(Arrays.asList(hadoopLoader.getURLs()));
     }
 
-    ClassLoader parent = null;
-    if (JvmUtils.isIsJava9Compatible()) {
-      try {
-        // See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
-        parent = (ClassLoader) ClassLoader.class.getMethod("getPlatformClassLoader").invoke(null);
-      }
-      catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
-        throw new RuntimeException(e);
-      }
-    }
+    ClassLoader parent = ClassLoader.getPlatformClassLoader();
     final ClassLoader classLoader = new URLClassLoader(
         localClassLoaderURLs.toArray(new URL[0]),
         parent
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
index 7e4dd6c39c22..1a34c76e33fb 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
@@ -244,9 +244,7 @@ public TaskStatus call()
 
                         command.add(config.getJavaCommand());
 
-                        if (JvmUtils.majorVersion() >= 11) {
-                          command.addAll(STRONG_ENCAPSULATION_PROPERTIES);
-                        }
+                        command.addAll(STRONG_ENCAPSULATION_PROPERTIES);
 
                         command.add("-cp");
                         command.add(taskClasspath);
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
index 130258e85d4c..741208dd0db4 100644
--- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
+++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
@@ -30,7 +30,6 @@
 import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.java.util.common.granularity.Granularity;
 import org.apache.druid.timeline.DataSegment;
-import org.apache.druid.utils.JvmUtils;
 import org.easymock.EasyMock;
 import org.joda.time.Interval;
 import org.junit.Assert;
@@ -131,13 +130,8 @@ public TaskStatus runTask(TaskToolbox toolbox)
 
   public static void assertClassLoaderIsSingular(ClassLoader classLoader)
   {
-    if (JvmUtils.isIsJava9Compatible()) {
-      // See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
-      Assert.assertEquals("PlatformClassLoader", classLoader.getParent().getClass().getSimpleName());
-    } else {
-      // This is a check against the current HadoopTask which creates a single URLClassLoader with null parent
-      Assert.assertNull(classLoader.getParent());
-    }
+    // See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
+    Assert.assertEquals("PlatformClassLoader", classLoader.getParent().getClass().getSimpleName());
     Assert.assertFalse(classLoader.getClass().getSimpleName().equals("ApplicationClassLoader"));
     Assert.assertTrue(classLoader instanceof URLClassLoader);
 
diff --git a/pom.xml b/pom.xml
index 3ee7600d5437..3f80196334aa 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1666,6 +1666,9 @@
                                 <ignore>sun.misc.Unsafe</ignore>
                                 <!-- ignore java reflection polymorphic api signatures -->
                                 <ignore>java.lang.invoke.MethodHandle</ignore>
+                                <!-- For ignoring java.lang.ClassLoader#getPlatformClassLoader since that's present in java 9+.
+                                Need to be added since animal sniffer is using JDK8 signature currently. -->
+                                <ignore>java.lang.ClassLoader</ignore>
                                 <!--
                                  For the following java.nio.* classes, we get errors like: "Undefined reference: java.nio.ByteBuffer java.nio.ByteBuffer.clear()"
                                  GitHub issue: https://github.com/mojohaus/animal-sniffer/issues/4
diff --git a/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
index fb67c3238347..696f0b629c9c 100644
--- a/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
+++ b/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
@@ -20,17 +20,14 @@
 package org.apache.druid.java.util.common;
 
 import org.apache.druid.collections.ResourceHolder;
-import org.apache.druid.utils.JvmUtils;
 
 import javax.annotation.Nullable;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
-import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import java.nio.MappedByteBuffer;
 import java.util.Comparator;
-import java.util.Objects;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -88,11 +85,12 @@ private static MethodHandle lookupUnmapMethodHandle()
   {
     final MethodHandles.Lookup lookup = MethodHandles.lookup();
     try {
-      if (JvmUtils.isIsJava9Compatible()) {
-        return unmapJava9(lookup);
-      } else {
-        return unmapJava7Or8(lookup);
-      }
+      MethodHandle unmapper = lookup.findVirtual(
+          UnsafeUtils.theUnsafeClass(),
+          "invokeCleaner",
+          MethodType.methodType(void.class, ByteBuffer.class)
+      );
+      return unmapper.bindTo(UnsafeUtils.theUnsafe());
     }
     catch (ReflectiveOperationException | RuntimeException e1) {
       throw new UnsupportedOperationException("Unmapping is not supported on this platform, because internal " +
@@ -100,51 +98,6 @@ private static MethodHandle lookupUnmapMethodHandle()
     }
   }
 
-  /**
-   * NB: while Druid no longer support Java 7, this method would still work with that version as well.
-   */
-  private static MethodHandle unmapJava7Or8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
-  {
-    // "Compile" a MethodHandle that is roughly equivalent to the following lambda:
-    //
-    // (ByteBuffer buffer) -> {
-    //   sun.misc.Cleaner cleaner = ((java.nio.DirectByteBuffer) byteBuffer).cleaner();
-    //   if (nonNull(cleaner))
-    //     cleaner.clean();
-    //   else
-    //     noop(cleaner); // the noop is needed because MethodHandles#guardWithTest always needs both if and else
-    // }
-    //
-    Class<?> directBufferClass = Class.forName("java.nio.DirectByteBuffer");
-    Method m = directBufferClass.getMethod("cleaner");
-    m.setAccessible(true);
-    MethodHandle directBufferCleanerMethod = lookup.unreflect(m);
-    Class<?> cleanerClass = directBufferCleanerMethod.type().returnType();
-    MethodHandle cleanMethod = lookup.findVirtual(cleanerClass, "clean", MethodType.methodType(void.class));
-    MethodHandle nonNullTest = lookup.findStatic(Objects.class, "nonNull",
-                                                 MethodType.methodType(boolean.class, Object.class)
-    ).asType(MethodType.methodType(boolean.class, cleanerClass));
-    MethodHandle noop = MethodHandles.dropArguments(MethodHandles.constant(
-        Void.class,
-        null
-    ).asType(MethodType.methodType(void.class)), 0, cleanerClass);
-    MethodHandle unmapper = MethodHandles.filterReturnValue(
-        directBufferCleanerMethod,
-        MethodHandles.guardWithTest(nonNullTest, cleanMethod, noop)
-    ).asType(MethodType.methodType(void.class, ByteBuffer.class));
-    return unmapper;
-  }
-
-  private static MethodHandle unmapJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
-  {
-    MethodHandle unmapper = lookup.findVirtual(
-        UnsafeUtils.theUnsafeClass(),
-        "invokeCleaner",
-        MethodType.methodType(void.class, ByteBuffer.class)
-    );
-    return unmapper.bindTo(UnsafeUtils.theUnsafe());
-  }
-
   /**
    * Same as {@link ByteBuffer#allocateDirect(int)}, but returns a closeable {@link ResourceHolder} that
    * frees the buffer upon close.
diff --git a/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java b/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
index 2de6a3e140db..2fa4ebdfafbb 100644
--- a/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
+++ b/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
@@ -19,8 +19,6 @@
 
 package org.apache.druid.java.util.common;
 
-import org.apache.druid.utils.JvmUtils;
-
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
@@ -62,11 +60,7 @@ private static Cleaner takeMeToTheCleaners()
   {
     final MethodHandles.Lookup lookup = MethodHandles.lookup();
     try {
-      if (JvmUtils.isIsJava9Compatible()) {
-        return lookupCleanerJava9(lookup);
-      } else {
-        return lookupCleanerJava8(lookup);
-      }
+      return lookupCleaner(lookup);
     }
     catch (ReflectiveOperationException | RuntimeException e) {
       throw new UnsupportedOperationException("Cleaning is not support on this platform, because internal " +
@@ -74,7 +68,7 @@ private static Cleaner takeMeToTheCleaners()
     }
   }
 
-  private static Cleaner lookupCleanerJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
+  private static Cleaner lookupCleaner(MethodHandles.Lookup lookup) throws ReflectiveOperationException
   {
     Class<?> cleaner = Class.forName("java.lang.ref.Cleaner");
     Class<?> cleanable = Class.forName("java.lang.ref.Cleaner$Cleanable");
@@ -100,19 +94,6 @@ private static Cleaner lookupCleanerJava9(MethodHandles.Lookup lookup) throws Re
     return new CleanerImpl(register, clean);
   }
 
-  private static Cleaner lookupCleanerJava8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
-  {
-    Class<?> cleaner = Class.forName("sun.misc.Cleaner");
-    MethodHandle register = lookup.findStatic(
-        cleaner,
-        "create",
-        MethodType.methodType(cleaner, Object.class, Runnable.class)
-    );
-
-    MethodHandle clean = lookup.findVirtual(cleaner, "clean", MethodType.methodType(void.class));
-    return new CleanerImpl(register, clean);
-  }
-
   public static Cleanable register(Object object, Runnable runnable)
   {
     if (CLEANER == null) {
diff --git a/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
index d79c517ae3d1..5e3bd6626f7d 100644
--- a/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
+++ b/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
@@ -19,12 +19,9 @@
 
 package org.apache.druid.java.util.common;
 
-import org.apache.druid.utils.JvmUtils;
-
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
-import java.security.ProtectionDomain;
 
 /**
  * This utility class provides a thin runtime abstraction to pick between
@@ -44,11 +41,7 @@ public class DefineClassUtils
     Exception exception = null;
     try {
       MethodHandles.Lookup lookup = MethodHandles.lookup();
-      if (JvmUtils.isIsJava9Compatible()) {
-        defineClass = defineClassJava9(lookup);
-      } else {
-        defineClass = defineClassJava8(lookup);
-      }
+      defineClass = getMethodHandle(lookup);
     }
     catch (ReflectiveOperationException | RuntimeException e) {
       exception = e;
@@ -70,9 +63,8 @@ public class DefineClassUtils
    *    return targetClassLookup.defineClass(byteCode);
    *  }
    */
-  private static MethodHandle defineClassJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
+  private static MethodHandle getMethodHandle(MethodHandles.Lookup lookup) throws ReflectiveOperationException
   {
-
     // this is getting meta
     MethodHandle defineClass = lookup.unreflect(MethodHandles.Lookup.class.getMethod("defineClass", byte[].class));
     MethodHandle privateLookupIn = lookup.findStatic(
@@ -94,93 +86,6 @@ private static MethodHandle defineClassJava9(MethodHandles.Lookup lookup) throws
     return defineClass;
   }
 
-  /**
-   * "Compile" a MethodHandle that is equivalent to:
-   *
-   *  Class<?> defineClass(Class targetClass, byte[] byteCode, String className) {
-   *    return Unsafe.defineClass(
-   *        className,
-   *        byteCode,
-   *        0,
-   *        byteCode.length,
-   *        targetClass.getClassLoader(),
-   *        targetClass.getProtectionDomain()
-   *    );
-   *  }
-   */
-  private static MethodHandle defineClassJava8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
-  {
-    MethodHandle defineClass = lookup.findVirtual(
-        UnsafeUtils.theUnsafeClass(),
-        "defineClass",
-        MethodType.methodType(
-            Class.class,
-            String.class,
-            byte[].class,
-            int.class,
-            int.class,
-            ClassLoader.class,
-            ProtectionDomain.class
-        )
-    ).bindTo(UnsafeUtils.theUnsafe());
-
-    MethodHandle getProtectionDomain = lookup.unreflect(Class.class.getMethod("getProtectionDomain"));
-    MethodHandle getClassLoader = lookup.unreflect(Class.class.getMethod("getClassLoader"));
-
-    // apply getProtectionDomain and getClassLoader to the targetClass, modifying the methodHandle as follows:
-    // defineClass = (String className, byte[] byteCode, int offset, int length, Class class1, Class class2) ->
-    //   defineClass(className, byteCode, offset, length, class1.getClassLoader(), class2.getProtectionDomain())
-    defineClass = MethodHandles.filterArguments(defineClass, 5, getProtectionDomain);
-    defineClass = MethodHandles.filterArguments(defineClass, 4, getClassLoader);
-
-    // duplicate the last argument to apply the methods above to the same class, modifying the methodHandle as follows:
-    // defineClass = (String className, byte[] byteCode, int offset, int length, Class targetClass) ->
-    //   defineClass(className, byteCode, offset, length, targetClass, targetClass)
-    defineClass = MethodHandles.permuteArguments(
-        defineClass,
-        MethodType.methodType(Class.class, String.class, byte[].class, int.class, int.class, Class.class),
-        0, 1, 2, 3, 4, 4
-    );
-
-    // set offset argument to 0, modifying the methodHandle as follows:
-    // defineClass = (String className, byte[] byteCode, int length, Class targetClass) ->
-    //   defineClass(className, byteCode, 0, length, targetClass)
-    defineClass = MethodHandles.insertArguments(defineClass, 2, (int) 0);
-
-    // JDK8 does not implement MethodHandles.arrayLength, so we have to roll our own
-    MethodHandle arrayLength = lookup.findStatic(
-        lookup.lookupClass(),
-        "getArrayLength",
-        MethodType.methodType(int.class, byte[].class)
-    );
-
-    // apply arrayLength to the length argument, modifying the methodHandle as follows:
-    // defineClass = (String className, byte[] byteCode1, byte[] byteCode2, Class targetClass) ->
-    //   defineClass(className, byteCode1, byteCode2.length, targetClass)
-    defineClass = MethodHandles.filterArguments(defineClass, 2, arrayLength);
-
-    // duplicate the byteCode argument and reorder to match JDK9 signature, modifying the methodHandle as follows:
-    // defineClass = (Class targetClass, byte[] byteCode, String className) ->
-    //   defineClass(className, byteCode, byteCode, targetClass)
-    defineClass = MethodHandles.permuteArguments(
-        defineClass,
-        MethodType.methodType(Class.class, Class.class, byte[].class, String.class),
-        2, 1, 1, 0
-    );
-
-    return defineClass;
-  }
-
-  /**
-   * This method is referenced in Java 8 using method handle, therefore it is not actually unused, and shouldn't be
-   * removed (till Java 8 is supported)
-   */
-  @SuppressWarnings("unused") // method is referenced and used in defineClassJava8
-  static int getArrayLength(byte[] bytes)
-  {
-    return bytes.length;
-  }
-
   public static Class defineClass(
       Class<?> targetClass,
       byte[] byteCode,
diff --git a/processing/src/main/java/org/apache/druid/utils/JvmUtils.java b/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
index 493ac901420d..696a15aaee28 100644
--- a/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
+++ b/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
@@ -71,11 +71,6 @@ public static int majorVersion()
     return MAJOR_VERSION;
   }
 
-  public static boolean isIsJava9Compatible()
-  {
-    return MAJOR_VERSION >= 9;
-  }
-
   public static RuntimeInfo getRuntimeInfo()
   {
     return RUNTIME_INFO;
diff --git a/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java b/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
index 86571eff9456..115787cfd666 100644
--- a/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
+++ b/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
@@ -50,7 +50,7 @@ public long getFreeHeapSizeBytes()
   public long getDirectMemorySizeBytes()
   {
     try {
-      Class<?> vmClass = Class.forName(JvmUtils.majorVersion() >= 9 ? "jdk.internal.misc.VM" : "sun.misc.VM");
+      Class<?> vmClass = Class.forName("jdk.internal.misc.VM");
       Object maxDirectMemoryObj = vmClass.getMethod("maxDirectMemory").invoke(null);
 
       if (maxDirectMemoryObj == null || !(maxDirectMemoryObj instanceof Number)) {