Repository navigation
Require trace IDs to increase in creation order during calibration - #885
Conversation
KlassTraceId::findOffset() accepted any offset at which the probes held distinct values within ID_SLACK of each other after >> 16. On aarch64 17u a pointer field at offset 24 passed that next to the real trace ID at 168, so calibration found two candidates, refused both, and fjmethodid=lazy stayed off for the JVM (CI: glibc-17-debug-aarch64). JFR assigns trace IDs from one global counter as each klass is created, and the probes are created one after another, so their IDs strictly increase, mostly by exactly one. plausibleIds() now requires both: values in creation order, and at least half the steps +1. Pointers neither follow creation order nor step by exactly one. The first attempt in a JVM regularly sees steps of 2 to 4 and is now rejected, so calibrate() makes up to six attempts instead of four. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The calibration checks reject the reported pointer lookalike while accepting ordered trace IDs with limited gaps. The completed static review identified no reportable defect.
🤖 Bits Code Review · Commit 32476eb
CI Test ResultsRun: #39782248041 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 31 | Failed: 0 Updated: 2026-10-22 13:18:55 UTC |
What
KlassTraceIdcalibration (fjmethodid=lazy, from #868) now only accepts an offset at which the fresh probe klasses' values increase strictly in creation order, with at least half the steps exactly +1. Before, it accepted any distinct values within the slack window.calibrate()also makes up to six attempts instead of four.Why
On
test-linux-glibc-aarch64 (17, debug, regular), seen on #883, calibration found two candidate offsets, refused both, and lazy mode stayed off for the whole JVM. All four lazy tests then failed. The probe dump showed (values>> 16):f5e11aef f5e11afb f5e11ad7 f5e11b11 f5e11b13 f5e11ae34173 4174 4175 4176 4177 4178JFR hands out trace IDs from one global counter as each klass is created. The probes are created one after another (array dimensions 1 to 6 of a fresh class), so their IDs strictly increase and mostly step by exactly one; other threads creating classes meanwhile only add gaps. Pointers neither follow creation order nor step by one. Offset 24 above already goes down at its third probe.
Why not a per-version offset table
The offset isn't a function of the JDK version alone. On mainline it depends on the CDS Java heap build option, and vendor builds may add fields. A table would still need a runtime check, and couldn't cover builds it doesn't know. The stronger check is that runtime check.
Cost
The first attempt in a JVM regularly sees steps of 2 to 4. It's now rejected, and the second attempt succeeds. Observed on JDK 17, 21 and 25 on macOS; I don't know what takes those IDs. That's why the attempt budget goes from four to six, leaving the same margin for interference from other threads. An attempt costs microseconds plus a tiny probe class loader that GC can unload.
Tests
klassTraceId_ut:RejectsPointersThatLookLikeCloseIdsuses the CI values above and must pick 168;RejectsIdsOutOfCreationOrderandRejectsIdsThatRarelyStepByOne;ToleratesIdsTakenByOtherThreadsreplaces the old any-order test, which was only needed while the probes weren't stored in creation order.🤖 Generated with Claude Code