Conversation
This was referenced Sep 17, 2026
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
CI Test ResultsRun: #36580839436 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-29 14:32:15 UTC |
Contributor
This comment has been minimized.
This comment has been minimized.
jbachorik
added this pull request to stack #803
September 17, 2026 20:02
jbachorik
marked this pull request as ready for review
September 17, 2026 20:03
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. |
jbachorik
marked this pull request as draft
September 17, 2026 20:04
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
from
September 17, 2026 20:05
f01e23f to
00c4c92
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f01e23fc6a
ℹ️ 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".
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
A second recording can give one leak tag to two live entries. Later cleanup can write past the free-tag array and corrupt native memory.
🤖 Datadog Autotest · Commit f01e23f · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
2 times, most recently
from
September 17, 2026 20:19
00c4c92 to
4804259
Compare
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
2 times, most recently
from
September 17, 2026 21:01
a5c60d0 to
cfa59db
Compare
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
from
September 17, 2026 22:15
c71e5ab to
5fc0326
Compare
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
2 times, most recently
from
September 18, 2026 08:04
b2dafa0 to
1752c4f
Compare
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
from
September 18, 2026 12:59
959a2ef to
6dd03d9
Compare
The tag guard accepted tag == 2^31 (idx == INT_MAX), letting growLocked(idx + 1) overflow to INT_MIN, bypass the capacity check and write far out of bounds. Reject tag - 1 >= INT_MAX everywhere the guard appears. FrontierTable::insert() now advances _table_size under the same exclusive lock as the slot write, so a lock-ordered reader can never pass its idx < _table_size check on an unwritten, zeroed slot.
The leak-tag free list and info entries are now guarded by a dedicated SpinLock (lock order: _table_lock before _leak_tag_pool_lock, never the reverse) - acquireLeakTag() runs under the SHARED table lock while releaseLeakTag() runs under the EXCLUSIVE one, and getLeakTagInfo() takes no table lock at all, so the pool must not depend on either. cleanup_table() resolves at most 256 surviving entries' classes per sweep; entries past the budget fall back to their cached class id (the semantics the allow_resolve=false path already uses), bounding the exclusive-lock window instead of scaling it with survivor count. Document the enforced LT -> RCT lock order at the correlateAdmittedLeakTag() call site and the single-writer invariant for _table[i].leak_tag; fix the RingThirdsStats / corroborateRecentHalf comments to describe the regression actually computed; cache the heapFloorRising() result instead of re-evaluating it in the trailing TEST_LOG; make ClassTagAllocator::resetForTest() an atomic exchange.
buildCanaryChainEvent()'s parent_tag walk now bounds at the frontier's maxCapacity() like reconstructChain() and returns false on a cyclic or corrupt chain instead of spinning the poll thread; the walk's event._root_kind now comes from the terminal root-attached entry rather than the parent-side interior entry (always 0), matching the appendStaticFieldRootType(terminal) call and suppressChainEvent()'s transient-root gate. Remove the retired canary marker-tag mechanism end to end: nothing pre-tags candidates anymore, so the decode branches in heapReferenceCallback() and pollWatchedTargets() were unreachable - and runPass()'s marker-release loop was worse than dead: with every _candidate_tags[i] left at 0 it asked GetObjectsWithTags to enumerate ALL untagged objects once per candidate slot on every search stop. Also: read the container usage file from the hierarchy that supplied the winning limit instead of probing v2/v1 filenames in order; clamp PainBudget::drain() against a backward clock step; refuse symlinked or non-owned rc-debug override files under /tmp; resolve each container interface tag independently instead of failing the whole call; reset consume_tier_fair()'s cursor on a completed-but-unproductive lap; drop the pod fixture's dead placeholder assignment and assert the representative carries its leak tag. Tests cover the bounded walk, the deep-chain reconstruction, the symlink rejection and the cycle fail-safe.
Test .cpp files #include sibling .inc files; NativeCompileTask hashes inputs by content, so an .inc-only edit left the test binary stale and the change silently never ran.
A renamer thread cycles the knob path through valid/garbage/symlink/
directory/missing states via rename() while three readers hammer
readRcDebugLevelFile(): outcomes stay in {-1,0,1,2}, both a valid level
and a rejection are observed, and the open-descriptor count returns to
baseline (all early-return paths close the fd). The foreign-owner fstat
branch needs privileges to stage and stays covered statically.
Op-sequence libFuzzer target driving every public FrontierTable method directly, including paths a well-formed BFS pass never produces: invalid tags, capacity-schedule exhaustion, cycling/dangling parent chains, and predicate-violating re-parent attempts. Asserts the capacity contract, lookup fidelity (including the stale-bytes window after resetForRestart), reconstructChain soundness, and the improveChain/reparentToDurableRoot mutation guards, all against a model replica of the documented behavior.
A pass thread drives runPass()/restartSearch() the way the BFS threadLoop does, while GC callbacks run on their own thread and a third thread holds stopThread-style abort requests in 1ms windows. Asserts restarted searches start from zeroed accounting (passes, leak-tag counters, frontier size), the recording boundary clears resolved chains, GC epochs stay monotone, and the DEBUG t_inGCCallback asserts survive concurrent walks.
Drive isUrgent()'s latch/release machine with synthetic heap-floor ramps (ring seeding via the existing LivenessTracker test hooks): mid-band oscillation around the 300s threshold never flaps, release requires five consecutive clear-or-unknown observations, and a re-latch after release restores the urgency search entitlement. Assert shouldRunPass() raises the CPU pain-budget refill 100x exactly while a canary chase is open, and that the backoff gate holds passes unless the OOM ramp is active.
start() clears the resolved-chain cache and the pending abandoned-event queue so a new recording never re-emits the previous one's chains with class-dictionary ids from a wiped generation - seed both states and assert the real stop()/start() path drops them while the resolve path still works in the new recording.
Each consecutive CANARY_STUCK restart doubles the detector's pass limit (30 -> 60 -> 120, capped), and any other terminal outcome resets the escalation. Reachable only while urgent - the ordinary whole-graph stall branch has the same frontier-stall precondition and is checked first - so the test seeds a 224s-to-OOM projection the same way the OOM tests do. Adds abandon-reason, pending-abandoned-count, and stuck-limit test accessors.
The urgency latch survives the test (it releases only after five clear readings that never come) and keeps bypassing candidate gates for any test that runs after it under shuffled order; restartSearch() also leaves the seeded watched-leak-klass list in place. Reset the latch, watched list, and search state at the end. Also raise the compiler-availability probe timeout from 5s to 30s: on a loaded macOS host the /usr/bin/clang++ Xcode shim can exceed 5s to answer --version and the probe failed intermittently from invocations that accepted the same path minutes earlier.
fuzz_infra only runs on scheduled/manual pipelines and is allow_failure, so a fuzz target that stopped compiling survived an entire PR unnoticed - PR 797 broke fuzz_callTraceStorage that way, caught only by a manual run. Run :ddprof-lib:fuzz:buildFuzz on every branch push (failing if no binaries are produced, so a failed libFuzzer probe cannot silently void the gate), and soak the concurrency-dependent chaos suites x50 under TSan in the same job.
The fuzz binaries link to bin/fuzz/<name>/<name> - one directory deeper than the gate's find maxdepth allowed, so the gate failed after a fully successful buildFuzz (11 binaries linked). Mirror the Dockerfile.fuzz lookup (-maxdepth 2) that the fuzz_infra image build has always used.
The abort thread's fixed usleep holds made the flag/pass overlap a race: a fast runner pushes the pass thread through all 40 searches before the abort thread's first store lands, every pass's post-pass read sees the flag clear, and the abort-was-exercised guard fails with aborted_passes=0 (seen on the arm64 release CI runner; the walk itself was never at fault). Hold each abort window until the guard's own counter moves (bounded spin, early exit when the pass thread is done), and open the first window before the pass thread starts, so at least one post-pass read observes the request by construction.
jbachorik
force-pushed
the
jb/rc-3-refchain-tracker
branch
from
September 29, 2026 14:10
d51ab55 to
39a8c3c
Compare
Contributor
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/140947904 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
Implements the reference-chain engine itself:
ReferenceChainTracker(referenceChains.*): per-klass class tags, the frontier table of retained references, the BFS expansion thread, chain resolution/improvement into per-sampleReferenceChainEventpayloads, leak-tag correlation, and the search-gate/pain-budget scheduling.LivenessTracker(livenessTracker.*): per-klass population table, heap-floor ring with the time-to-OOM projection, and leak-candidate selection feeding the search gate.referencechainsArguments consumption, string-dictionary generation counter for class-tag cache invalidation, container-memory/os queries the projection needs, and the newcounters/rcDebugLevel/painBudget/classTagAllocatorheaders.Motivation:
Core of PROF-15341: attribute surviving live-heap samples to reference chains so leak candidates carry an actionable retention path.
Additional Notes:
Stacked on #796. Compiles and links standalone against the profiler API (only pre-existing
Profilermethods are called); the lifecycle wiring lands in the next PR of the stack. The largest review chunk in the series —referenceChains.cppis the engine,referenceChains.hdocuments its invariants.How to test the change?:
buildDebug -Pskip-testscompiles and links this layer; the C++ unit tests for all of the above are the next PRs in the stack.For Datadog employees: