Return JMETHODID_NOT_WALKABLE instead of nullptr for unpopulated jmethodID slots - #733
Return JMETHODID_NOT_WALKABLE instead of nullptr for unpopulated jmethodID slots#733jbachorik wants to merge 1 commit into
Conversation
…hodID slots VMMethod::id() returned nullptr when the jmethodID cache was not yet populated for a klass. fillJavaFrame treats nullptr as 'store raw Method*', which defers a dereference of potentially-reclaimed metadata to dump time. This fires on every RedefineClasses/RetransformClasses: the cache is briefly empty between invalidation and repopulation, and a thread sampled in that window stores a raw Method* that may fault at dump time. Return JMETHODID_NOT_WALKABLE instead so fillJavaFrame stores the sentinel without the raw flag. The dump thread maps the sentinel to the shared unknown method, avoiding any raw Method* dereference.
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 465607fc9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // No jmethodID cache allocated for this klass yet; same reason | ||
| // as above — return the sentinel, not nullptr. | ||
| return JMETHODID_NOT_WALKABLE; |
There was a problem hiding this comment.
Preserve the raw fallback for intentionally unprimed classes
When profiling HotSpot with fjmethodid=false and VM/default C-stack mode, initExecution() enters Partial_loaded, and loadMethodIDsIfNeededImpl() deliberately leaves ordinary bootstrap, platform, and application-loader classes without a jmethodID cache because their stable Method* values are resolved through this raw-pointer fallback. Thus ids == NULL is an expected persistent state, not necessarily the brief retransformation race described here. Returning the sentinel makes compiled frames for those classes serialize as unknown, while interpreter paths that reject JMETHODID_NOT_WALKABLE can terminate the walk at the first such frame, substantially corrupting profiles in this supported configuration. Restrict the sentinel behavior to invalidation-sensitive cases while retaining the null/raw fallback for deliberately unprimed stable classes.
Useful? React with 👍 / 👎.
CI Test ResultsRun: #31506496226 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 29 | Failed: 0 Updated: 2026-08-11 15:44:33 UTC |
|
❌ 38 passed, 1 failed out of 39 configurations Test Matrix
Failure Detailsmusl-x64-openj9-jdk17Profiler-only: Links
|
|
What does this PR do?:
Changes
VMMethod::id()to returnJMETHODID_NOT_WALKABLEinstead ofnullptrwhen the jmethodID cache is not yet populated for a klass. This preventsfillJavaFramefrom taking the rawMethod*pointer fallback path.Motivation:
VMMethod::id()returnednullptrwhen a klass's jmethodID cache was not yet populated.fillJavaFrametreatsnullptras "store rawMethod*pointer", deferring a dereference of that pointer to dump time (up to ~60s later per JFR chunk length). If HotSpot's redefinition cleanup reclaimed the underlyingMethod*/Symbol*metadata in that window, the unguarded memcpys inHotspotSupport::resolve()faulted on freed memory.This fires on every
RedefineClasses/RetransformClasses(routine, continuous — e.g. dd-trace-java agent retransformation): the cache is briefly empty between invalidation and repopulation, and a thread sampled in that window hits thenullptrpath. Independent of the_force_jmethodID/fjmethodidconfig — that flag only controls the startup-time preload sweep, not this fallback.Chain:
id()returnsnullptr→fillFrameRawstores rawMethod*+isRawPointerflagresolveMethodseesisRawPointer(bci)→ callsHotspotSupport::resolve()→ unguarded memcpys fault on reclaimed metadataFix: Return
JMETHODID_NOT_WALKABLEfrom all unpopulated-cache paths inVMMethod::id().fillJavaFramestores the sentinel without the raw flag; the dump thread maps it to the shared "unknown" method. No rawMethod*dereference occurs. Covers 7 call sites (2 directid()callers + 5getMethodId()→validatedId()→id()callers).Additional Notes:
fillFrameRawandisRawPointerleft in place as defensive dead codeHotspotSupport::resolve()still returnsnullptras the dump-time "unknown" signal — that contract withflightRecorder.cppis unchangedfillJavaFrame/fillFrameRaw/isRawPointerHow to test the change?:
HotspotMethodIdTest.RejectedMethodIdStaysNonRawAndResolvesToUnknown— passes (gtest)JMethodIDInvalidationStressTest— passes (integration, slow suite)For Datadog employees: