feat: provided-style extensions and manifest-libs

[jbachorik]

Summary

This PR introduces provided-style extensions and fat agent deployment for BTrace, enabling application class tracing without classpath pollution and single-JAR deployment for distributed environments.

Features

Provided-Style Extensions

• Application class tracing without classpath injection - Access classes loaded by application classloader through reflection
• Runtime class discovery - ClassLoadingUtil loads classes from application context at trace time
• Safe method handles - MethodHandleCache provides efficient, cached reflective access
• Manifest-driven library management - New declarative approach replaces error-prone libs= parameter

Embedded Extensions & Fat Agent

• Single-JAR deployment - Bundle agent + extensions into one JAR for Spark/Hadoop/Kubernetes
• ClassDataLoader - Runtime loading of impl classes from .classdata resources with thread-safe parallel class loading (registerAsParallelCapable + per-class locking)
• EmbeddedExtensionRepository - Discovers extensions from JAR manifest with pre-compiled Pattern validation for class names (prevents regex backtracking)
• Gradle Plugin (org.openjdk.btrace.fat-agent) - Auto-discovers extensions, supports project/maven/file sources
• Maven Plugin (btrace-maven-plugin) - Equivalent functionality for Maven users

Bug Fixes

• Bootstrap classloader NPE in Main and EmbeddedExtensionRepository
• Probe listing after disconnect and test isolation
• Thread retransform after startup scripts for early hooks
• Java 8 compatibility (List.of → Collections.emptyList)
• ClassDataLoader thread safety — fix double-define race in findClass() by adding registerAsParallelCapable() and synchronized(getClassLoadingLock(name)) with double-checked locking to prevent LinkageError from concurrent class definitions
• Regex safety — fix catastrophic backtracking in EmbeddedExtensionRepository.isValidClassName() using possessive quantifiers and pre-compiled static Pattern
• JDK 25 compatibility — exclude new Thread accessor methods (inheritableThreadLocals, getAndClearInterrupt, dispatchUncaughtException, etc.) from instrumentation to prevent infinite recursion
• Fat agent config — update to use btraceJar task (masked JAR architecture)
• Test port isolation — ManifestLibsTests uses port 2022 to avoid contention with other tests on default port 2020

Documentation

• Updated all docs to reference btrace.jar (masked JAR) instead of legacy btrace-agent.jar/btrace-boot.jar
• Fixed broken cross-references between doc files
• Added JDK 25 Thread method exclusion documentation
• Added ClassDataLoader thread-safety documentation

Documentation

Document	Description
Fat Agent Plugin Architecture	Single-JAR deployment design
Provided-Style Extensions	API on bootstrap, impl via TCCL
Migration Guide	Migrating from deprecated libs/profiles
Extension Development Guide	Building extensions
Gradle Plugin README	Both extension and fat-agent plugins

Example Extensions

• btrace-spark - Trace Spark job lifecycle
• btrace-hadoop - Trace Hadoop file operations

Usage

Fat Agent (Gradle)

plugins { id 'org.openjdk.btrace.fat-agent' }
btraceFatAgent {
    embedExtensions {
        project(':btrace-metrics')
        maven('io.btrace:btrace-statsd:2.3.0')
    }
}

Fat Agent (Maven)

<plugin>
    <groupId>org.openjdk.btrace</groupId>
    <artifactId>btrace-maven-plugin</artifactId>
    <configuration>
        <extensions>
            <extension>io.btrace:btrace-metrics:${btrace.version}</extension>
        </extensions>
    </configuration>
</plugin>

Breaking Changes

• libs= and profiles= deprecated (removal planned N+2)

🤖 Generated with Claude Code

---

This change is

[reviewabot]

It looks like the pull request diff is too large to be displayed directly on GitHub. Here are some general recommendations for handling large pull requests:

1. Break Down the PR: Try to break down the pull request into smaller, more manageable chunks. This makes it easier to review and ensures that each part can be thoroughly checked.

2. Review Locally: Clone the repository and check out the branch locally. This allows you to review the changes using your preferred tools and environment.

3. Focus on Key Areas: Identify and focus on the key areas of the code that are most critical or have the highest impact. This can help prioritize the review process.

4. Automated Tools: Use automated code review tools and linters to catch common issues and enforce coding standards.

