Skip to content

Commit 5df7be9

Browse files
jbachorikmuse
jbachorik
and
muse
committed
address: fix rkennke review comments
- Remove dead `line_number_table = nullptr` (local var, no effect after Deallocate) - Extract AbstractDynamicClassTest base class; deduplicate generateClassBytecode, tempFile (now uses java.io.tmpdir), and IsolatedClassLoader from GetLineNumberTableLeakTest and WriteStackTracesAfterClassUnloadTest Co-Authored-By: muse <muse@noreply>
1 parent 5a0f2c4 commit 5df7be9

4 files changed

Lines changed: 85 additions & 148 deletions

File tree

‎ddprof-lib/src/main/cpp/flightRecorder.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method,
313313
if (dealloc_err != JVMTI_ERROR_NONE) {
314314
TEST_LOG("Unexpected error while deallocating linenumber table: %d", dealloc_err);
315315
}
316-
line_number_table = nullptr;
317316
if (owned_table != nullptr) {
318317
mi->_line_number_table = std::make_shared<SharedLineNumberTable>(
319318
line_number_table_size, owned_table);

‎ddprof-test/src/test/java/com/datadoghq/profiler/memleak/AbstractDynamicClassTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright 2026, Datadog, Inc
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*/
10+
package com.datadoghq.profiler.memleak;
11+
12+
import com.datadoghq.profiler.AbstractProfilerTest;
13+
import org.objectweb.asm.ClassWriter;
14+
import org.objectweb.asm.Label;
15+
import org.objectweb.asm.MethodVisitor;
16+
import org.objectweb.asm.Opcodes;
17+
18+
import java.io.IOException;
19+
import java.nio.file.Files;
20+
import java.nio.file.Path;
21+
import java.nio.file.Paths;
22+
23+
abstract class AbstractDynamicClassTest extends AbstractProfilerTest {
24+
25+
protected int classCounter = 0;
26+
27+
/**
28+
* Generates ASM bytecode for a class with a constructor and {@code methodCount} public
29+
* int-returning no-arg methods, each with multiple line-number table entries.
30+
*/
31+
protected byte[] generateClassBytecode(String className, int methodCount) {
32+
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
33+
cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, className, null, "java/lang/Object", null);
34+
35+
MethodVisitor ctor = cw.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
36+
ctor.visitCode();
37+
ctor.visitVarInsn(Opcodes.ALOAD, 0);
38+
ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
39+
ctor.visitInsn(Opcodes.RETURN);
40+
ctor.visitMaxs(0, 0);
41+
ctor.visitEnd();
42+
43+
for (int i = 0; i < methodCount; i++) {
44+
MethodVisitor mv = cw.visitMethod(Opcodes.ACC_PUBLIC, "method" + i, "()I", null, null);
45+
mv.visitCode();
46+
47+
Label l1 = new Label(); mv.visitLabel(l1); mv.visitLineNumber(100 + i * 3, l1);
48+
mv.visitInsn(Opcodes.ICONST_0);
49+
mv.visitVarInsn(Opcodes.ISTORE, 1);
50+
51+
Label l2 = new Label(); mv.visitLabel(l2); mv.visitLineNumber(101 + i * 3, l2);
52+
mv.visitVarInsn(Opcodes.ILOAD, 1);
53+
mv.visitInsn(Opcodes.ICONST_1);
54+
mv.visitInsn(Opcodes.IADD);
55+
mv.visitVarInsn(Opcodes.ISTORE, 1);
56+
57+
Label l3 = new Label(); mv.visitLabel(l3); mv.visitLineNumber(102 + i * 3, l3);
58+
mv.visitVarInsn(Opcodes.ILOAD, 1);
59+
mv.visitInsn(Opcodes.IRETURN);
60+
61+
mv.visitMaxs(0, 0);
62+
mv.visitEnd();
63+
}
64+
65+
cw.visitEnd();
66+
return cw.toByteArray();
67+
}
68+
69+
protected static Path tempFile(String prefix) throws IOException {
70+
Path dir = Paths.get(System.getProperty("java.io.tmpdir"), "recordings");
71+
Files.createDirectories(dir);
72+
return Files.createTempFile(dir, prefix + "-", ".jfr");
73+
}
74+
75+
protected static class IsolatedClassLoader extends ClassLoader {
76+
public Class<?> defineClass(String name, byte[] bytecode) {
77+
return defineClass(name, bytecode, 0, bytecode.length);
78+
}
79+
}
80+
}

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

Lines changed: 3 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,12 @@
1515
*/
1616
package com.datadoghq.profiler.memleak;
1717

18-
import com.datadoghq.profiler.AbstractProfilerTest;
1918
import org.junit.jupiter.api.Test;
20-
import org.objectweb.asm.ClassWriter;
21-
import org.objectweb.asm.Label;
22-
import org.objectweb.asm.MethodVisitor;
23-
import org.objectweb.asm.Opcodes;
2419

2520
import java.io.IOException;
2621
import java.lang.reflect.Method;
2722
import java.nio.file.Files;
2823
import java.nio.file.Path;
29-
import java.nio.file.Paths;
3024

