Skip to content

Commit 2bd422e

Browse files
committed
Stop racing the BFS thread in the urgent-OOM gate test
The test asserted shouldRunPassForTest0() directly, but opening the gate spends the episode's one-shot urgency entitlement on the FIRST evaluation - and the BFS thread, whose cadence ramps to ~10ms once urgency latches, races the test's own call for it. The direct call only returned true when it won that race, failing deterministically on fast runners. Assert through the thread instead: seed the rising floor, then wait for referenceChainPassesRunForTest0() to advance. A pass only runs after shouldRunPass() returned true, and with an empty per-klass population table the urgent-OOM projection is the only possible trigger.
1 parent becb134 commit 2bd422e

1 file changed

Lines changed: 31 additions & 7 deletions

File tree

‎ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,22 @@ private static void assumeDebugBuild() {
6565
* Seeds ten heap-floor-ring samples rising fast enough that {@code secondsToOOM()}'s projection
6666
* lands well under {@code OOM_URGENT_THRESHOLD_S} (300s), with LivenessTracker's per-klass
6767
* population table left empty throughout - {@code selectLeakCandidateKlassIds0()} returns
68-
* nothing at any point in this test. Asserts the search-restart gate still opens, proving the
69-
* urgent-OOM projection alone - not a per-klass candidate - is what let it through.
68+
* nothing at any point in this test. Then waits for the BFS thread to actually run passes,
69+
* proving the search-restart gate opened from the urgent-OOM projection alone.
70+
*
71+
* <p>The gate is asserted through the live BFS thread rather than by calling
72+
* {@code shouldRunPassForTest0()} directly: opening the gate spends the episode's one-shot
73+
* urgency entitlement ({@code _urgent_search_spent}) on the FIRST evaluation, and the BFS
74+
* thread - whose cadence ramps down to ~10ms once urgency latches - races the test's own
75+
* call for that first evaluation. A direct call therefore only returns true when it wins
76+
* that race, which made this test flaky by construction. A pass only ever runs after
77+
* {@code shouldRunPass()} returned true, and with {@code generations=true} plus a provably
78+
* empty per-klass population table the urgent-OOM projection is the only possible trigger -
79+
* so "passes ran while zero candidates existed" is the same assertion, without the race.
7080
*/
7181
@Test
72-
public void shouldOpenSearchGateOnAggressiveHeapWideGrowthWithNoLeakCandidate() {
82+
public void shouldOpenSearchGateOnAggressiveHeapWideGrowthWithNoLeakCandidate()
83+
throws InterruptedException {
7384
assumeDebugBuild();
7485
JavaProfiler.setHeapFloorRecordingForTest0(false);
7586
JavaProfiler.resetKlassPopulationForTest0();
@@ -87,12 +98,25 @@ public void shouldOpenSearchGateOnAggressiveHeapWideGrowthWithNoLeakCandidate()
8798

8899
int[] candidates = JavaProfiler.selectLeakCandidateKlassIds0();
89100
assertTrue(candidates == null || candidates.length == 0,
90-
"This test's own precondition: no per-klass candidate should exist, so a true result "
101+
"This test's own precondition: no per-klass candidate should exist, so passes running "
91102
+ "below can only come from the aggregate urgent-OOM bypass");
92103

93-
assertTrue(JavaProfiler.shouldRunPassForTest0(),
94-
"Expected the search-restart gate to open from the urgent heap-wide OOM projection "
95-
+ "alone, with zero per-klass leak candidate");
104+
int passesBefore = JavaProfiler.referenceChainPassesRunForTest0();
105+
// Idle BFS cadence is ~1s/pass; urgency ramps it to ~10ms after the latch. 20s covers the
106+
// first evaluation landing up to one idle cadence after the seeding loop.
107+
long deadline = System.currentTimeMillis() + 20_000;
108+
while (JavaProfiler.referenceChainPassesRunForTest0() == passesBefore
109+
&& System.currentTimeMillis() < deadline) {
110+
Thread.sleep(50);
111+
}
112+
assertTrue(JavaProfiler.referenceChainPassesRunForTest0() > passesBefore,
113+
"Expected the BFS thread to run passes from the urgent heap-wide OOM projection alone, "
114+
+ "with zero per-klass leak candidate");
115+
116+
candidates = JavaProfiler.selectLeakCandidateKlassIds0();
117+
assertTrue(candidates == null || candidates.length == 0,
118+
"The passes that just ran must still have had zero per-klass candidates - the "
119+
+ "aggregate urgent-OOM projection was the only possible trigger");
96120
} finally {
97121
JavaProfiler.setMaxHeapBytesForTest0(-1);
98122
JavaProfiler.setHeapFloorRecordingForTest0(true);

0 commit comments

Comments
 (0)