[fault-injection] Unprotected access to JVM class loader data mutex and method list - #779
Open
zhengyu123 wants to merge 7 commits into
Open
zhengyu123 wants to merge 7 commits into
zhengyu123 wants to merge 7 commits into
Conversation
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
Contributor
CI Test ResultsRun: #35213508028 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-17 11:20:07 UTC |
Contributor
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 2 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 2ba3f13 | Docs | View more details | Give us feedback! |
…va-profiler into zgu/jdk8_patchClassLoaderData
zhengyu123
marked this pull request as ready for review
September 17, 2026 19:43
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The static review finds no defect in the JDK 8 lock recovery or per-class delta tag paths.
🤖 Datadog Autotest · Commit 2ba3f13 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
Hotspot JDK8 specific.
patchClassLoaderData()prependsMethodListblocks onto a class'sClassLoaderDatato work around a JDK 8 jmethodID layout bug. This PR hardens that path:cld->lock()/methodList()mutation and its matchingcld->unlock()with the profiler's signal/crash-protection machinery (ProfiledThread,JmpCtxScope,sigsetjmp/SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP), via the newLockStateRAII helper, so a crash while the classloader-data mutex is held during a signal-unsafe window unlocks it instead of deadlocking the JVM.jclass(via JVMTIGet/SetTag) with themethod_countalready prepended, so repeated calls for the same class (profiler restart replay,RedefineClasses/RetransformClasses) only prepend the delta instead of leaking another full set ofMethodListblocks on every re-run.cld->lock()suppresses safepoint checks whileGet/SetTagare safepoint-polling JVMTI entry points.Motivation:
Fault injection testing found that unprotected access to the JVM class loader data mutex and method list in
patchClassLoaderData()could crash or deadlock the JVM.Additional Notes:
The
SetTag()call is not serialized against a concurrentpatchClassLoaderData()call for the same class, so two racing callers can both observe a stale tag and both patch. This only wastes one extra round of preallocated blocks in that rare case; unlike the original unguarded code, it can no longer grow unboundedly.How to test the change?:
Added unit tests in
ddprof-lib/src/test/cpp/hotspotSupport_ut.cppcovering the locking/crash-protection and idempotent-tagging behavior ofpatchClassLoaderData().For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.Unsure? Have a question? Request a review!