3125
/**
3226
* Test to validate that method_map cleanup prevents unbounded memory growth in continuous profiling.
@@ -55,26 +49,14 @@
5549
* <li>Combined cleanup: method_map cleanup + class unloading</li>
5650
* </ul>
5751
*/
58-
public class GetLineNumberTableLeakTest extends AbstractProfilerTest {
52+
public class GetLineNumberTableLeakTest extends AbstractDynamicClassTest {
5953

6054
@Override
6155
protected String getProfilerCommand() {
6256
// Aggressive sampling to maximize method encounters and GetLineNumberTable calls
6357
return "cpu=1ms,alloc=512k";
6458
}
6559

66-
private int classCounter = 0;
67-
68-
/**
69-
* Custom ClassLoader for each dynamically generated class.
70-
* Using a new ClassLoader for each class ensures truly unique Class objects and jmethodIDs.
71-
*/
72-
private static class DynamicClassLoader extends ClassLoader {
73-
public Class<?> defineClass(String name, byte[] bytecode) {
74-
return defineClass(name, bytecode, 0, bytecode.length);
75-
}
76-
}
77-
7860
/**
7961
* Generates and loads truly unique classes using ASM bytecode generation.
8062
* Each class has multiple methods with line number tables to trigger GetLineNumberTable calls.
@@ -88,10 +70,10 @@ private Class<?>[] generateUniqueMethodCalls() throws Exception {
8870

8971
for (int i = 0; i < 5; i++) {
9072
String className = "com/datadoghq/profiler/generated/TestClass" + (classCounter++);
91-
byte[] bytecode = generateClassBytecode(className);
73+
byte[] bytecode = generateClassBytecode(className, 20);
9274

9375
// Use a fresh ClassLoader to ensure truly unique Class object and jmethodIDs
94-
DynamicClassLoader loader = new DynamicClassLoader();
76+
IsolatedClassLoader loader = new IsolatedClassLoader();
9577
Class<?> clazz = loader.defineClass(className.replace('/', '.'), bytecode);
9678
generatedClasses[i] = clazz;
9779

@@ -123,66 +105,6 @@ private void invokeClassMethods(Class<?> clazz) {
123105
}
124106
}
125107

126-
/**
127-
* Generates bytecode for a class with multiple methods.
128-
* Each method has line number table entries to trigger GetLineNumberTable allocations.
129-
*/
130-
private byte[] generateClassBytecode(String className) {
131-
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
132-
cw.visit(
133-
Opcodes.V1_8,
134-
Opcodes.ACC_PUBLIC,
135-
className,
136-
null,
137-
"java/lang/Object",
138-
null);
139-
140-
// Generate constructor
141-
MethodVisitor constructor =
142-
cw.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
143-
constructor.visitCode();
144-
constructor.visitVarInsn(Opcodes.ALOAD, 0);
145-
constructor.visitMethodInsn(
146-
Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
147-
constructor.visitInsn(Opcodes.RETURN);
148-
constructor.visitMaxs(0, 0);
149-
constructor.visitEnd();
150-
151-
// Generate 20 methods per class, each with line number tables
152-
for (int i = 0; i < 20; i++) {
153-
MethodVisitor mv =
154-
cw.visitMethod(Opcodes.ACC_PUBLIC, "method" + i, "()I", null, null);
155-
mv.visitCode();
156-
157-
// Add multiple line number entries (this is what causes GetLineNumberTable allocations)
158-
Label label1 = new Label();
159-
mv.visitLabel(label1);
160-
mv.visitLineNumber(100 + i, label1);
161-
mv.visitInsn(Opcodes.ICONST_0);
162-
mv.visitVarInsn(Opcodes.ISTORE, 1);
163-
164-
Label label2 = new Label();
165-
mv.visitLabel(label2);
166-
mv.visitLineNumber(101 + i, label2);
167-
mv.visitVarInsn(Opcodes.ILOAD, 1);
168-
mv.visitInsn(Opcodes.ICONST_1);
169-
mv.visitInsn(Opcodes.IADD);
170-
mv.visitVarInsn(Opcodes.ISTORE, 1);
171-
172-
Label label3 = new Label();
173-
mv.visitLabel(label3);
174-
mv.visitLineNumber(102 + i, label3);
175-
mv.visitVarInsn(Opcodes.ILOAD, 1);
176-
mv.visitInsn(Opcodes.IRETURN);
177-
178-
mv.visitMaxs(0, 0);
179-
mv.visitEnd();
180-
}
181-
182-
cw.visitEnd();
183-
return cw.toByteArray();
184-
}
185-
186108
/**
187109
* Comparison test that validates cleanup effectiveness by running the same workload twice: once
188110
* without cleanup (mcleanup=false) and once with cleanup (mcleanup=true, default). This provides
@@ -316,9 +238,4 @@ public void testCleanupEffectivenessComparison() throws Exception {
316238
}
317239
}
318240

319-
private Path tempFile(String name) throws IOException {
320-
Path dir = Paths.get("/tmp/recordings");
321-
Files.createDirectories(dir);
322-
return Files.createTempFile(dir, name + "-", ".jfr");
323-
}
324241
}

‎ddprof-test/src/test/java/com/datadoghq/profiler/memleak/WriteStackTracesAfterClassUnloadTest.java‎

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,14 @@
99
*/
1010
package com.datadoghq.profiler.memleak;
1111

12-
import com.datadoghq.profiler.AbstractProfilerTest;
1312
import org.junit.jupiter.api.Test;
1413
import org.junit.jupiter.api.Timeout;
15-
import org.objectweb.asm.ClassWriter;
16-
import org.objectweb.asm.Label;
17-
import org.objectweb.asm.MethodVisitor;
18-
import org.objectweb.asm.Opcodes;
1914

2015
import java.io.IOException;
2116
import java.lang.ref.WeakReference;
2217
import java.lang.reflect.Method;
2318
import java.nio.file.Files;
2419
import java.nio.file.Path;
25-
import java.nio.file.Paths;
2620

2721
import static org.junit.jupiter.api.Assertions.assertTrue;
2822
import static org.junit.jupiter.api.Assumptions.assumeTrue;
@@ -69,16 +63,14 @@
6963
* covered by {@code CleanupAfterClassUnloadTest}.</li>
7064
* </ul>
7165
*/
72-
public class WriteStackTracesAfterClassUnloadTest extends AbstractProfilerTest {
66+
public class WriteStackTracesAfterClassUnloadTest extends AbstractDynamicClassTest {
7367

7468
@Override
7569
protected String getProfilerCommand() {
7670
// Aggressive CPU sampling to ensure the dynamic class lands in many traces.
7771
return "cpu=1ms";
7872
}
7973

80-
private int classCounter = 0;
81-
8274
@Test
8375
@Timeout(60)
8476
public void testNoSigsegvInWriteStackTracesAfterClassUnload() throws Exception {
@@ -141,7 +133,7 @@ public void testNoSigsegvInWriteStackTracesAfterClassUnload() throws Exception {
141133

142134
private WeakReference<ClassLoader> loadAndProfileDynamicClass() throws Exception {
143135
String className = "com/datadoghq/profiler/generated/DynamicUnloadClass" + (classCounter++);
144-
byte[] bytecode = generateClassBytecode(className);
136+
byte[] bytecode = generateClassBytecode(className, 5);
145137

146138
IsolatedClassLoader loader = new IsolatedClassLoader();
147139
Class<?> clazz = loader.defineClass(className.replace('/', '.'), bytecode);
@@ -170,55 +162,4 @@ private WeakReference<ClassLoader> loadAndProfileDynamicClass() throws Exception
170162
return ref;
171163
}
172164

173-
private byte[] generateClassBytecode(String className) {
174-
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
175-
cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, className, null, "java/lang/Object", null);
176-
177-
MethodVisitor ctor = cw.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
178-
ctor.visitCode();
179-
ctor.visitVarInsn(Opcodes.ALOAD, 0);
180-
ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
181-
ctor.visitInsn(Opcodes.RETURN);
182-
ctor.visitMaxs(0, 0);
183-
ctor.visitEnd();
184-
185-
// Several methods, each with multiple line number entries — guarantees a non-trivial
186-
// line number table is allocated by JVMTI and stored in MethodInfo._line_number_table.
187-
for (int i = 0; i < 5; i++) {
188-
MethodVisitor mv = cw.visitMethod(Opcodes.ACC_PUBLIC, "method" + i, "()I", null, null);
189-
mv.visitCode();
190-
191-
Label l1 = new Label(); mv.visitLabel(l1); mv.visitLineNumber(100 + i * 3, l1);
192-
mv.visitInsn(Opcodes.ICONST_0);
193-
mv.visitVarInsn(Opcodes.ISTORE, 1);
194-
195-
Label l2 = new Label(); mv.visitLabel(l2); mv.visitLineNumber(101 + i * 3, l2);
196-
mv.visitVarInsn(Opcodes.ILOAD, 1);
197-
mv.visitInsn(Opcodes.ICONST_1);
198-
mv.visitInsn(Opcodes.IADD);
199-
mv.visitVarInsn(Opcodes.ISTORE, 1);
200-
201-
Label l3 = new Label(); mv.visitLabel(l3); mv.visitLineNumber(102 + i * 3, l3);
202-
mv.visitVarInsn(Opcodes.ILOAD, 1);
203-
mv.visitInsn(Opcodes.IRETURN);
204-
205-
mv.visitMaxs(0, 0);
206-
mv.visitEnd();
207-
}
208-
209-
cw.visitEnd();
210-
return cw.toByteArray();
211-
}
212-
213-
private static Path tempFile(String prefix) throws IOException {
214-
Path dir = Paths.get("/tmp/recordings");
215-
Files.createDirectories(dir);
216-
return Files.createTempFile(dir, prefix + "-", ".jfr");
217-
}
218-
219-
private static class IsolatedClassLoader extends ClassLoader {
220-
public Class<?> defineClass(String name, byte[] bytecode) {
221-
return defineClass(name, bytecode, 0, bytecode.length);
222-
}
223-
}
224165
}

0 commit comments

Comments
 (0)