Skip to content

Commit 739d86f

Browse files
jbachorikclaude
andcommitted
Fix GetLineNumberTableLeakTest to detect JVMTI leaks
Test now checks Internal NMT category (where JVMTI allocations appear) with profiler restarts and tighter thresholds to catch line table leaks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent d8978d6 commit 739d86f

3 files changed

Lines changed: 155 additions & 126 deletions

File tree

.claude/settings.local.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
"Bash(jj status:*)",
2424
"Bash(jj diffedit:*)",
2525
"Bash(jj restore:*)",
26-
"Bash(jj squash:*)"
26+
"Bash(jj squash:*)",
27+
"Bash(jj bookmark set:*)",
28+
"Bash(jj git push:*)"
2729
],
2830
"deny": [],
2931
"ask": []

ddprof-test/src/test/java/com/datadoghq/profiler/memleak/GetLineNumberTableLeakTest.java

Lines changed: 107 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -73,188 +73,170 @@ public void testGetLineNumberTableNoLeak() throws Exception {
7373
"Baseline NMT: malloc=%d KB, Internal=%d KB",
7474
baseline.mallocKB, baseline.internalReservedKB));
7575

76-
// Phase 1: Warmup - generate unique classes to populate _method_map
77-
System.out.println("Phase 1: Warmup - generating unique classes...");
78-
final int warmupFlushes = 100;
79-
final int steadyStateFlushes = 200;
80-
final int checkpointInterval = 50;
76+
// Phase 1: Warmup - generate many unique classes with ASM to populate _method_map
77+
System.out.println("Phase 1: Warmup - generating many unique classes via ASM...");
78+
final int warmupClassGenerations = 20; // Generate classes in batches
79+
final int totalRestarts = 25; // Need 25 restarts for 5 checkpoint intervals (5 restarts each)
80+
final int checkpointInterval = 5; // Check every 5 restarts
8181

82-
// Track snapshots to detect super-linear growth
82+
// Track snapshots: baseline + afterWarmup + 5 checkpoint intervals = 7 total
8383
NativeMemoryTracking.NMTSnapshot[] checkpoints = new NativeMemoryTracking.NMTSnapshot[7];
8484
checkpoints[0] = baseline;
8585
int checkpointIndex = 1;
8686

87-
// Cache the generated classes for reuse in steady state
88-
Class<?>[] cachedClasses = new Class<?>[warmupFlushes * 5];
87+
// Cache many generated classes for reuse in steady state
88+
// generateUniqueMethodCalls() returns 5 classes, each with 20 methods
89+
// Total: 20 batches × 5 classes/batch = 100 classes with ~2,000 methods
90+
Class<?>[] cachedClasses = new Class<?>[warmupClassGenerations * 5];
8991
int cachedClassIndex = 0;
9092

91-
for (int i = 0; i < warmupFlushes; i++) {
92-
// Generate unique classes and cache them
93+
for (int i = 0; i < warmupClassGenerations; i++) {
94+
// Generate 5 unique classes per batch via ASM (each with 20 methods with line tables)
9395
Class<?>[] newClasses = generateUniqueMethodCalls();
9496
System.arraycopy(newClasses, 0, cachedClasses, cachedClassIndex, newClasses.length);
9597
cachedClassIndex += newClasses.length;
9698

9799
// Flush profiler to trigger method resolution and GetLineNumberTable calls
98100
dump(tempFile("warmup-" + i));
99101

100-
// Take checkpoint snapshots
101-
if ((i + 1) % checkpointInterval == 0) {
102-
NativeMemoryTracking.NMTSnapshot progress = NativeMemoryTracking.takeSnapshot();
103-
checkpoints[checkpointIndex++] = progress;
104-
105-
long mallocGrowthKB = progress.mallocKB - baseline.mallocKB;
106-
long internalGrowthKB = progress.internalReservedKB - baseline.internalReservedKB;
102+
if ((i + 1) % 20 == 0) {
107103
System.out.println(
108-
String.format(
109-
"After %d warmup flushes: malloc=%d KB (+%d KB), Internal=%d KB (+%d KB)",
110-
(i + 1),
111-
progress.mallocKB,
112-
mallocGrowthKB,
113-
progress.internalReservedKB,
114-
internalGrowthKB));
104+
String.format("Generated %d classes so far (%d batches)...", cachedClassIndex, i + 1));
115105
}
116106
}
117107

118-
// Phase 2: Steady state - reuse existing classes with profiler restarts
119-
// Stop/start cycles trigger SharedLineNumberTable destructors, exposing leaks
108+
// Take snapshot after warmup
109+
NativeMemoryTracking.NMTSnapshot afterWarmup = NativeMemoryTracking.takeSnapshot();
110+
checkpoints[checkpointIndex++] = afterWarmup;
111+
long warmupInternalGrowthKB = afterWarmup.internalReservedKB - baseline.internalReservedKB;
120112
System.out.println(
121113
String.format(
122-
"Phase 2: Steady state - reusing %d cached classes with profiler restarts...",
123-
cachedClassIndex));
124-
NativeMemoryTracking.NMTSnapshot afterWarmup = checkpoints[checkpointIndex - 1];
125-
126-
// Restart profiler every 10 flushes to trigger destructor calls
127-
final int flushesPerRestart = 10;
128-
int flushCount = 0;
114+
"After warmup: %d classes generated, Internal=%d KB (+%d KB)",
115+
cachedClassIndex,
116+
afterWarmup.internalReservedKB,
117+
warmupInternalGrowthKB));
118+
119+
// Phase 2: Steady state - 1000+ restarts to accumulate leak if present
120+
// Stop/start cycles trigger SharedLineNumberTable destructors
121+
// With bug: each restart leaks ~16 KB → 1000 restarts = ~16 MB detectable leak
122+
System.out.println(
123+
String.format(
124+
"Phase 2: Performing %d profiler restarts to test for accumulated leaks...",
125+
totalRestarts));
129126

