Fix J9 Module (#2341)

* create agent boostrap module

* remove previous dispatcher + setter

* use new bootstrap module

* simplify test

* use single version of antrun plugin

* update comment in OpenJ9 tests

* fix java7 compatibility

* fix minor details

* update changelog

* add comments to clarify bootstrap class inj.

* check-in packaged resources + more doc comments

CHANGELOG.asciidoc

@@ -32,6 +32,7 @@ endif::[]
 
 [float]
 ===== Bug fixes
+* Fix module loading errors on J9 JVM - {pull}2341[#2341]
 
 [[release-notes-1.x]]
 === Java Agent version 1.x

apm-agent-bootstrap/pom.xml

@@ -0,0 +1,138 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <artifactId>apm-agent-parent</artifactId>
+        <groupId>co.elastic.apm</groupId>
+        <version>1.28.2-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>apm-agent-bootstrap</artifactId>
+    <name>${project.groupId}:${project.artifactId}</name>
+
+    <properties>
+        <maven-deploy-plugin.skip>true</maven-deploy-plugin.skip>
+
+        <apm-agent-parent.base.dir>${project.basedir}/..</apm-agent-parent.base.dir>
+
+        <!-- not supported/relevant beyond java9, see https://github.com/mojohaus/animal-sniffer/issues/62
+        now enforced through the maven.compiler.release property -->
+        <animal.sniffer.skip>true</animal.sniffer.skip>
+
+        <tmpdir>${project.basedir}/target/tmp</tmpdir>
+    </properties>
+
+    <build>
+        <plugins>
+            <plugin>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>compile-java7</id>
+                        <goals>
+                            <goal>compile</goal>
+                        </goals>
+                        <configuration>
+                            <includes>
+                                <include>bootstrap/dispatcher/**/*.java</include>
+                            </includes>
+                            <excludes>
+                                <exclude>bootstrap/modulesetter</exclude>
+                            </excludes>
+                        </configuration>
+                    </execution>
+                    <execution>
+                        <id>compile-java9</id>
+                        <goals>
+                            <goal>compile</goal>
+                        </goals>
+                        <configuration>
+                            <target>9</target>
+                            <release>9</release>
+                            <includes>
+                                <include>bootstrap/modulesetter/**/*.java</include>
+                            </includes>
+                            <excludes>
+                                <exclude>bootstrap/dispatcher</exclude>
+                            </excludes>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <artifactId>maven-shade-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>shade-classes</id>
+                        <phase>package</phase>
+                        <goals>
+                            <goal>shade</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <relocations>
+                        <relocation>
+                            <!-- dispatcher needs to be in 'java.lang' to be included in 'java.base' module on java 9+ -->
+                            <pattern>bootstrap.dispatcher</pattern>
+                            <shadedPattern>java.lang</shadedPattern>
+                        </relocation>
+                        <relocation>
+                            <!-- module setter is only required for J9 JVM and MUST NOT be in 'java.base' module
+                            on java 9+ as it relies on Unsafe, which is not accessible from 'java.base' but is available
+                            to unnamed modules by default -->
+                            <pattern>bootstrap.modulesetter</pattern>
+                            <shadedPattern>co.elastic.apm.agent.modulesetter</shadedPattern>
+                        </relocation>
+                    </relocations>
+                </configuration>
+            </plugin>
+            <plugin>
+                <artifactId>maven-antrun-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>rename-classes</id>
+                        <phase>package</phase>
+                        <goals>
+                            <goal>run</goal>
+                        </goals>
+                        <configuration>
+                            <target name="move-and-rename" description="Move and Rename">
+                                <!-- After relocation, we still have to move those classes to ensure they can't interfere
+                                with regular classloading. Their content will be injected as resources into the bootstrap
+                                classloader -->
+
+                                <mkdir dir="${tmpdir}"/>
+                                <unzip dest="${tmpdir}" src="${project.basedir}/target/${project.build.finalName}.jar"/>
+                                <mkdir dir="${tmpdir}/bootstrap"/>
+                                <move todir="${tmpdir}/bootstrap">
+                                    <fileset dir="${tmpdir}" includes="**/*.class"/>
+                                    <mapper type="regexp" from="^(.*)\.class$$" to="\1\.esclazz"/>
+                                </move>
+                                <delete dir="${tmpdir}/java"/>
+                                <delete dir="${tmpdir}/co"/>
+                                <delete dir="${tmpdir}/META-INF"/>
+
+                                <!--
+                                classpath resources are checked-in in 'src/main/resources' to allow IDE to resolve them
+                                without relying on this build script to run. When updating/modifying them we have to run
+                                'mvn clean package' once to ensure that they are updated.
+                                -->
+                                <mkdir dir="${project.basedir}/src/main/resources"/>
+                                <copy todir="${project.basedir}/src/main/resources">
+                                    <fileset dir="${tmpdir}" includes="**/*"/>
+                                </copy>
+
+                                <zip basedir="${tmpdir}"
+                                     destfile="${project.basedir}/target/${project.build.finalName}.jar"/>
+                                <delete dir="${tmpdir}"/>
+                            </target>
+                        </configuration>
+                    </execution>
+
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+</project>

apm-agent-core/src/main/resources/bootstrap/IndyBootstrapDispatcher.java → apm-agent-bootstrap/src/main/java/bootstrap/dispatcher/IndyBootstrapDispatcher.java

@@ -1,3 +1,4 @@
+package bootstrap.dispatcher;
 /*
  * Licensed to Elasticsearch B.V. under one or more contributor
  * license agreements. See the NOTICE file distributed with
@@ -16,7 +17,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package java.lang;
 
 import java.lang.invoke.CallSite;
 import java.lang.invoke.ConstantCallSite;
@@ -26,7 +26,18 @@
 import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 
+/**
+ * Indy bootstrap dispatcher
+ * <p>
+ * IMPORTANT: This class is relocated in a different package and stored as a classpath resource to be injected into bootstrap classloader.
+ * A copy of this resource is stored in 'src/main/resources' and should be updated by running 'mvn clean package' whenever
+ * this class is being modified. This has only an effect when running code/tests in the IDE as the resources are loaded
+ * from the project classpath and not the packaged artifact.
+ * </p>
+ */
+@SuppressWarnings("unused")
 public class IndyBootstrapDispatcher {
+
     public static Method bootstrap;
 
     private static final MethodHandle VOID_NOOP;

apm-indy-bootstrap-module/src/main/java/co/elastic/apm/agent/bci/IndyBootstrapDispatcherModuleSetter.java → apm-agent-bootstrap/src/main/java/bootstrap/modulesetter/ModuleSetter.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package co.elastic.apm.agent.bci;
+package bootstrap.modulesetter;
 
 import sun.misc.Unsafe;
 
@@ -27,16 +27,25 @@
  * <p>
  * NOTE: This class is compiled with Java 9 so it must only be loaded though reflection and only when running on Java 9.
  * In addition, since it relies on the {@link Unsafe} API, it must be loaded by the bootstrap or platform class loaders.
+ * </p>
+ * <p>
+ * IMPORTANT: This class is relocated in a different package and stored as a classpath resource to be injected into bootstrap classloader.
+ * A copy of this resource is stored in 'src/main/resources' and should be updated by running 'mvn clean package' whenever
+ * this class is being modified. This has only an effect when running code/tests in the IDE as the resources are loaded
+ * from the project classpath and not the packaged artifact.
+ * </p>
  */
+
 @SuppressWarnings("unused")
-public class IndyBootstrapDispatcherModuleSetter {
+public class ModuleSetter {
+
+    public static void setJavaBaseModule(Class<?> targetClass) throws Exception {
 
-    public static void setJavaBaseModule(Class<?> indyBootstrapDispatcherClass) throws Exception {
         Field moduleField = Class.class.getDeclaredField("module");
         if (moduleField.getType() == Module.class) {
             Module javaBaseModule = Class.class.getModule();
             Unsafe unsafe = Unsafe.getUnsafe();
-            unsafe.putObject(indyBootstrapDispatcherClass, unsafe.objectFieldOffset(moduleField), javaBaseModule);
+            unsafe.putObject(targetClass, unsafe.objectFieldOffset(moduleField), javaBaseModule);
         } else {
             throw new IllegalStateException("Unexpected module field type: " + moduleField.getType().getName());
         }

apm-agent-core/pom.xml

@@ -93,10 +93,11 @@
 
         <dependency>
             <groupId>${project.groupId}</groupId>
-            <artifactId>apm-indy-bootstrap-module</artifactId>
+            <artifactId>apm-agent-bootstrap</artifactId>
             <version>${project.version}</version>
-            <scope>test</scope>
+            <scope>runtime</scope>
         </dependency>
+
         <dependency>
             <groupId>com.squareup.okhttp3</groupId>
             <artifactId>okhttp</artifactId>

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java

@@ -168,19 +168,28 @@
 public class IndyBootstrap {
 
     /**
-     * Starts with {@code java.lang} so that OSGi class loaders don't restrict access to it
+     * Starts with {@code java.lang} so that OSGi class loaders don't restrict access to it.
+     * This also allows to load it in {@code java.base} module on Java9+ for Hotspot, Open J9 requires {@code ModuleSetter}
      */
     private static final String INDY_BOOTSTRAP_CLASS_NAME = "java.lang.IndyBootstrapDispatcher";
+
+    /**
+     * The class file of {@code IndyBootstrapDispatcher}, loaded from classpath resource, {@code esclazz} extension avoids
+     * being loaded as a regular class.
+     */
+    private static final String INDY_BOOTSTRAP_RESOURCE = "bootstrap/java/lang/IndyBootstrapDispatcher.esclazz";
+
     /**
      * Needs to be loaded from the bootstrap CL because it uses {@code sun.misc.Unsafe}.
      * In addition, needs to be loaded explicitly by name only when running on Java 9, because compiled with Java 9
      */
-    private static final String INDY_BOOTSTRAP_MODULE_SETTER_CLASS_NAME = "co.elastic.apm.agent.bci.IndyBootstrapDispatcherModuleSetter";
+    private static final String INDY_BOOTSTRAP_MODULE_SETTER_CLASS_NAME = "co.elastic.apm.agent.modulesetter.ModuleSetter";
+
     /**
-     * The class file of {@code java.lang.IndyBootstrapDispatcher}.
-     * Ends with {@code clazz} because if it ended with {@code class}, it would be loaded like a regular class.
+     * The class file of {@code ModuleSetter}, loaded from classpath resource, {@code esclazz} extension avoids being
+     * loaded as a regular class.
      */
-    private static final String INDY_BOOTSTRAP_RESOURCE = "bootstrap/IndyBootstrapDispatcher.clazz";
+    private static final String INDY_BOOTSTRAP_MODULE_SETTER_RESOURCE = "bootstrap/co/elastic/apm/agent/modulesetter/ModuleSetter.esclazz";
 
     /**
      * The name of the class we use as the lookup class during the invokedynamic bootstrap flow. The bytecode of this
@@ -220,17 +229,7 @@ public static Method getIndyBootstrapMethod(final Logger logger) {
      * Injects the {@code java.lang.IndyBootstrapDispatcher} class into the bootstrap class loader if it wasn't already.
      */
     private static Class<?> initIndyBootstrap(final Logger logger) throws Exception {
-        Class<?> indyBootstrapDispatcherClass;
-        try {
-            indyBootstrapDispatcherClass = Class.forName(INDY_BOOTSTRAP_CLASS_NAME, false, null);
-        } catch (ClassNotFoundException e) {
-            byte[] bootstrapClass = IOUtils.readToBytes(ElasticApmAgent.getAgentClassLoader().getResourceAsStream(INDY_BOOTSTRAP_RESOURCE));
-            if (bootstrapClass == null || bootstrapClass.length == 0) {
-                throw new IllegalStateException("Could not locate " + INDY_BOOTSTRAP_RESOURCE);
-            }
-            ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(Collections.singletonMap(INDY_BOOTSTRAP_CLASS_NAME, bootstrapClass));
-            indyBootstrapDispatcherClass = Class.forName(INDY_BOOTSTRAP_CLASS_NAME, false, null);
-        }
+        Class<?> indyBootstrapDispatcherClass = loadClassInBootstrap(INDY_BOOTSTRAP_CLASS_NAME, INDY_BOOTSTRAP_RESOURCE);
 
         if (JvmRuntimeInfo.ofCurrentVM().getMajorVersion() >= 9 && JvmRuntimeInfo.ofCurrentVM().isJ9VM()) {
             try {
@@ -243,6 +242,42 @@ private static Class<?> initIndyBootstrap(final Logger logger) throws Exception
         return indyBootstrapDispatcherClass;
     }
 
+    /**
+     * Loads a class from classpath resource in bootstrap classloader.
+     * <p>
+     * Ensuring that classes loaded through this method can ONLY be loaded in the bootstrap CL requires the following:
+     * <ul>
+     *     <li>The class bytecode resource name should not end with the {@code .class} suffix</li>
+     *     <li>The class bytecode resource name should be in a location that reflects its package</li>
+     *     <li>For tests in IDE, the class name used here should be distinct from its original class name to ensure
+     *     that only the relocated resource is being used</li>
+     * </ul>
+     *
+     * @param className    class name
+     * @param resourceName class resource name
+     * @return class loaded in bootstrap classloader
+     * @throws IOException            if unable to open provided resource
+     * @throws ClassNotFoundException if unable to load class in bootstrap CL
+     */
+    private static Class<?> loadClassInBootstrap(String className, String resourceName) throws IOException, ClassNotFoundException {
+        Class<?> bootstrapClass;
+        try {
+            // Will return non-null value only if the class has already been loaded.
+            // Ensuring that a class can ONLY be loaded through this method and not from regular classloading relies
+            // on applying the listed instructions in method documentation
+            bootstrapClass = Class.forName(className, false, null);
+        } catch (ClassNotFoundException e) {
+            byte[] classBytes = IOUtils.readToBytes(ElasticApmAgent.getAgentClassLoader().getResourceAsStream(resourceName));
+            if (classBytes == null || classBytes.length == 0) {
+                throw new IllegalStateException("Could not locate " + resourceName);
+            }
+            ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(Collections.singletonMap(className, classBytes));
+            bootstrapClass = Class.forName(className, false, null);
+        }
+        return bootstrapClass;
+    }
+
+
     /**
      * A package-private method for unit-testing of the module overriding functionality
      *
@@ -252,7 +287,8 @@ private static Class<?> initIndyBootstrap(final Logger logger) throws Exception
     static void setJavaBaseModule(Class<?> targetClass) throws Throwable {
         // In order to override the original unnamed module assigned to the IndyBootstrapDispatcher, we rely on the
         // Unsafe API, which requires the caller to be loaded by the Bootstrap CL
-        Class<?> moduleSetterClass = Class.forName(INDY_BOOTSTRAP_MODULE_SETTER_CLASS_NAME, false, null);
+
+        Class<?> moduleSetterClass = loadClassInBootstrap(INDY_BOOTSTRAP_MODULE_SETTER_CLASS_NAME, INDY_BOOTSTRAP_MODULE_SETTER_RESOURCE);
         MethodHandles.lookup()
             .findStatic(moduleSetterClass, "setJavaBaseModule", MethodType.methodType(void.class, Class.class))
             .invoke(targetClass);

apm-agent-core/src/test/java/co/elastic/apm/agent/bci/IndyBootstrapTest.java

@@ -19,12 +19,7 @@
 package co.elastic.apm.agent.bci;
 
 import co.elastic.apm.agent.AbstractInstrumentationTest;
-import net.bytebuddy.dynamic.loading.ClassInjector;
 import org.junit.jupiter.api.Test;
-import org.stagemonitor.util.IOUtils;
-
-import java.io.InputStream;
-import java.util.Collections;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -35,12 +30,6 @@ void testSetJavaBaseModule() throws Throwable {
         Module javaBaseModule = Class.class.getModule();
         assertThat(IndyBootstrapTest.class.getModule()).isNotEqualTo(javaBaseModule);
 
-        // In order to test this functionality, IndyBootstrapDispatcherModuleSetter needs to be loaded from the Boot CL.
-        // We don't mind loading it with the test's class loader as well only to get it's class file
-        InputStream classFileAsStream = IndyBootstrapDispatcherModuleSetter.class.getResourceAsStream("IndyBootstrapDispatcherModuleSetter.class");
-        byte[] bootstrapClass = IOUtils.readToBytes(classFileAsStream);
-        ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(Collections.singletonMap(IndyBootstrapDispatcherModuleSetter.class.getName(), bootstrapClass));
-
         IndyBootstrap.setJavaBaseModule(IndyBootstrapTest.class);
         assertThat(IndyBootstrapTest.class.getModule()).isEqualTo(javaBaseModule);
     }

apm-agent/pom.xml

@@ -36,7 +36,7 @@
         </dependency>
         <dependency>
             <groupId>${project.groupId}</groupId>
-            <artifactId>apm-indy-bootstrap-module</artifactId>
+            <artifactId>apm-agent-bootstrap</artifactId>
             <version>${project.version}</version>
         </dependency>