Fix thread safety, error handling, and code clarity in agent bootstrap

[jbachorik]

Summary

Addresses code review findings in the masked agent JAR bootstrap code.

• Fix class load ordering: Append agent JAR to bootstrap classpath before loading Main, so Main's static initializers can find bootstrap classes
• Improve error handling: Surface clear error messages on init failure; throw RuntimeException in premain so the JVM aborts instead of silently continuing without BTrace
• Fix dual-parent classloader: Pass parent to super() instead of keeping a separate parent field, eliminating confusing dual delegation paths
• Simplify AgentClassLoader: Override loadClass() instead of findClass() for explicit delegation model; use readAllBytes() instead of manual 4KB+65KB buffer management
• Fix build tasks: Use DefaultTask for rename task, add inputs/outputs for Gradle cacheability, replace deprecated buildDir with layout.buildDirectory
• Fix shadowJar filter: Replace dead org/openjdk/btrace/services/ reference with org/openjdk/btrace/extension/
• Simplify processClasspaths: Bootstrap path is now handled by Agent; remove redundant Loader.class detection logic; move debug log inside null-check

Test plan

• Build the agent JAR: ./gradlew :btrace-agent:agentJar
• Verify JAR structure: only Agent.class and AgentClassLoader*.class remain as .class under org/openjdk/btrace/agent/
• Run test suite: ./gradlew test
• Test agent attach with a sample BTrace script against a target JVM

This change is 

[Copilot]

Pull request overview

Refactors the standalone btrace-agent bootstrap/packaging so the agent entrypoint is a small unmasked Agent class that loads the real Main from .classdata, alongside Gradle changes to build a masked agent JAR.

Changes:

• Replaces Main’s premain/agentmain entrypoints with a new Agent entrypoint and AgentClassLoader for loading masked .classdata.
• Simplifies/adjusts bootstrap/system classpath processing in Main.processClasspaths.
• Adds Shadow-based packaging and post-processing tasks to produce an agentJar with most org.openjdk.btrace.agent classes renamed to .classdata.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
btrace-agent/src/main/resources/META-INF/MANIFEST.MF	Removes the static manifest resource (manifest now configured via Gradle task).
btrace-agent/src/main/java/org/openjdk/btrace/agent/Main.java	Drops premain/agentmain, exposes main(args, inst) and adjusts bootstrap classpath handling.
btrace-agent/src/main/java/org/openjdk/btrace/agent/AgentClassLoader.java	New loader that reads .classdata resources and defines classes.
btrace-agent/src/main/java/org/openjdk/btrace/agent/Agent.java	New agent entrypoint that appends to bootstrap search path and reflectively invokes Main.main.
btrace-agent/build.gradle	Adds Shadow config and tasks to build a masked agentJar and rename .class → .classdata under the agent package.

[Copilot]

The agent-class masking/AgentClassLoader approach means Main is loaded from .classdata, but processClasspaths() still relies on addPreconfLibs(libs) which locates the agent JAR via Main.class resource. In the produced agentJar, Main.class won't exist (it becomes Main.classdata), so the preconfigured btrace-libs enhancement will silently stop working. Consider locating the jar via Agent.class/ProtectionDomain (or another unmasked .class) instead of Main.class resources.

[Copilot]

InputStream.readAllBytes() is a Java 9+ API. Since this project targets/runs on Java 8+ (and compiles for 8), using it here will cause a NoSuchMethodError when the agent runs on Java 8. Replace with a Java-8-compatible read loop (e.g., ByteArrayOutputStream + buffer) or Files.readAllBytes(Path) when you have a Path.

[Copilot]

renameClassFiles renames almost all org/openjdk/btrace/agent/**/*.class to .classdata. That makes classes like org.openjdk.btrace.agent.Main and PerfReaderImpl no longer loadable via Class.forName(...) from other packages/classloaders, which currently happens (e.g., btrace-instr Preprocessor and btrace-runtime PerfReader loading). Either keep a minimal set of agent classes as real .class (exclude them from renaming) or update those reflective call sites to use the masking-aware loader/entrypoint.