130-
for (int restart = 0; restart < steadyStateFlushes / flushesPerRestart; restart++) {
127+
for (int restart = 0; restart < totalRestarts; restart++) {
131128
// Stop profiler to destroy Recording and trigger all SharedLineNumberTable destructors
132-
if (restart > 0) {
133-
profiler.stop();
134-
Thread.sleep(50); // Allow cleanup to complete
129+
profiler.stop();
130+
Thread.sleep(10); // Allow cleanup to complete
135131

136-
// Restart profiler (creates new Recording, repopulates _method_map from same classes)
137-
profiler.execute(
138-
"start," + getProfilerCommand() + ",jfr,file=" + tempFile("restart-" + restart));
139-
}
132+
// Restart profiler (creates new Recording, repopulates _method_map from same classes)
133+
profiler.execute(
134+
"start," + getProfilerCommand() + ",jfr,file=" + tempFile("restart-" + restart));
140135

141-
// Perform flushes with cached classes
142-
for (int i = 0; i < flushesPerRestart; i++) {
143-
flushCount++;
136+
// Reuse cached classes to trigger GetLineNumberTable again
137+
invokeCachedClasses(cachedClasses, restart);
144138

145-
// Reuse cached classes (cycle through them)
146-
invokeCachedClasses(cachedClasses, flushCount);
139+
// Single flush per restart
140+
dump(tempFile("steady-" + restart));
147141

148-
// Flush profiler
149-
dump(tempFile("steady-" + flushCount));
150-
}
151-
152-
// Take checkpoint snapshots at intervals
153-
if ((flushCount) % checkpointInterval == 0) {
142+
// Take checkpoint snapshots every 200 restarts
143+
if ((restart + 1) % checkpointInterval == 0) {
154144
NativeMemoryTracking.NMTSnapshot progress = NativeMemoryTracking.takeSnapshot();
155145
checkpoints[checkpointIndex++] = progress;
156146

157-
long mallocGrowthKB = progress.mallocKB - baseline.mallocKB;
158-
long mallocGrowthFromWarmupKB = progress.mallocKB - afterWarmup.mallocKB;
147+
long internalGrowthFromWarmupKB = progress.internalReservedKB - afterWarmup.internalReservedKB;
159148
long internalGrowthKB = progress.internalReservedKB - baseline.internalReservedKB;
160149
System.out.println(
161150
String.format(
162-
"After %d steady flushes (%d restarts): malloc=%d KB (+%d KB total, +%d KB from warmup), Internal=%d KB (+%d KB)",
163-
flushCount,
151+
"After %d restarts: Internal=%d KB (+%d KB from warmup, +%d KB total)",
164152
restart + 1,
165-
progress.mallocKB,
166-
mallocGrowthKB,
167-
mallocGrowthFromWarmupKB,
168153
progress.internalReservedKB,
154+
internalGrowthFromWarmupKB,
169155
internalGrowthKB));
170156
}
171157
}
172158

173159
// Take final snapshot
174-
NativeMemoryTracking.NMTSnapshot afterFlushes = checkpoints[6];
175-
long totalMallocGrowthKB = afterFlushes.mallocKB - baseline.mallocKB;
176-
long warmupMallocGrowthKB = afterWarmup.mallocKB - baseline.mallocKB;
177-
long steadyStateMallocGrowthKB = afterFlushes.mallocKB - afterWarmup.mallocKB;
178-
long internalGrowthKB = afterFlushes.internalReservedKB - baseline.internalReservedKB;
179-
180-
// Calculate growth rates for steady state intervals (should plateau)
181-
// checkpoints[2] = after 100 warmup flushes
182-
// checkpoints[3-6] = steady state at 50, 100, 150, 200 flushes
183-
long[] steadyStateGrowths = new long[4];
184-
for (int i = 0; i < 4; i++) {
185-
steadyStateGrowths[i] = checkpoints[i + 3].mallocKB - checkpoints[i + 2].mallocKB;
160+
NativeMemoryTracking.NMTSnapshot afterRestarts = checkpoints[6];
161+
long steadyStateInternalGrowthKB =
162+
afterRestarts.internalReservedKB - afterWarmup.internalReservedKB;
163+
long totalInternalGrowthKB = afterRestarts.internalReservedKB - baseline.internalReservedKB;
164+
165+
// Calculate Internal category growth rates for steady state intervals
166+
// checkpoints[1] = after warmup
167+
// checkpoints[2-6] = after 200, 400, 600, 800, 1000 restarts
168+
long[] steadyStateInternalGrowths = new long[5];
169+
for (int i = 0; i < 5; i++) {
170+
steadyStateInternalGrowths[i] =
171+
checkpoints[i + 2].internalReservedKB - checkpoints[i + 1].internalReservedKB;
186172
}
187173

188-
// Assert that steady state growth is minimal and doesn't accelerate
189-
// With fix: growth should be small (<5 MB) and stable even with restarts
190-
// Without fix: each restart triggers 10,000 method destructor leaks
191-
// Expected leak: 10,000 methods × 100 bytes × 20 restarts = ~20 MB
192-
long maxSteadyStateGrowth = 0;
193-
for (int i = 0; i < steadyStateGrowths.length; i++) {
194-
maxSteadyStateGrowth = Math.max(maxSteadyStateGrowth, steadyStateGrowths[i]);
174+
// Assert that Internal category doesn't show super-linear growth
175+
// With fix: Internal should plateau after warmup (< 2 MB per 200 restarts from minor JVM allocations)
176+
// Without fix: each restart leaks ~16 KB → 200 restarts = ~3.2 MB per interval
177+
long maxIntervalGrowth = 0;
178+
for (int i = 0; i < steadyStateInternalGrowths.length; i++) {
179+
maxIntervalGrowth = Math.max(maxIntervalGrowth, steadyStateInternalGrowths[i]);
195180

196181
System.out.println(
197182
String.format(
198-
"Steady state interval %d: +%d KB (includes %d profiler restarts)",
183+
"Interval %d (restarts %d-%d): Internal +%d KB",
199184
i + 1,
200-
steadyStateGrowths[i],
201-
checkpointInterval / flushesPerRestart));
202-
203-
// Assert individual intervals don't show excessive growth
204-
// With restarts, each interval includes 5 stop/start cycles
205-
// If leak exists: 5 restarts × 10,000 methods × 100 bytes = ~5 MB
206-
if (steadyStateGrowths[i] > 8 * 1024) { // 8 MB per interval
185+
i * checkpointInterval,
186+
(i + 1) * checkpointInterval,
187+
steadyStateInternalGrowths[i]));
188+
189+
// Assert individual intervals don't show excessive JVMTI leak growth
190+
// Threshold: 10 KB per 5 restarts
191+
// With fix: < 5 KB (minor allocations)
192+
// Without fix: ~10-20 KB per interval (line table leaks accumulate)
193+
if (steadyStateInternalGrowths[i] > 10) { // 10 KB per 5 restarts
207194
fail(
208195
String.format(
209-
"malloc growth in steady state is excessive (leak detected)!\n"
210-
+ "Steady state interval %d (flushes %d-%d with %d restarts): +%d KB\n"
211-
+ "Expected: malloc should stay flat (destructors properly deallocate)\n"
212-
+ "Actual: continued growth indicates leaked line number tables on destructor calls",
196+
"Internal category growth indicates JVMTI leak!\n"
197+
+ "Interval %d (restarts %d-%d): +%d KB\n"
198+
+ "Expected: Internal should plateau (< 2.5 MB per 200 restarts)\n"
199+
+ "Actual: continued growth indicates leaked GetLineNumberTable allocations",
213200
i + 1,
214-
(i) * 50,
215-
(i + 1) * 50,
216-
checkpointInterval / flushesPerRestart,
217-
steadyStateGrowths[i]));
201+
i * checkpointInterval,
202+
(i + 1) * checkpointInterval,
203+
steadyStateInternalGrowths[i]));
218204
}
219205
}
220206

221-
// Verify total steady state growth is bounded
222-
// With fix: should be < 10 MB (normal JVM operations like GC, minor allocations)
223-
// Without fix: 20 restarts × 10,000 methods × 100 bytes = ~20 MB leak
224-
NativeMemoryTracking.assertMallocMemoryBounded(
207+
// Verify total steady state Internal growth is bounded
208+
// With fix: should be < 20 KB total (minor JVM allocations over 25 restarts)
209+
// Without fix: 25 restarts × ~1.6 KB/restart = ~40 KB JVMTI leak (based on observed leak rate)
210+
NativeMemoryTracking.assertInternalMemoryBounded(
225211
afterWarmup,
226-
afterFlushes,
227-
15 * 1024 * 1024, // 15 MB - accounts for restarts but no leaks
228-
"malloc memory grew excessively during steady state - indicates potential leak!");
212+
afterRestarts,
213+
20 * 1024, // 20 KB - tight enough to catch 40 KB leak
214+
"Internal category grew excessively - JVMTI leak detected!");
229215

230216
// Print summary
231217
System.out.println(
232218
String.format(
233-
"\nTest completed successfully:\n"
234-
+ " Phase 1 (Warmup - unique classes):\n"
235-
+ " Flushes: %d\n"
236-
+ " malloc growth: +%d KB\n"
237-
+ " Phase 2 (Steady state - reused classes with restarts):\n"
238-
+ " Flushes: %d\n"
239-
+ " Profiler restarts: %d (triggers destructor calls)\n"
240-
+ " malloc growth: +%d KB (should plateau)\n"
241-
+ " Total:\n"
242-
+ " malloc: %d KB → %d KB (+%d KB, +%d allocs)\n"
243-
+ " Internal category: %d KB → %d KB (+%d KB)\n"
244-
+ " Note: JVMTI allocations tracked under Internal category → malloc\n"
245-
+ " Note: Restarts destroy Recording, triggering SharedLineNumberTable destructors",
246-
warmupFlushes,
247-
warmupMallocGrowthKB,
248-
steadyStateFlushes,
249-
steadyStateFlushes / flushesPerRestart,
250-
steadyStateMallocGrowthKB,
251-
baseline.mallocKB,
252-
afterFlushes.mallocKB,
253-
totalMallocGrowthKB,
254-
afterFlushes.mallocCount - baseline.mallocCount,
219+
"\n=== Test Completed Successfully ===\n"
220+
+ "Phase 1 (Warmup):\n"
221+
+ " Classes generated: %d (via ASM)\n"
222+
+ " Internal growth: +%d KB\n"
223+
+ "Phase 2 (Leak Detection):\n"
224+
+ " Profiler restarts: %d\n"
225+
+ " Internal steady state growth: +%d KB (threshold: < 20 KB)\n"
226+
+ " Max interval growth: %d KB per 5 restarts (threshold: < 10 KB)\n"
227+
+ "Total Internal: %d KB → %d KB (+%d KB)\n"
228+
+ "\n"
229+
+ "Result: No JVMTI memory leak detected\n"
230+
+ "Note: GetLineNumberTable allocations tracked in Internal NMT category\n"
231+
+ "Note: Each restart destroys Recording, triggering SharedLineNumberTable destructors",
232+
cachedClassIndex,
233+
warmupInternalGrowthKB,
234+
totalRestarts,
235+
steadyStateInternalGrowthKB,
236+
maxIntervalGrowth,
255237
baseline.internalReservedKB,
256-
afterFlushes.internalReservedKB,
257-
internalGrowthKB));
238+
afterRestarts.internalReservedKB,
239+
totalInternalGrowthKB));
258240
}
259241

260242
private int classCounter = 0;

ddprof-test/src/test/java/com/datadoghq/profiler/util/NativeMemoryTracking.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,51 @@ public static void assertMallocMemoryBounded(
187187
}
188188
}
189189

190+
/**
191+
* Asserts that Internal category memory growth between two snapshots is bounded.
192+
* JVMTI allocations (like GetLineNumberTable) appear in the Internal category.
193+
*
194+
* @param before snapshot taken before the operation
195+
* @param after snapshot taken after the operation
196+
* @param maxGrowthBytes maximum allowed growth in bytes
197+
* @param failureMessage message to display if assertion fails
198+
*/
199+
public static void assertInternalMemoryBounded(
200+
NMTSnapshot before, NMTSnapshot after, long maxGrowthBytes, String failureMessage) {
201+
long growth = after.getInternalReservedBytes() - before.getInternalReservedBytes();
202+
203+
if (growth > maxGrowthBytes) {
204+
String diagnostic =
205+
String.format(
206+
"Internal category memory grew by %d bytes (%.2f MB), exceeds limit of %d bytes (%.2f MB)\n"
207+
+ "Before: %d KB reserved, %d KB committed\n"
208+
+ "After: %d KB reserved (+%d KB), %d KB committed (+%d KB)\n"
209+
+ "malloc before: %d KB (%d allocations)\n"
210+
+ "malloc after: %d KB (+%d KB, %d allocations, +%d)\n\n"
211+
+ "=== NMT Before ===\n%s\n\n"
212+
+ "=== NMT After ===\n%s",
213+
growth,
214+
growth / (1024.0 * 1024.0),
215+
maxGrowthBytes,
216+
maxGrowthBytes / (1024.0 * 1024.0),
217+
before.internalReservedKB,
218+
before.internalCommittedKB,
219+
after.internalReservedKB,
220+
after.internalReservedKB - before.internalReservedKB,
221+
after.internalCommittedKB,
222+
after.internalCommittedKB - before.internalCommittedKB,
223+
before.mallocKB,
224+
before.mallocCount,
225+
after.mallocKB,
226+
after.mallocKB - before.mallocKB,
227+
after.mallocCount,
228+
after.mallocCount - before.mallocCount,
229+
before.fullOutput,
230+
after.fullOutput);
231+
fail(failureMessage + "\n" + diagnostic);
232+
}
233+
}
234+
190235
/**
191236
* Asserts that JVMTI memory growth between two snapshots is bounded.
192237
* Legacy method - JVMTI category doesn't exist in modern JVMs.

0 commit comments

Comments
 (0)