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

Always set parent of agent class loader to bootstrap class loader #5169

Merged
merged 2 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ public class AgentClassLoader extends URLClassLoader {
*
* @param javaagentFile Used for resource lookups.
* @param internalJarFileName File name of the internal jar
* @param parent Classloader parent. Should null (bootstrap), or the platform classloader for java
*/
public AgentClassLoader(File javaagentFile, String internalJarFileName, ClassLoader parent) {
super(new URL[] {}, parent);
public AgentClassLoader(File javaagentFile, String internalJarFileName) {
super(new URL[] {}, null);
if (javaagentFile == null) {
throw new IllegalArgumentException("Agent jar location should be set");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import java.io.File;
import java.lang.instrument.Instrumentation;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -100,31 +98,8 @@ public static ClassLoader getExtensionsClassLoader() {
* classloader
* @return Agent Classloader
*/
private static ClassLoader createAgentClassLoader(String innerJarFilename, File javaagentFile)
throws Exception {
ClassLoader agentParent;
if (isJavaBefore9()) {
agentParent = null; // bootstrap
} else {
// platform classloader is parent of system in java 9+
agentParent = getPlatformClassLoader();
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why we were doing this before?

Copy link
Member

Choose a reason for hiding this comment

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

this was inherited, my best guess is for access to java.sql package for some reason(?)

}

return new AgentClassLoader(javaagentFile, innerJarFilename, agentParent);
}

private static ClassLoader getPlatformClassLoader()
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
/*
Must invoke ClassLoader.getPlatformClassLoader by reflection to remain
compatible with java 8.
*/
Method method = ClassLoader.class.getDeclaredMethod("getPlatformClassLoader");
return (ClassLoader) method.invoke(null);
}

public static boolean isJavaBefore9() {
return System.getProperty("java.version").startsWith("1.");
private static ClassLoader createAgentClassLoader(String innerJarFilename, File javaagentFile) {
return new AgentClassLoader(javaagentFile, innerJarFilename);
}

private static AgentStarter createAgentStarter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AgentClassLoaderTest extends Specification {
def className2 = 'some/class/Name2'
// any jar would do, use opentelemety sdk
URL testJarLocation = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "", null)
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
Phaser threadHoldLockPhase = new Phaser(2)
Phaser acquireLockFromMainThreadPhase = new Phaser(2)

Expand Down Expand Up @@ -58,7 +58,7 @@ class AgentClassLoaderTest extends Specification {
boolean jdk8 = "1.8" == System.getProperty("java.specification.version")
// sdk is a multi release jar
URL multiReleaseJar = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI()), "", null) {
AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI()), "") {
@Override
protected String getClassSuffix() {
return ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class UnsafeTest extends Specification {
setup:
ByteBuddyAgent.install()
URL testJarLocation = AgentClassLoader.getProtectionDomain().getCodeSource().getLocation()
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "", null)
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
UnsafeInitializer.initialize(ByteBuddyAgent.getInstrumentation(), loader, false)

expect:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package io.opentelemetry.javaagent.tooling;

import static io.opentelemetry.javaagent.bootstrap.AgentInitializer.isJavaBefore9;
import static io.opentelemetry.javaagent.tooling.OpenTelemetryInstaller.installOpenTelemetrySdk;
import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.load;
import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered;
Expand Down Expand Up @@ -259,6 +258,10 @@ && isAppUsingCustomLogManager()) {
}
}

private static boolean isJavaBefore9() {
return System.getProperty("java.version").startsWith("1.");
}

private static void addByteBuddyRawSetting() {
String savedPropertyValue = System.getProperty(TypeDefinition.RAW_TYPES_PROPERTY);
try {
Expand Down