Suggested change

	if (file.name != 'Agent.class' && !file.name.startsWith('AgentClassLoader')) {
	if (!(file.name == 'Agent.class'
	|| file.name.startsWith('AgentClassLoader')
	|| file.name.startsWith('Main')
	|| file.name.startsWith('PerfReaderImpl'))) {

[Copilot]

renameClassFiles declares the same directory as both input and output and then mutates it in place. This is generally not cacheable and can lead to incorrect up-to-date/build-cache behavior. Prefer writing renamed files into a separate output directory (and have agentJar consume that), or mark the task uncacheable/avoid declaring an output if it must mutate inputs.

[Copilot]

The error message here is built from e.getMessage(), which is often null/low-signal for reflective failures (notably InvocationTargetException). This can produce unhelpful output like "...: null" even though the cause is meaningful. Consider unwrapping InvocationTargetException (or using e.toString() / root cause class + message) when building msg.

[Copilot]

New Java files here don't follow the repo's enforced Google Java Format (e.g., 4-space indentation). This will likely fail spotlessCheck in CI. Please run the formatter (./gradlew spotlessApply) or reformat to GJF style before merging.

Suggested change

	private static final class Singleton {
	private static final AgentClassLoader INSTANCE = new AgentClassLoader();
	}
	private AgentClassLoader() {
	super(Agent.class.getClassLoader() == null
	? ClassLoader.getSystemClassLoader()
	: Agent.class.getClassLoader());
	}
	public static AgentClassLoader getInstance() {
	return Singleton.INSTANCE;
	}
	@Override
	public Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
	synchronized (getClassLoadingLock(name)) {
	Class<?> c = findLoadedClass(name);
	if (c != null) {
	return c;
	}
	// Try loading from .classdata resources first, then delegate to parent
	try (InputStream is = getParent().getResourceAsStream(name.replace('.', '/') + ".classdata")) {
	if (is != null) {
	byte[] data = is.readAllBytes();
	c = defineClass(name, data, 0, data.length);
	if (resolve) {
	resolveClass(c);
	}
	return c;
	}
	} catch (IOException e) {
	throw new ClassNotFoundException("Failed to read classdata for " + name, e);
	}
	return super.loadClass(name, resolve);
	}
	}
	private static final class Singleton {
	private static final AgentClassLoader INSTANCE = new AgentClassLoader();
	}
	private AgentClassLoader() {
	super(
	Agent.class.getClassLoader() == null
	? ClassLoader.getSystemClassLoader()
	: Agent.class.getClassLoader());
	}
	public static AgentClassLoader getInstance() {
	return Singleton.INSTANCE;
	}
	@Override
	public Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
	synchronized (getClassLoadingLock(name)) {
	Class<?> c = findLoadedClass(name);
	if (c != null) {
	return c;
	}
	// Try loading from .classdata resources first, then delegate to parent
	try (InputStream is =
	getParent().getResourceAsStream(name.replace('.', '/') + ".classdata")) {
	if (is != null) {
	byte[] data = is.readAllBytes();
	c = defineClass(name, data, 0, data.length);
	if (resolve) {
	resolveClass(c);
	}
	return c;
	}
	} catch (IOException e) {
	throw new ClassNotFoundException("Failed to read classdata for " + name, e);
	}
	return super.loadClass(name, resolve);
	}
	}

[Copilot]

appendToBootstrapClassLoaderSearch(new JarFile(...)) opens a JarFile that is never closed. Elsewhere in the codebase (e.g., extension loader) JarFile is closed after appending to avoid leaking file descriptors. Use try-with-resources around the JarFile, and also handle getProtectionDomain().getCodeSource() potentially being null (to avoid NPEs in non-standard classloading environments).

[Copilot]

New Java file formatting here appears inconsistent with Google Java Format (Spotless-enforced), similar to AgentClassLoader.java. Please reformat (e.g., ./gradlew spotlessApply) to avoid CI failures.

Suggested change

	public static void premain(String args, Instrumentation inst) {
	main(args, inst, true);
	}
	public static void agentmain(String args, Instrumentation inst) {
	main(args, inst, false);
	}
	private static void main(String args, Instrumentation inst, boolean isPremain) {
	try {
	// Append to bootstrap classpath before loading Main to ensure classes
	// required by Main's static initializers are available
	URL pd = Agent.class.getProtectionDomain().getCodeSource().getLocation();
	if (pd.toString().endsWith(".jar")) {
	inst.appendToBootstrapClassLoaderSearch(new JarFile(new File(pd.toURI())));
	}
	Class<?> mainClass = AgentClassLoader.getInstance().loadClass("org.openjdk.btrace.agent.Main");
	Method mainMethod = mainClass.getMethod("main", String.class, Instrumentation.class);
	mainMethod.invoke(null, args, inst);
	} catch (Exception e) {
	String msg = "BTrace agent failed to initialize: " + e.getMessage();
	System.err.println(msg);
	e.printStackTrace();
	if (isPremain) {
	throw new RuntimeException(msg, e);
	}
	}
	}
	public static void premain(String args, Instrumentation inst) {
	main(args, inst, true);
	}
	public static void agentmain(String args, Instrumentation inst) {
	main(args, inst, false);
	}
	private static void main(String args, Instrumentation inst, boolean isPremain) {
	try {
	// Append to bootstrap classpath before loading Main to ensure classes
	// required by Main's static initializers are available
	URL pd = Agent.class.getProtectionDomain().getCodeSource().getLocation();
	if (pd.toString().endsWith(".jar")) {
	inst.appendToBootstrapClassLoaderSearch(new JarFile(new File(pd.toURI())));
	}
	Class<?> mainClass =
	AgentClassLoader.getInstance().loadClass("org.openjdk.btrace.agent.Main");
	Method mainMethod =
	mainClass.getMethod("main", String.class, Instrumentation.class);
	mainMethod.invoke(null, args, inst);
	} catch (Exception e) {
	String msg = "BTrace agent failed to initialize: " + e.getMessage();
	System.err.println(msg);
	e.printStackTrace();
	if (isPremain) {
	throw new RuntimeException(msg, e);
	}
	}
	}