5. Collaborate with the Team: If possible, involve multiple reviewers to share the workload and get different perspectives on the changes.

If you can provide a smaller subset of the changes or specific files that need review, I'd be happy to help with a more detailed review.

[Copilot]

Pull request overview

This PR introduces provided-style extensions and manifest-driven library loading to enable tracing application-specific classes without polluting the application classpath. The core innovation is using reflection and object hand-off patterns to access application classes at runtime, keeping BTrace isolated.

Changes:

• New extension utilities (ClassLoadingUtil, MethodHandleCache) for runtime class resolution and reflective method access
• Manifest-driven library path resolution (AgentManifestLibs) with security restrictions to BTRACE_HOME
• Refactored agent classpath processing with deduplication and better error handling
• Example extensions for Spark and Hadoop demonstrating the provided-style pattern
• Comprehensive architecture documentation and migration guides
• Deprecation of libs/profiles mechanism with escape hatch for edge cases

Reviewed changes

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

Show a summary per file

File	Description
settings.gradle	Added example extension modules (btrace-spark, btrace-hadoop)
integration-tests/src/test/java/tests/RuntimeTest.java	Fixed debug flag insertion and added timing delays for test stability
integration-tests/src/test/java/tests/ManifestLibsTests.java	New test suite for manifest-libs feature
integration-tests/src/test/java/tests/BTraceFunctionalTests.java	Improved unattended test with better synchronization and cleanup
integration-tests/build.gradle	Added test property for skipping preconfigured libs
docs/examples/README.md	Guide for building and configuring provided-style extension examples
docs/architecture/provided-style-extensions.md	Comprehensive guide on provided-style extension pattern
docs/architecture/migrating-from-libs-profiles.md	Migration guide from deprecated libs/profiles to extensions
docs/architecture/agent-manifest-libs.md	Design document for manifest-driven library loading
compile.log	Build artifact that should not be committed
btrace-extensions/examples/btrace-spark/*	Example Spark extension with API and provided-style implementation
btrace-extensions/examples/btrace-hadoop/*	Example Hadoop extension with API and provided-style implementation
btrace-extension/src/main/java/org/openjdk/btrace/extension/util/MethodHandleCache.java	Cache for reflective method handles to reduce lookup overhead
btrace-extension/src/main/java/org/openjdk/btrace/extension/util/ClassLoadingUtil.java	Helper utilities for TCCL-based class loading and service discovery
btrace-client/src/main/java/org/openjdk/btrace/client/Main.java	Removed exit hooks for list-only operations to prevent unintended probe removal
btrace-client/src/main/java/org/openjdk/btrace/client/Client.java	Added agent-already-running detection and system property pass-through
btrace-agent/src/main/java/org/openjdk/btrace/agent/RemoteClient.java	Improved disconnect handling with disconnecting flag
btrace-agent/src/main/java/org/openjdk/btrace/agent/Main.java	Refactored classpath processing with manifest support, deduplication, and escape hatch for single-jar injection
btrace-agent/src/main/java/org/openjdk/btrace/agent/AgentManifestLibs.java	New utility for manifest-driven library resolution with security checks
README.md	Added deprecation notice for libs/profiles with migration guidance

[Copilot]

@jbachorik I've opened a new pull request, #792, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@jbachorik I've opened a new pull request, #793, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@jbachorik I've opened a new pull request, #794, to work on those changes. Once the pull request is ready, I'll request review from you.

[jbachorik]

ReDoS in EmbeddedExtensionRepository.isValidClassName — fixed in dbd3964

Addresses the CodeQL ReDoS alert (js/redos, alert #15) on the class-name validation pattern.

The regex had three ambiguities the engine could explore:

1. Inner char class [a-zA-Z0-9_$]* inside the .-group and the $-group both allowed $
2. The outer (\$[a-zA-Z_][a-zA-Z0-9_$]*)* group also started with $
3. All three repetitions were standard (backtracking) *

On an input like A$A$A$A…! (trailing mismatch) this produced exponential partition searching before failing.

Fix: switch the three repetitions to possessive quantifiers (*+). Each commit is final, so the engine cannot re-partition $-separated tail segments. Match semantics are preserved for every accepted and rejected input in SecurityValidationTest > Class name validation (valid / invalid / special-chars / code-injection / null-and-empty — all green).

Stress-check: A$A × 30 + ! now rejects in ~0 ms instead of hanging.

Items 1–5 of the earlier review (unused Enumeration import, redundant homeStr null check, three zip-slip sites in FatAgentMojo) were already addressed in prior commits on this branch.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated 12 comments.

[Copilot]

argVals is built with debugBTrace ? "-v" : "" and then (when debugBTrace=true) another -v is inserted at index 4. This results in a duplicate -v argument, and when debugBTrace=false it leaves an empty-string argument in the command line. Consider removing the placeholder element and only adding -v conditionally (without inserting empty args).

[jbachorik]

Addressed in eaf1686 — removed the debugBTrace ? "-v" : "" ternary placeholder from the initial Arrays.asList(); -v is now added once via the existing conditional add(4, "-v") when debugBTrace=true.

[Copilot]

This additional fixed Thread.sleep(1000L) adds another full second of delay even though there is already a 1s sleep immediately above for output flushing. Using two unconditional sleeps increases test runtime and can still be flaky; prefer waiting on a concrete condition (e.g., latch for expected app output) or reusing the existing flush wait.

[jbachorik]

Intentionally kept — the two sleeps serve distinct purposes: the first allows late BTrace output to flush in on-startup mode, the second allows the traced app to produce output after ready:. Collapsing them into one risks missing app output. A latch-based approach would be preferable but requires broader refactoring of the test harness.

[Copilot]

The note says legacy multi-JAR users should “substitute btrace.jar”, which is the same value used in the example and doesn’t explain what to substitute. This looks like a copy/paste error; it should point to the legacy jar(s) (e.g., btrace-agent.jar / btrace-boot.jar) or otherwise clarify the substitution.

[jbachorik]

Addressed in eaf1686 — note now reads: "If using a legacy multi-JAR distribution, replace btrace.jar with btrace-agent.jar (and add -Xbootclasspath/a:btrace-boot.jar where needed)."

[Copilot]

The note says legacy multi-JAR users should “substitute btrace.jar”, which is the same value used in the example. This looks incorrect/ambiguous; update it to reference the actual legacy jar name(s) or clarify what should be substituted.

[jbachorik]

Addressed in eaf1686 — same correction applied to BTraceTutorial.md.

[Copilot]

Link target architecture/extension-invokedynamic-bridge.md does not exist in docs/architecture/ (the file present is ExtensionInvokeDynamicBridge.md). Also, this section uses a different filename casing (GettingStarted.md, QuickReference.md, etc.) than other links earlier in this doc; please make link casing consistent across the whole file to avoid broken links on case-sensitive filesystems.

[jbachorik]

Addressed in eaf1686 — link target corrected to architecture/ExtensionInvokeDynamicBridge.md; link casing standardized throughout the "Documentation by Topic" section to match the capitalized filenames used elsewhere.

[Copilot]

Main.class.getClassLoader() can be null when loaded from the bootstrap classloader; calling getResourceAsStream(...) on it will throw an NPE. Use the same null-safe resource loading approach used elsewhere (fallback to ClassLoader.getSystemResourceAsStream) so embedded probe lookup works reliably.

Suggested change

	try (InputStream is = Main.class.getClassLoader().getResourceAsStream(resourcePath)) {
	ClassLoader loader = Main.class.getClassLoader();
	try (InputStream is =
	loader != null
	? loader.getResourceAsStream(resourcePath)
	: ClassLoader.getSystemResourceAsStream(resourcePath)) {

[Copilot]

ClassDataLoader stores resourceLoader as-is and later unconditionally calls resourceLoader.getResourceAsStream(...). If the loader is null (bootstrap), this will throw an NPE. Consider normalizing null to ClassLoader.getSystemClassLoader() (as done in EmbeddedExtensionRepository) or guarding at use sites.

Suggested change

	this.resourceLoader = resourceLoader;
	this.resourceLoader =
	resourceLoader != null ? resourceLoader : ClassLoader.getSystemClassLoader();

[jbachorik]

Addressed in eaf1686 — constructor now normalizes: this.resourceLoader = resourceLoader != null ? resourceLoader : ClassLoader.getSystemClassLoader();

[Copilot]

This test uses List.of(...) (Java 9+) which will fail if unit tests are ever executed on Java 8 (and contradicts the PR’s stated Java 8 compatibility goal). Prefer Collections.singletonList(...) / Arrays.asList(...) to keep tests Java-8-runtime compatible.

[jbachorik]

Addressed in eaf1686 — replaced with Collections.singletonList() and added the required import java.util.Collections;.

[Copilot]

This module hard-codes version = '2.3.0-SNAPSHOT', which can diverge from the repository’s root version and lead to inconsistent published artifacts. Prefer inheriting rootProject.version (or a shared version property) so the Maven plugin version stays aligned with the rest of the build.

[jbachorik]

Addressed in eaf1686 — version = '2.3.0-SNAPSHOT' replaced with version = rootProject.version.

[Copilot]

The runBTrace(..., checkLines, ...) helper still builds argVals with debugBTrace ? "-v" : "" and then conditionally inserts another -v at index 4. This duplicates -v when enabled and leaves an empty-string arg when disabled. Remove the placeholder element and only add -v once when needed.

[jbachorik]

Addressed in eaf1686 — same fix applied to the second runBTrace helper (lines ~1136–1147).

[jbachorik]

Review-fix commits pushed

Addressed 17 of 23 accepted findings from the muse chorus review (session review-20260421-095158), in two commits:

• 84576bfa — fix: address review findings in PR feat: provided-style extensions and manifest-libs #791 (7 files, 15 findings)
• 21c59452 — docs: document ordering and null-classloader contracts (2 files, 2 findings)

Highlights

• Main.java: bootstrap/system jar append failures now log at warn; Thread.class retransform failure warns; explicit warn when both deprecated libs= and manifest-libs are in use; FIXME marker on loadBundledProbes() placeholder; comment clarifying Thread-class retransform ordering is intentional.
• ClassDataLoader.findClass: IOException cause is preserved when converting to ClassNotFoundException.
• MethodHandleCache: negative cache removed — failed lookups no longer permanently poison the cache.
• FatAgentMojo.createJar: Files.walk() now in try-with-resources.
• AgentManifestLibs: skip/validation log levels upgraded (DEBUG → INFO / WARN) so operators see non-existent / invalid entries.
• EmbeddedExtensionRepository.readManifestIndex: iterates all BTrace-Embedded-Extensions manifests instead of returning on first match; dedupes extension IDs and warns on duplicates. Null-classloader handling now explicit via BOOTSTRAP_SENTINEL + isBootstrap flag.
• ClassFilter: expanded rationale comment for JDK-25 Thread-method exclusions — kept getUncaughtExceptionHandler in the exclusion set because probes on the getter could re-enter the dispatcher path.

Verification

• ./gradlew :btrace-agent:compileJava :btrace-extension:compileJava :btrace-instr:compileJava :btrace-maven-plugin:compileJava — green
• ./gradlew spotlessCheck — green
• Unit tests: 930/930 pass (920 module + 10 gradle-plugin), 0 failures, 0 errors

Deferred for follow-up

6 findings were not addressed in these commits because they need architectural or design decisions best handled separately:

1. ClassDataLoader.java:98 — add ProtectionDomain/CodeSource on defineClass. Planner concertato recommended deferring: post-JDK 17 removal of SecurityManager makes a correct ProtectionDomain non-trivial.
2. FatAgentMojo.java:~6055 — extract duplicated isValidExtensionId logic. Needs a module-boundary decision (new btrace-plugin-common vs. intentional duplication).
3. FatAgentMojoTest.java:189 — replace "no-behavior-assertion" tests with concrete assertions.
4. SecurityValidationTest.java:3392 — replace mock-based bytecode magic test with a real ClassDataLoader integration test.
5. BTraceFatAgentPluginTest.java:~5117 — test duplication (phantom line numbers — requires locating the actual duplicated test code).
6. BTraceFatAgentPlugin.groovy:~4722 — unify Gradle and Maven plugin extension-staging logic. Planner concertato flagged as out of scope for this PR.

The user also dismissed two earlier findings with explicit rationale: ClassDataLoader double-check-lock (intentional lockfree fast path) and ClassDataLoader:2468 concurrency (assumed harmless race, to be validated).

🤖 Generated with Claude Code