Skip to content

Fix reference-chain discovery and leak test prerequisites - #889

Draft
kaahos wants to merge 3 commits into
mainfrom
paul.fournillon/fix-reference-chain-release-failures
Draft

kaahos wants to merge 3 commits into
mainfrom
paul.fournillon/fix-reference-chain-release-failures

Conversation

@kaahos

@kaahos kaahos commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?:

Fix native reference-chain discovery and correct two test-fixture prerequisites:

  • Register candidates before the first heap walk can finish and release its tags. Let later allocating threads enter the bounded thread summary without inheriting an evicted thread's counts or ages.
  • Keep observed collections eligible for bounded revisits after reparenting. Index static-field holders discovered through another object, and JNI global roots on both first admission and rediscovery. Deduplicate overlapping FIFO selections.
  • Preserve class histories explicitly seeded by debug JNI test hooks across real GC folds. An explicit DEBUG-only seed method owns this bookkeeping. The new class-history field and preservation branch are compiled out of release builds; the production recorder retains its original signature without a synthetic_sample parameter. Ordinary production histories retain their normal updates and decay.
  • Increase the external cache fixture's inline padding from 32 to 128 longs. Preserve its one-shot allocator threads, allocation schedule, object count limit, and strict chain assertions.

Motivation:

Release validation exposed missing reference chains and incomplete holder-index registration. Native regressions reproduce the missing admission paths without depending on timing.

The remaining macOS failures also exposed fixture problems. Real GC updates could overwrite a debug test's seeded class signal. In a captured release failure, all 63 sampled CachedPayload survivors were spread across workers with at most six samples each; the detector requires eight for these one-age cohorts. Waiting cannot create the missing allocation samples. The original 272-byte payload was therefore an unreliable positive fixture at the 256KiB sampling interval. With 1,040-byte payloads in the same JVM, the captured passing run had 20 workers with at least eight samples. Padding stays on the payload itself, avoiding a competing array allocation.

Additional Notes:

No chain assertion, allocator-thread pattern, allocation-round count, GC-round count, timeout, traversal budget, detection threshold, test tag, or quarantine is relaxed. The fixture change increases retained bytes per object; the object graph and maximum object count are preserved. The small-cohort rejection tests remain in place. Thread summaries remain bounded and approximate.

How to test the change?:

Added native regressions for candidate registration order, late allocating threads, persistent collection revisits after mutation and reparenting, static/JNI-global admission paths, and seeded class-history preservation across real GC folds. Negative checks cover invented counts, non-collection eligibility, and ordinary production-history decay. The bug regressions failed before their respective fixes.

Latest full Linux validation on commit ba9328222:

  • Linux ARM64/glibc, Oracle GraalVM 17.0.12+8.1. The container's commit and all 822 tracked regular files matched the PR worktree before and after testing.
  • Full slow debug: 19 passed, no skips or failures.
  • Full slow release: 18 passed, one skipped, no failures.
  • Full regular release: 203 passed, 46 skipped, no failures.
  • Complete native debug and release configurations: all 82 test executables ran successfully in each configuration, with no skipped native tasks.
  • The complete wrapper invocation succeeded in 11m 51s, with no test filters, exclusion flags, skip-tests or skip-gtest properties:
./.claude/commands/build-and-summarize \
  :ddprof-test:testSlowDebug --rerun \
  :ddprof-test:testSlowRelease --rerun \
  :ddprof-test:testRelease --rerun \
  -PkeepJFRs --no-daemon --continue

The reported Java skip counts are retained as skips, including unmet JVM-extension and DEBUG-hook assumptions. There were no source changes or manual retries during this complete Linux run. Full logs, XML reports and source hashes are preserved locally.

Earlier validation also confirmed that the new class-history field, seed method and preservation branch are absent from release compiler output, and that ordinary-history decay remains covered in debug and release. Formatting and git diff --check passed.

Intermittency remains a limitation: an earlier full Linux slow-release run failed the thread-created-after-recording-start correlation case; this fresh full run did not reproduce it. Earlier macOS full runs also had missing-chain failures followed by passing focused reruns. The debug-only refactor does not establish that every intermittent failure is fixed. At the user's request, macOS is outside the latest release validation.

For Datadog employees:

  • This PR doesn't touch code that signs or publishes builds or packages, or handles credentials.

@dd-octo-sts

dd-octo-sts Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #40012168750 | Commit: 3b9c706 | Duration: 24m 45s (longest job)

✅ All 76 test jobs passed

Status Overview

JDK glibc-aarch64/asan glibc-aarch64/debug glibc-aarch64/tsan glibc-amd64/asan glibc-amd64/debug glibc-amd64/tsan 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

Summary: Total: 76 | Passed: 76 | Failed: 0


Updated: 2026-10-23 20:09:37 UTC

@jbachorik jbachorik added test:asan Run CI tests with AddressSanitizer configuration test:tsan Run CI tests with ThreadSanitizer configuration test:fuzz labels Oct 23, 2026
@dd-octo-sts

dd-octo-sts Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 ba932822

@kaahos kaahos changed the title Fix reference-chain discovery for changing caches and later allocating threads Fix reference-chain discovery and leak test prerequisites Oct 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:asan Run CI tests with AddressSanitizer configuration test:fuzz test:tsan Run CI tests with ThreadSanitizer configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants