Repository navigation
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
CI Test ResultsRun: #39982535700 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled
|
| Test | Cells | Message |
|---|---|---|
VirtualThreadWallClockTest.cstack_vmx |
glibc-21-graal-debug-amd64 |
org.opentest4j.AssertionFailedError: No sample showed carrier-thread frames (ForkJoinWorkerThread) — continuation unwind |
These fail the build. Passing on a second run makes a test flaky, not passing. Fix it, or quarantine it against a ticket so the debt is tracked rather than forgotten.
Consider quarantining these — click for ready-made entries
A quarantined test still runs and still reports; its failures just stop
turning CI red. To quarantine one:
- Open a PROF ticket for the test, linking the failing job.
- Append the line below to
ddprof-test/quarantine.txt, replacing
PROF-XXXXXwith the ticket number. - Check the
cellsandreasoncolumns — the proposal only knows what
failed in this run, and a narrowercellsglob keeps the same test
gating everywhere it has not misbehaved.
CI fails once review_by passes, so an entry expires instead of piling up.
# test | ticket | added | review_by | cells | reason
# WIDENED: observed `com.datadoghq.profiler.wallclock.VirtualThreadWallClockTest.cstack=vmx`, which covers() cannot match exactly; this entry mutes the whole class
com.datadoghq.profiler.wallclock.VirtualThreadWallClockTest.* | PROF-XXXXX | 2026-10-23 | 2027-01-21 | glibc-21-graal-debug-amd64 | org.opentest4j.AssertionFailedError: No sample showed carrier-thread frames (ForkJoinWorkerThread) — continuation unwind may be broken ==> expected: <true> but was: <false> (seen in: glibc-21-graal-debug-amd64)
❌ Failing tests
| Test | Cells | Message |
|---|---|---|
VirtualThreadWallClockTest.cstack_vmx |
glibc-21-debug-amd64 |
org.opentest4j.AssertionFailedError: No sample showed carrier-thread frames (ForkJoinWorkerThread) — continuation unwind |
Retried 2 of 32 cells.
Failed Jobs
Summary: Total: 32 | Passed: 30 | Failed: 2
Updated: 2026-10-23 16:39:10 UTC
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-prod-us1-3[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Static review supports setting truncation before either continuation-root termination or carrier walking when frozen callers are skipped. Fully thawed continuations retain their existing completeness behavior.
🤖 Bits Code Review · Commit e3bf2d3
|
the 2 failing CI tests highlight a bug that is tracked in a follow-up PR: #887 |
This comment has been minimized.
This comment has been minimized.
Reliability & Chaos Results❌ 1 failure(s) detected Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/146812389 ❌ chaos: profiler tcmalloc aarch64 25 0 3 temXchaos |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
| /** Ending at a fully thawed continuation is a complete logical stack in the ordinary VM mode. */ | ||
| @TestTemplate | ||
| @ValueSource(strings = {"vm"}) | ||
| public void keepsFullyThawedContinuationComplete() throws Exception { |
There was a problem hiding this comment.
The 'fully thawed continuation' control test calls runWorkload(true, false): park=false, so no continuation is ever frozen/re-thawed — the test exercises the never-unmounted path while its name and message imply a remounted-then-fully-thawed continuation. Consider renaming or re-documenting the intent.
There was a problem hiding this comment.
[Sphinx Review — LOW] asyncJavaTraceWithPostProcessing appends a synthetic "JVM Continuation" frame when the carrier thread is a VT carrier, but does not set *truncated even though AGCT (like the VM walker before this fix) does not reconstruct the frozen virtual-thread callers. The truncated flag is available in that function's signature (it is passed through to getJavaTraceAsync) and the new test asserts completeness via cstack=vm/vmx only, so the cstack modes that dispatch to the AGCT arm (cstack < CSTACK_VM, e.g. default/dwarf) can still emit stacks that omit a JVM-confirmed logical caller while claiming completeness. This is the same class of incompleteness the change fixes for walkVM.
Suggestion: In the isCarryingVirtualThread() branch of asyncJavaTraceWithPostProcessing, set *truncated = true (guarded on truncated != nullptr) before appending the synthetic frame, mirroring the walkThroughContinuation change; extend the new test's cstack matrix to cover at least one AGCT-dispatched mode.
|
Partially fixed in b5c9832 (general review comment): ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp:572-574 — speculative unknown-nmethod crossing now passes CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM (Unverified); walkThroughContinuation (:444-446) marks *truncated for every non-Thawed boundary, so frozen callers can no longer be dropped silently there, and a static_assert pins the constant. The known limitation (JDK 21-26 thawed fp layout guess may be off by one frame for a frozen continuation) is documented in the comment at :555-565. Residual: the coverage is a JVM-free gtest of the classification (contBoundary_ut.cpp) plus a compile-time check; no end-to-end test drives the speculative path with a misresolved cont_entry_return_pc, as the review suggested. — residual named for the author |
b5c9832 to
aa66961
Compare
Keep the walker truncation flag when a stack ends at a Java entry method. Exercise speculative continuation recovery and stack-pool header encoding with native regression tests, and rename the never-parked virtual-thread control.
What does this PR do?:
Mark VM-walked stacks as truncated when a continuation return barrier leaves older Java callers frozen in heap chunks. Apply the flag before stopping at the synthetic continuation root (
cstack=vm) or continuing onto carrier frames (cstack=vmx). Speculative crossings are explicitly classified as unverified and truncated; verified fully thawed boundaries retain their existing completeness behavior.Preserve explicit walker truncation when serializing a stack whose root is a Java entry method. Reaching carrier ancestry must not clear the flag for skipped application callers.
Add Java regression coverage against the JVM's logical stack, with platform-thread and never-parked virtual-thread controls, plus deterministic native tests of continuation recovery and serialized stack-pool headers.
Motivation:
After a virtual thread unmounts and remounts, HotSpot can thaw only part of its stack. The profiler skips the remaining frozen callers but previously could serialize the result as complete. Carrier frames can still be visible while application callers are missing.
Additional Notes:
How to test the change?:
Validation of the final changes on macOS arm64 / OpenJDK 25.0.2:
spotlessApplyandgit diff --checkpass.The deterministic native fixtures drive the production walker with synthetic metadata and inspect bytes from the production stack-pool header encoder. They do not emulate a live JVM freeze/thaw sequence with misresolved metadata or produce a whole JFR recording. Linux, sanitizer configurations, and the complete Java integration suite were not rerun for the final serializer/test changes.
For Datadog employees: