Skip to content

chaos: vthread-context-cascade antagonist + glibc heap-corruption defaults#642

Draft
jbachorik wants to merge 12 commits into
mainfrom
jb/vt_churn
Draft

chaos: vthread-context-cascade antagonist + glibc heap-corruption defaults#642
jbachorik wants to merge 12 commits into
mainfrom
jb/vt_churn

Conversation

@jbachorik

@jbachorik jbachorik commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?:
Adds a new vthread-context-cascade chaos antagonist that races tracer-style
context propagate/activate/restore semantics against virtual-thread carrier
churn, targeting the ContextStorageMode.THREAD stale-carrier
DirectByteBuffer use-after-free. Also defaults MALLOC_CHECK_/
MALLOC_PERTURB_ for glibc-backed test runs (chaos reliability runner and
the shared ddprof-test Gradle test-task configuration), turning silent
heap corruption into an immediate, attributable SIGABRT instead of a
crash much later in an unrelated allocation.

Also fixes a real J9-only bug (PROF-15364) uncovered while chasing the
MALLOC_CHECK_ default's fallout: FrameType::isRawPointer() (frame.h)
misidentified raw, unencoded J9 ASGCT BCIs as HotSpot raw-pointer-encoded
frames, routing them into JVMSupport::resolve()'s assert(false) for
non-HotSpot VMs — a debug-only SIGABRT. This is required for this PR's own
J9 debug CI config to pass; outside this branch/config (i.e. before the
MALLOC_CHECK_ default made the J9 debug suite deterministic) it wasn't
easily reproducible.

Motivation:
While building the vthread-context-cascade reproducer it became clear
that this class of use-after-free bug corrupts glibc heap/tcache metadata
silently and only manifests as a crash far later, in an unrelated
allocation call — making it unlikely to be caught within a short chaos
run or CI test window. Enabling glibc's own heap-corruption checks by
default closes that gap cheaply, without needing a sanitizer build (which
can't run against a live JVM, see reference_sanitizers_no_jvm).

Additional Notes:
The MALLOC_CHECK_/MALLOC_PERTURB_ defaults are skipped on musl (no
MALLOC_CHECK_ support) and whenever a sanitizer/allocator already
replaces malloc via LD_PRELOAD (ASan/TSan/tcmalloc/jemalloc configs),
mirroring the existing gmalloc vs tcmalloc/jemalloc mutual
exclusivity in chaos_check.sh.

The isRawPointer() fix (frame.h) makes the check also require
ENCODED_MASK (bit 20) before trusting RAW_POINTER_MASK (bit 30) —
RAW_POINTER_MASK is only ever set by encode(..., rawPointer=true),
which is HotSpot-only and always sets ENCODED_MASK too, so a raw
unencoded ASGCT BCI (used on J9) can no longer false-positive just because
it happens to have bit 30 set. PROF-15360 (originally filed as suspected
"silent glibc heap corruption on J9") is closed in favor of PROF-15364,
which documents this corrected root cause.

How to test the change?:

  • ./gradlew :ddprof-stresstest:chaosJar builds cleanly with the new antagonist registered.
  • ./gradlew :build-logic:conventions:compileKotlin compiles cleanly with the ProfilerTestPlugin change.
  • Ran the antagonist standalone in a JDK21 container (-Dddprof.debug.context.storage.mode=thread, tuned virtual-thread scheduler flags, MALLOC_CHECK_=3/MALLOC_PERTURB_ set) for several minutes; no crash reproduced yet in this window — the antagonist is being landed as a reliability-suite addition to keep exercising this race over longer soak runs, not as a guaranteed-repro smoke test.
  • Added FrameTypeIsRawPointerTest.FalseForRawAsgctBciWithBit30SetButNoEncodedMarker to frame_ut.cpp (regression test for the isRawPointer() fix); ddprof-lib:gtestDebug_frame_ut passes 18/18, including the pre-existing TrueWhenBit30IsSet case (no regression on the legitimate HotSpot path). ./gradlew :ddprof-lib:buildDebug -x test succeeds.
  • Originally reproduced the J9 debug SIGABRT deterministically in a glibc/jdk8/j9/aarch64 debug container (Assertion 'false && "Should not reach here"' failed at jvmSupport.inline.h:34) before this fix — this is the repro that led to filing and fixing PROF-15364.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a security review (run the dd:platform-security-review
    skill, or file a request via the PSEC review form).
    bewaire also runs automatically on every PR.

  • This PR doesn't touch any of that.

  • JIRA: PROF-15364 (fixed by this PR); closes [PROF-15360]

Unsure? Have a question? Request a review!

…ALLOC_PERTURB_ for glibc test runs

Antagonist races tracer-style context propagation against virtual-thread
carrier churn to target the ContextStorageMode.THREAD stale-carrier
use-after-free. Since such heap corruption manifests silently until an
unrelated later allocation, also turn on glibc's own corruption checks
by default for chaos runs and ddprof-test suites (skipped on musl and
whenever a sanitizer/allocator already owns malloc via LD_PRELOAD).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jbachorik jbachorik added the AI label Jul 8, 2026
@dd-octo-sts

dd-octo-sts Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #29116987971 | Commit: 202bdbc | Duration: 13m 25s (longest job)

10 of 32 test jobs failed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Failed Tests

musl-amd64/debug / 25-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-amd64/debug / 8-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-aarch64/debug / 21-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-aarch64/debug / 25-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-amd64/debug / 17-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-aarch64/debug / 11-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-amd64/debug / 21-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-amd64/debug / 11-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-aarch64/debug / 8-librca

Job: View logs

No detailed failure information available. Check the job logs.

musl-aarch64/debug / 17-librca

Job: View logs

No detailed failure information available. Check the job logs.

Summary: Total: 32 | Passed: 22 | Failed: 10


Updated: 2026-07-10 19:34:20 UTC

J9 hits a pre-existing, unrelated glibc heap-corruption bug that MALLOC_CHECK_ now surfaces as a hard abort; gate it off until that's investigated separately.
@dd-octo-sts

dd-octo-sts Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

1 failure(s) detected Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/123849937

❌ chaos: profiler tcmalloc amd64 21 0 3 temXchaos
Chaos harness crashed (RC=124)

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pipelines  Tests

Fix all issues with BitsAI

⚠️ Warnings

🚦 14 Pipeline jobs failed

CI Run | test-matrix / test-linux-musl-amd64 (11-librca, debug)   View in Datadog   GitHub Actions

CI Run | test-matrix / test-linux-musl-amd64 (17-librca, debug)   View in Datadog   GitHub Actions

CI Run | test-matrix / test-linux-musl-amd64 (21-librca, debug)   View in Datadog   GitHub Actions

View all 14 failed jobs.

ℹ️ Info

🔄 Datadog auto-retried 3 jobs - 0 passed on retry View in Datadog

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: bf5c239 | Docs | Datadog PR Page | Give us feedback!

jbachorik added 3 commits July 9, 2026 10:00
isRawPointer() only checked RAW_POINTER_MASK, letting raw J9 ASGCT BCIs
that never went through encode() misroute into JVMSupport::resolve(),
which asserts false for non-HotSpot VMs. Require ENCODED_MASK too.
It was registered in Main.java's antagonist factory but never added to
either config's ANTAGONISTS string, so it silently never ran in CI.
@jbachorik jbachorik marked this pull request as ready for review July 9, 2026 09:04
@jbachorik jbachorik requested a review from a team as a code owner July 9, 2026 09:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1dd0fb1c7

ℹ️ 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".

Include ACTIVE_PHASE_MILLIS in STALE_RACER_SLEEP_MILLIS so racers wake as
the next active phase begins instead of ~3s early. Bound stopGracefully's
total wait to the caller's timeout via a shared deadline instead of giving
each join/tryAcquire the full budget independently.
@jbachorik jbachorik marked this pull request as draft July 9, 2026 13:58
jbachorik and others added 3 commits July 9, 2026 18:10
…ntext via @trace

driverLoop() called JavaProfiler.getInstance() directly, but JavaProfiler is a
chaosCompileOnly dependency never bundled into chaos.jar and not visible to the
app classloader (the agent's copy is shaded/relocated). The resulting
NoClassDefFoundError is an Error, not an Exception, so it escaped the
catch (Exception e) block and silently killed the driver thread — leaving
activePhase stuck true and letting the sibling pinningChurnLoop run
full-throttle for the rest of the test instead of its intended
active/quiet duty cycle, which is the likely driver behind the OOM seen in
reliability-chaos-aarch64 [profiler, gmalloc, 21.0.3-tem].

Rewrite the antagonist to only touch the tracer through @Trace-annotated
methods (the same proven-safe dd-trace-api compileOnly pattern already used
by TraceContextAntagonist), letting the real tracer and its virtual-thread
instrumentation drive setContext/clearContext instead of calling
com.datadoghq.profiler.* directly. Drops the custom-attribute/baggage
simulation in favor of nested @trace hops plus forceUnmount and
stale-buffer racer threads that still race OtelContextStorage's
thread-scoped fallback across carrier churn.

Verified with a 90s live run (JDK 21.0.11, profiler+tracer, gmalloc): no
NoClassDefFoundError, no exceptions, completed cleanly with RC=0.

Environment: Datadog workspace

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Without a RUNTIME arg, chaos_check.sh silently proceeds with an empty
value, which becomes --duration s in the chaos harness invocation and
fails 15+ seconds later — after the JDK/agent/jar setup — with a
NumberFormatException buried in the harness output, reported generically
as "FAIL:Chaos harness crashed (RC=1)". Check for a missing RUNTIME right
after arg parsing and fail immediately with a clear usage message.

Environment: Datadog workspace

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dd-octo-sts

dd-octo-sts Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 3e379a7)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/123961967 Commit: 3e379a760b7320f2334eb3fbfafbedc8347c0e2f

✅ Within expected boundaries

No significant runtime deltas (all within run-to-run noise) and no internal-counter outliers.

Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10276 ms (21 iters) ✅ 10301 ms (21 iters) ≈ +0.2% (±11.2%) — / —
akka-uct 25 ✅ 8891 ms (24 iters) ✅ 8795 ms (24 iters) ≈ -1.1% (±10%) — / —
finagle-chirper 21 ✅ 5962 ms (33 iters) ✅ 6006 ms (33 iters) ≈ +0.7% (±25.6%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5428 ms (36 iters) ✅ 5434 ms (36 iters) ≈ +0.1% (±24.4%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2833 ms (66 iters) ✅ 2761 ms (68 iters) ≈ -2.5% (±2.6%) — / —
fj-kmeans 25 ✅ 2813 ms (66 iters) ✅ 2796 ms (66 iters) ≈ -0.6% (±2.6%) — / —
future-genetic 21 ✅ 2048 ms (90 iters) ✅ 2066 ms (90 iters) ≈ +0.9% (±2.6%) — / —
future-genetic 25 ✅ 2003 ms (92 iters) ✅ 2019 ms (92 iters) ≈ +0.8% (±2.8%) — / —
naive-bayes 21 ✅ 1291 ms (133 iters) ✅ 1323 ms (131 iters) ≈ +2.5% (±33.2%) — / —
naive-bayes 25 ✅ 1015 ms (168 iters) ✅ 1020 ms (168 iters) ≈ +0.5% (±31.9%) — / —
reactors 21 ✅ 16390 ms (15 iters) ✅ 16273 ms (15 iters) ≈ -0.7% (±7.4%) — / —
reactors 25 ✅ 18523 ms (15 iters) ✅ 18461 ms (15 iters) ≈ -0.3% (±4.5%) — / —
Internal counter details (ddprof)

ddprof internal counters, latest / dev (✅ = 0, · = unavailable):

Benchmark JDK Dropped rec Dropped jvmti Dropped trace Skipped WC AGCT fail Unwind fail
akka-uct 21 ✅ / ✅ ✅ / ✅ 2 / 3 2026 / 1886 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 3 2286 / 2215 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 1 / 1 8524 / 8931 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ ✅ / 1 8465 / 8838 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 4 / 1 1249 / 1268 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 3 / 4 1276 / 1265 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 1 / 1 2967 / 3001 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 1 / 2 2906 / 2864 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 3 / 3 3482 / 3544 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 4 / 2 3463 / 3508 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ ✅ / 1 1674 / 1652 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / ✅ 1937 / 1857 ✅ / ✅ ✅ / ✅

@rkennke

rkennke commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Not a review, but one thing that I can't quickly see from looking at the code: does it trigger actual migrations of vthreads across carriers? Because that is the fault scenario that led to the heap corruption.

@jbachorik

Copy link
Copy Markdown
Collaborator Author

@rkennke It is trying to - but it is very probabilistic. It is trying to hold back carrier threads so new ones have to be spawned, then let the old one die so the vthread will have to be remounted but DBB still point to now invalid memory.
TBH, I haven't really got any invalid access, so the test might not be completely successful in doing that :/

@rkennke

rkennke commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

J9 MALLOC_CHECK_ skip: stale ticket reference + open question

The comment justifying the J9 skip in ProfilerTestPlugin.kt contradicts this PR's own narrative, and it's the shipped comment (not just the description) that carries the problem:

// ... skip on J9 (its own allocator usage trips a pre-existing, unrelated heap
// corruption that needs separate investigation — see PROF-15360).

Two issues:

  1. Stale ticket reference. This cites PROF-15360 as an open "needs separate investigation" item, but the PR description says PROF-15360 is closed — and closed precisely because it wasn't heap corruption at all; the real root cause was the isRawPointer() debug-assert SIGABRT that this same PR fixes (PROF-15364). So the comment describes the fixed bug as an unrelated, still-open one.

  2. The stale justification is load-bearing. It's the stated reason for skipping MALLOC_CHECK_/MALLOC_PERTURB_ on J9 — a real behavioral decision. So this isn't cosmetic: it drives whether J9 test runs get heap-corruption checking at all.

That raises a substantive question: if the only known J9 problem was PROF-15360 and it's fixed here, is the J9 skip still needed? Either

  • there's a genuinely separate J9 heap issue → then point the comment at a real open ticket with an accurate description; or
  • there isn't → then the skip is suppressing MALLOC_CHECK_ coverage on exactly the VM where the recent trouble surfaced, and should probably be dropped so J9 gets the same checking as glibc/HotSpot.

Could you reconcile the comment and confirm which case applies before merge?

@rkennke

rkennke commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@rkennke It is trying to - but it is very probabilistic. It is trying to hold back carrier threads so new ones have to be spawned, then let the old one die so the vthread will have to be remounted but DBB still point to now invalid memory. TBH, I haven't really got any invalid access, so the test might not be completely successful in doing that :/

I see. If you have the time/nerve(/AI tokens) for it, you could perhaps try using a custom FJP where you can control the thread lifecycles. Probabilistic is fine, though, better than nothing.

@jbachorik

Copy link
Copy Markdown
Collaborator Author

@rkennke That's a great idea. Thanks!

@jbachorik jbachorik added the sphinx:critical Sphinx: critical — human review required label Jul 10, 2026
Extract chaos_check.sh's logic into utils/run-chaos-harness.sh for local
repro. Add a #ifdef DEBUG-only watchdog in ProfiledThread's free path
(threadLocalData.cpp) that detects writes into freed OTel-context memory,
and beef up VirtualThreadContextCascadeAntagonist's stale-carrier racing
(continuous decoupled driver, randomized race window, dead-code removal).

Fixes from review: Main.java antagonist-start try/finally, watcher thread
signal-mask inheritance, racer-loop backoff under saturation, redundant
frame_ut.cpp assertions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI sphinx:critical Sphinx: critical — human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants