Skip to content

Remove Unsafe class injection from Java agent - #21542

Merged
andrross merged 1 commit into
opensearch-project:mainfrom
andrross:agent-warnings
May 8, 2026
Merged

Remove Unsafe class injection from Java agent#21542
andrross merged 1 commit into
opensearch-project:mainfrom
andrross:agent-warnings

Conversation

@andrross

@andrross andrross commented May 7, 2026

Copy link
Copy Markdown
Member

The Java agent previously used ClassInjector.UsingUnsafe at premain time to inject AgentPolicy, SubjectInterceptor, and the two stack caller chain extractors into the boot classloader. This relied on sun.misc.Unsafe, which is deprecated for removal on recent JDKs and emits JVM warnings.

This change relocates StackCallerClassChainExtractor, StackCallerProtectionDomainChainExtractor, and SubjectInterceptor from :libs:agent-sm:agent into :libs:agent-sm:bootstrap, under a new org.opensearch.javaagent.bootstrap.internal subpackage to distinguish them from AgentPolicy, which is the shared API between the server and the agent. The bootstrap jar is listed on the agent jar's Boot-Class-Path manifest attribute, so the JVM loads these classes into the boot classloader natively with no runtime injection needed.

Also set -Dnet.bytebuddy.safe=true on test JVMs to disable the Unsafe-based dispatchers inside ByteBuddy's ClassInjector. ByteBuddy falls back to MethodHandles.Lookup / reflection injection, which works on JDK 9+.

Check List

  • Functionality includes testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

The Java agent previously used ClassInjector.UsingUnsafe at premain
time to inject AgentPolicy, SubjectInterceptor, and the two stack
caller chain extractors into the boot classloader. This relied on
sun.misc.Unsafe, which is deprecated for removal on recent JDKs and
emits JVM warnings.

This change relocates StackCallerClassChainExtractor,
StackCallerProtectionDomainChainExtractor, and SubjectInterceptor
from :libs:agent-sm:agent into :libs:agent-sm:bootstrap, under a new
org.opensearch.javaagent.bootstrap.internal subpackage to distinguish
them from AgentPolicy, which is the shared API between the server and
the agent. The bootstrap jar is listed on the agent jar's
Boot-Class-Path manifest attribute, so the JVM loads these classes
into the boot classloader natively with no runtime injection needed.

Also set -Dnet.bytebuddy.safe=true on test JVMs to disable the
Unsafe-based dispatchers inside ByteBuddy's ClassInjector. ByteBuddy
falls back to MethodHandles.Lookup / reflection injection, which
works on JDK 9+.

Signed-off-by: Andrew Ross <andrross@amazon.com>
@andrross
andrross requested a review from a team as a code owner May 7, 2026 17:43
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Move internal classes to bootstrap internal subpackage

Relevant files:

  • libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/internal/StackCallerClassChainExtractor.java
  • libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/internal/StackCallerProtectionDomainChainExtractor.java
  • libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/internal/SubjectInterceptor.java
  • libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/internal/package-info.java

Sub-PR theme: Remove UsingUnsafe injection and update agent references

Relevant files:

  • libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java
  • libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/FileInterceptor.java
  • libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/RuntimeHaltInterceptor.java
  • libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/SocketChannelInterceptor.java
  • libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/SystemExitInterceptor.java
  • libs/agent-sm/agent/src/test/java/org/opensearch/javaagent/StackCallerProtectionDomainExtractorTests.java
  • libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/AccessController.java

⚡ Recommended focus areas for review

Boot ClassLoader Loading

The PR removes the explicit ClassInjector.UsingUnsafe injection and relies on the bootstrap jar's Boot-Class-Path manifest attribute to load classes into the boot classloader. It's important to verify that the bootstrap jar is correctly listed in the agent jar's manifest Boot-Class-Path attribute, and that the new internal subpackage classes are actually included in the bootstrap jar at build time. If the manifest or build configuration is not updated correctly, the classes won't be available in the boot classloader at runtime, causing NoClassDefFoundError when ByteBuddy-woven JDK code tries to reference them.

final AgentBuilder.Transformer subjectTransformer = (b, typeDescription, classLoader, module, pd) -> b.method(
    ElementMatchers.named("getSubject")
).intercept(MethodDelegation.to(SubjectInterceptor.class));

final ByteBuddy byteBuddy = new ByteBuddy().with(Implementation.Context.Disabled.Factory.INSTANCE);
var builder = new AgentBuilder.Default(byteBuddy).with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE)
    .with(AgentBuilder.RedefinitionStrategy.REDEFINITION)
    .with(AgentBuilder.TypeStrategy.Default.REDEFINE)
Package Visibility

SubjectInterceptor has been moved to the internal subpackage. Since ByteBuddy's MethodDelegation references this class by name at instrumentation time, verify that the class is still accessible (public) and that the delegation target methods have the correct visibility modifiers after the package change.

package org.opensearch.javaagent.bootstrap.internal;

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Bootstrap classloader visibility must be ensured

The removal of ClassInjector.UsingUnsafe.ofBootLoader() means SubjectInterceptor,
StackCallerProtectionDomainChainExtractor, StackCallerClassChainExtractor, and
AgentPolicy are no longer injected into the bootstrap classloader. When ByteBuddy
weaves advice into JDK classes, those classes are loaded by the bootstrap
classloader and cannot see agent-classloader classes, causing NoClassDefFoundError
at runtime. Ensure these classes are appended to the bootstrap search path via
instrumentation.appendToBootstrapClassLoaderSearch(...) before the agent builder is
invoked.

libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [92-96]

 final AgentBuilder.Transformer subjectTransformer = (b, typeDescription, classLoader, module, pd) -> b.method(
     ElementMatchers.named("getSubject")
 ).intercept(MethodDelegation.to(SubjectInterceptor.class));
 
+// Ensure internal bootstrap classes are visible to the bootstrap classloader
+// instrumentation.appendToBootstrapClassLoaderSearch(agentJarFile);
+
 final ByteBuddy byteBuddy = new ByteBuddy().with(Implementation.Context.Disabled.Factory.INSTANCE);
Suggestion importance[1-10]: 5

__

Why: This is a legitimate concern — removing ClassInjector.UsingUnsafe.ofBootLoader() without an alternative bootstrap injection mechanism could cause NoClassDefFoundError at runtime for JDK-instrumented classes. However, the improved_code only adds a commented-out placeholder rather than a real fix, and the PR may have an alternative mechanism (e.g., Boot-Class-Path manifest entry) not visible in the diff.

Low
Clarify boot classloader loading mechanism

The package-info.java states these classes must be loaded by the boot classloader,
but the ClassInjector.UsingUnsafe.ofBootLoader() injection was removed in this PR.
Without that injection, these classes will be loaded by the agent classloader
instead of the boot classloader, which means JDK classes instrumented via ByteBuddy
Advice/MethodDelegation may fail to resolve them at runtime with a
NoClassDefFoundError. Verify that an alternative mechanism (e.g., Boot-Class-Path
manifest entry or Instrumentation.appendToBootstrapClassLoaderSearch) is in place to
ensure these classes are available to the boot classloader.

libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/internal/package-info.java [11-14]

 * so that bytecode woven into JDK classes (either inlined ByteBuddy Advice or
 * MethodDelegation stubs) can resolve them. These classes are implementation
 * details of the Java agent and are not part of any public API; do not depend
 * on them from outside {@code :libs:agent-sm:agent}.
+* They are placed on the bootstrap classpath via the agent JAR's
+* {@code Boot-Class-Path} manifest attribute or
+* {@code Instrumentation.appendToBootstrapClassLoaderSearch}.
Suggestion importance[1-10]: 4

__

Why: The suggestion raises a valid concern about how the internal classes reach the bootstrap classloader after the ClassInjector.UsingUnsafe.ofBootLoader() removal, but the improved_code only adds more Javadoc text rather than fixing any actual code issue. The suggestion asks the reviewer to verify an alternative mechanism exists, which is a documentation/verification concern rather than a code fix.

Low

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for ed7cc24: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for ed7cc24: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for ed7cc24: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

✅ Gradle check result for ed7cc24: SUCCESS

@andrross

andrross commented May 8, 2026

Copy link
Copy Markdown
Member Author

@reta About 3000 fewer warnings in the build log! For now anyway...

% grep 'Unsafe' pre-agent-update.log | wc -l
    3372
% grep 'Unsafe' post-agent-update.log | wc -l
     231

@andrross
andrross merged commit e0a06f2 into opensearch-project:main May 8, 2026
34 of 46 checks passed
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.45%. Comparing base (6888345) to head (ed7cc24).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #21542      +/-   ##
============================================
+ Coverage     73.42%   73.45%   +0.02%     
- Complexity    74547    74548       +1     
============================================
  Files          5978     5975       -3     
  Lines        338743   338720      -23     
  Branches      48843    48843              
============================================
+ Hits         248707   248790      +83     
+ Misses        70229    70067     -162     
- Partials      19807    19863      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrross
andrross deleted the agent-warnings branch May 8, 2026 04:21
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026
The Java agent previously used ClassInjector.UsingUnsafe at premain
time to inject AgentPolicy, SubjectInterceptor, and the two stack
caller chain extractors into the boot classloader. This relied on
sun.misc.Unsafe, which is deprecated for removal on recent JDKs and
emits JVM warnings.

This change relocates StackCallerClassChainExtractor,
StackCallerProtectionDomainChainExtractor, and SubjectInterceptor
from :libs:agent-sm:agent into :libs:agent-sm:bootstrap, under a new
org.opensearch.javaagent.bootstrap.internal subpackage to distinguish
them from AgentPolicy, which is the shared API between the server and
the agent. The bootstrap jar is listed on the agent jar's
Boot-Class-Path manifest attribute, so the JVM loads these classes
into the boot classloader natively with no runtime injection needed.

Also set -Dnet.bytebuddy.safe=true on test JVMs to disable the
Unsafe-based dispatchers inside ByteBuddy's ClassInjector. ByteBuddy
falls back to MethodHandles.Lookup / reflection injection, which
works on JDK 9+.

Signed-off-by: Andrew Ross <andrross@amazon.com>
Bukhtawar pushed a commit to Bukhtawar/OpenSearch that referenced this pull request May 10, 2026
The Java agent previously used ClassInjector.UsingUnsafe at premain
time to inject AgentPolicy, SubjectInterceptor, and the two stack
caller chain extractors into the boot classloader. This relied on
sun.misc.Unsafe, which is deprecated for removal on recent JDKs and
emits JVM warnings.

This change relocates StackCallerClassChainExtractor,
StackCallerProtectionDomainChainExtractor, and SubjectInterceptor
from :libs:agent-sm:agent into :libs:agent-sm:bootstrap, under a new
org.opensearch.javaagent.bootstrap.internal subpackage to distinguish
them from AgentPolicy, which is the shared API between the server and
the agent. The bootstrap jar is listed on the agent jar's
Boot-Class-Path manifest attribute, so the JVM loads these classes
into the boot classloader natively with no runtime injection needed.

Also set -Dnet.bytebuddy.safe=true on test JVMs to disable the
Unsafe-based dispatchers inside ByteBuddy's ClassInjector. ByteBuddy
falls back to MethodHandles.Lookup / reflection injection, which
works on JDK 9+.

Signed-off-by: Andrew Ross <andrross@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants