Repository navigation
Conversation
| // G1/Serial/Parallel relocate objects at safepoints. A sampled Java thread | ||
| // cannot reach a safepoint while its signal handler is reading these oops. | ||
| // Concurrently relocating collectors require load barriers not provided here. |
There was a problem hiding this comment.
Can the profiling signal land on a thread with frozen stack and already in safepoint?
Or is this structurally impossible?
There was a problem hiding this comment.
yes it can, it is not structurally excluded. There is a protection in FrozenStackWalker::append: it rejects any thread state other than _thread_in_Java before reading heap objects. HotSpot switches to _thread_blocked before waiting at the safepoint, so those samples skip frozen-frame recovery and retain truncated=true. For accepted samples, the PC must also belong to interpreted or compiled Java code, excluding freeze/thaw runtime execution.
CI Test ResultsRun: #40700749903 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled
|
| Test | Cells | Message |
|---|---|---|
HighArityVirtualThreadStackTest.retainsCarrierWithMoreThan128StackArgumentWords___1_ |
musl-25-librca-debug-aarch64 |
High-arity samples must retain exactly one carrier com.datadoghq.profiler.generated.HighArityContinuation.recursive() co |
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.HighArityVirtualThreadStackTest.retainsCarrierWithMoreThan128StackArgumentWords()[1]`, which covers() cannot match exactly; this entry mutes the whole class
com.datadoghq.profiler.wallclock.HighArityVirtualThreadStackTest.* | PROF-XXXXX | 2026-10-28 | 2027-01-26 | musl-25-librca-debug-aarch64 | High-arity samples must retain exactly one carrier com.datadoghq.profiler.generated.HighArityContinuation.recursive() com.datadoghq.profiler.generated.HighArityContinuation.recursive() ==> expected: < (seen in: musl-25-librca-debug-aarch64)
Retried 1 of 31 cells.
Failed Jobs
- musl-aarch64/debug / 25-librca — view logs
Summary: Total: 32 | Passed: 30 | Failed: 1 | Cancelled: 1
Updated: 2026-10-28 16:58:23 UTC
…nto paul.fournillon/recover-frozen-virtual-thread-callers
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/146813861 |
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.
Nested continuation transitions can make frozen-stack recovery append suspended child callers instead of parent callers and incorrectly report the trace as complete.
🤖 Bits Code Review · Commit 746acbe
| // carrier Thread.cont, including when the current Java thread is virtual. | ||
| uintptr_t continuation = readOop(carrier, layout.carrier_cont); | ||
| for (unsigned int i = 0; i < continuation_depth && continuation != 0; i++) { | ||
| continuation = readOop(continuation, layout.cont_parent); |
There was a problem hiding this comment.
When a partially thawed parent resumes a nested continuation, Continuation.run publishes the child in Thread.cont before enterSpecial and restores the parent only after it returns. Samples in those Java intervals pass the recovery guards, but the first native return barrier belongs to the parent. Reading Thread.cont therefore selects the child's chunks, potentially appending suspended child frames, omitting parent callers, and clearing truncation. Match recovery to the crossed native continuation boundary, or reject recovery during these transitions. Correctly identifying that boundary requires broader changes, so no localized suggestion is provided.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
…nto paul.fournillon/recover-frozen-virtual-thread-callers
This comment has been minimized.
This comment has been minimized.
…nto paul.fournillon/recover-frozen-virtual-thread-callers
There was a problem hiding this comment.
More details
Local JDK checks supported the stack-chunk layout and deoptimization metadata assumptions used to recover frozen callers. The completed static review identified no concrete regression within the examined scope.
🤖 Bits Code Review · Commit af03bf7
Preserve heap recovery at verified frozen boundaries while keeping speculative crossings truncated. Combine carrier and deoptimization metadata, retain SP-based carrier recovery, and adapt carrier regressions to accept restored outer callers.
Clear live and saved Java-frame anchors when crossing a continuation boundary so fallback recovery cannot duplicate continuation roots or carrier frames. Add deterministic regressions and report the first invalid stack in the virtual-thread chaos test.
Account for C1 temporary argument and save areas when recovering compiled callers. Use live LR for native DWARF entry and restored-return rules, and stop with truncation at rt_sigreturn when signal-frame CFI is unavailable. Add native regressions for caller stack adjustment, stale LR slots and unsupported signal-frame walks. Correct synthetic CFI fixtures to declare their saved return addresses.
… paul.fournillon/recover-frozen-virtual-thread-callers
There was a problem hiding this comment.
More details
Frozen-stack reads require Java thread state and an interrupted Java-body PC, matching the safeguards described in the human PR discussion. Incomplete recovery preserves truncation.
🤖 Bits Code Review · Commit 833aceb
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| /** Concurrently relocating collectors retain the safe incomplete-stack fallback. */ | ||
| @TestTemplate | ||
| @ValueSource(strings = {"vm", "vmx"}) | ||
| public void preservesTruncationWhenRecoveryUnavailable(@CStack String cstack) throws Exception { |
| /** Heap walking must obey the same capture limit and truncation contract as native walking. */ | ||
| @TestTemplate | ||
| @ValueSource(strings = {"vm", "vmx"}) | ||
| public void respectsConfiguredDepth(@CStack String cstack) throws Exception { |
| /** A JNI call may be sampled while older callers are frozen, but raw heap reads are unsafe. */ | ||
| @TestTemplate | ||
| @ValueSource(strings = {"vm", "vmx"}) | ||
| public void preservesTruncationDuringNativeCalls(@CStack String cstack) throws Exception { |
What does this PR do?:
Recover frozen Java callers from HotSpot stack chunks at a verified continuation boundary. Restore deoptimized and inlined callers while preserving truthful truncation when recovery is incomplete.
Motivation:
A partial thaw leaves older Java callers in the heap. Recovering carrier frames alone cannot restore that ancestry. This change appends the frozen callers before the continuation root or carrier walk.
Additional Notes:
How to test the change?:
Validation binds to source tree
15c3173cc4a7a3af70ce38c6c333d01c7aad597a:For Datadog employees: