Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix J9 Module #2341

Merged
merged 12 commits into from
Dec 15, 2021
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ endif::[]

[float]
===== Bug fixes
* Fix module loading errors on J9 JVM - {pull}2341[#2341]

[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
136 changes: 136 additions & 0 deletions apm-agent-bootstrap/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?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"/>

<!--
duplicate the resources in 'target/classes' to allow the IDE to resolve them from the
project location as the packaged jar file is not used unlike with maven.
-->
<copy todir="${project.basedir}/target/classes">
<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>
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -26,7 +26,9 @@
import java.lang.reflect.Array;
import java.lang.reflect.Method;

@SuppressWarnings("unused")
public class IndyBootstrapDispatcher {

public static Method bootstrap;

private static final MethodHandle VOID_NOOP;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -29,14 +29,15 @@
* In addition, since it relies on the {@link Unsafe} API, it must be loaded by the bootstrap or platform class loaders.
*/
@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());
}
Expand Down
5 changes: 3 additions & 2 deletions apm-agent-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -243,6 +242,22 @@ private static Class<?> initIndyBootstrap(final Logger logger) throws Exception
return indyBootstrapDispatcherClass;
}

private static Class<?> loadClassInBootstrap(String className, String resourceName) throws IOException, ClassNotFoundException {
Class<?> bootstrapClass;
try {
bootstrapClass = Class.forName(className, false, null);
eyalkoren marked this conversation as resolved.
Show resolved Hide resolved
} 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
*
Expand All @@ -252,7 +267,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);
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion apm-agent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
28 changes: 0 additions & 28 deletions apm-indy-bootstrap-module/pom.xml

This file was deleted.

1 change: 0 additions & 1 deletion elastic-apm-agent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@
</plugin>
<plugin>
<artifactId>maven-antrun-plugin</artifactId>
<version>3.0.0</version>
<executions>
<execution>
<id>shade-cached-lookup-key</id>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ public static Iterable<Object[]> data() {
{"9-jre11-slim"},
{"9.0.39-jdk14-openjdk-oracle"},
{"jdk8-adoptopenjdk-openj9"},
// TODO openj9 on JDK11 has an access problem from java.base
//{"jdk11-adoptopenjdk-openj9"},
//{"9.0.50-jdk11-adoptopenjdk-openj9"}
{"jdk11-adoptopenjdk-openj9"},
{"9.0.50-jdk11-adoptopenjdk-openj9"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

});
}

Expand Down
6 changes: 5 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
<module>integration-tests</module>
<module>apm-agent-attach</module>
<module>apm-agent-plugin-sdk</module>
<module>apm-indy-bootstrap-module</module>
<module>apm-agent-bootstrap</module>
<module>apm-agent</module>
<module>apm-agent-attach-cli</module>
<module>apm-agent-common</module>
Expand Down Expand Up @@ -514,6 +514,10 @@
<artifactId>maven-site-plugin</artifactId>
<version>3.8.2</version>
</plugin>
<plugin>
<artifactId>maven-antrun-plugin</artifactId>
<version>3.0.0</version>
</plugin>
</plugins>
</pluginManagement>
<extensions>
Expand Down