Skip to content

Add a test for crash-recovery in the 'vm' stackwalker. #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbachorik merged 1 commit into from
May 27, 2025
Merged

Add a test for crash-recovery in the 'vm' stackwalker. #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ddprof-lib/src/main/cpp/profiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "arguments.h"
#include "callTraceStorage.h"
#include "codeCache.h"
#include "common.h"
#include "dictionary.h"
#include "engine.h"
#include "event.h"
Expand All @@ -36,6 +37,11 @@ __asm__(".symver exp,exp@GLIBC_2.17");
#endif
#endif

#ifdef DEBUG
#include <signal.h>
static const char* force_stackwalk_crash_env = getenv("DDPROF_FORCE_STACKWALK_CRASH");
Copy link
Contributor

@r1viollet r1viollet May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple but efficient 👍

#endif

const int MAX_NATIVE_FRAMES = 128;
const int RESERVED_FRAMES = 10; // for synthetic frames

Expand Down Expand Up @@ -278,6 +284,14 @@ class Profiler {

// Keep backward compatibility with the upstream async-profiler
inline CodeCache* findLibraryByAddress(const void *address) {
#ifdef DEBUG
// we need this code to simulate segfault during stackwalking
// this is a safe place to do it since this wrapper is used solely from the 'vm' stackwalker implementation
if (force_stackwalk_crash_env) {
TEST_LOG("FORCE_SIGSEGV");
raise(SIGSEGV);
}
#endif
return Libraries::instance()->findLibraryByAddress(address);
}

Expand Down
81 changes: 63 additions & 18 deletions ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,38 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.LockSupport;
import java.util.function.Function;

import static org.junit.jupiter.api.Assertions.*;


public abstract class AbstractProcessProfilerTest {
protected final boolean launch(String target, List<String> jvmArgs, String commands, Function<String, Boolean> onStdoutLine, Function<String, Boolean> onStderrLine) throws Exception {
public static final class LaunchResult {
Copy link
Contributor

@r1viollet r1viollet May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see we are investing in controlled behaviour tests.
If we want to extend these, we could discuss what would be a good structure.
Nothing blocking though it is hard to see the structure. I would expect something like:

TestProcessController controller = new TestProcessController(WorkloadType.CPU_INTENSIVE);
controller.launch();
controller.waitForReady();
controller.sendStartSignal();
controller.waitForWorkingOutput();
controller.assertProfilerDidX();
controller.shutdown();

I think we basically have different parts

  • controller
  • assertions
  • workload
  • protocol

public final boolean inTime;
public final int exitCode;

public LaunchResult(boolean inTime, int exitCode) {
this.inTime = inTime;
this.exitCode = exitCode;
}
}

public enum LineConsumerResult {
CONTINUE,
STOP,
IGNORE
}

protected final LaunchResult launch(String target, List<String> jvmArgs, String commands, Function<String, LineConsumerResult> onStdoutLine, Function<String, LineConsumerResult> onStderrLine) throws Exception {
return launch(target, jvmArgs, commands, Collections.emptyMap(), onStdoutLine, onStderrLine);
}

protected final LaunchResult launch(String target, List<String> jvmArgs, String commands, Map<String, String> env, Function<String, LineConsumerResult> onStdoutLine, Function<String, LineConsumerResult> onStderrLine) throws Exception {
String javaHome = System.getenv("JAVA_TEST_HOME");
if (javaHome == null) {
javaHome = System.getenv("JAVA_HOME");
Expand All @@ -34,23 +57,34 @@ protected final boolean launch(String target, List<String> jvmArgs, String comma
}

ProcessBuilder pb = new ProcessBuilder(args);
pb.environment().putAll(env);
Process p = pb.start();
Thread stdoutReader = new Thread(() -> {
Function<String, Boolean> lineProcessor = onStdoutLine != null ? onStdoutLine : l -> true;
Function<String, LineConsumerResult> lineProcessor = onStdoutLine != null ? onStdoutLine : l -> LineConsumerResult.CONTINUE;
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println("[out] " + line);
if (!lineProcessor.apply(line)) {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
LineConsumerResult lResult = lineProcessor.apply(line);
switch (lResult) {
case STOP: {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
}
break;
}
case CONTINUE: {
if (line.contains("[ready]")) {
p.getOutputStream().write(1);
p.getOutputStream().flush();
}
break;
}
} else {
if (line.contains("[ready]")) {
p.getOutputStream().write(1);
p.getOutputStream().flush();
case IGNORE: {
// ignore
break;
}
}
}
Expand All @@ -60,16 +94,27 @@ protected final boolean launch(String target, List<String> jvmArgs, String comma
}
}, "stdout-reader");
Thread stderrReader = new Thread(() -> {
Function<String, Boolean> lineProcessor = onStderrLine != null ? onStderrLine : l -> true;
Function<String, LineConsumerResult> lineProcessor = onStderrLine != null ? onStderrLine : l -> LineConsumerResult.CONTINUE;
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println("[err] " + line);
if (!lineProcessor.apply(line)) {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
LineConsumerResult lResult = lineProcessor.apply(line);
switch (lResult) {
case STOP: {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
}
break;
}
case CONTINUE: {
break;
}
case IGNORE: {
// ignore
break;
}
}
}
Expand All @@ -89,6 +134,6 @@ protected final boolean launch(String target, List<String> jvmArgs, String comma
if (!val) {
p.destroyForcibly();
}
return val;
return new LaunchResult(val, p.exitValue());
}
}
51 changes: 51 additions & 0 deletions ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
package com.datadoghq.profiler;

import java.lang.management.ManagementFactory;
import java.lang.management.ThreadMXBean;
import java.util.Random;
import java.util.concurrent.atomic.LongAdder;

/**
* External launcher for the profiler under test.
* <p>
* This class is used to launch the profiler in a separate process for testing purposes.
* </p>
* The main method takes the following arguments:
* <ul>
* <li>library - loads the profiler library</li>
* <li>profiler [comma delimited profiler command list] - starts the profiler</li>
* <li>profiler-work:<expectedCpuTime> [comma delimited profiler command list] - starts the profiler and runs a CPU-intensive task</li>
* </ul>
*/
public class ExternalLauncher {
public static void main(String[] args) throws Exception {
Thread worker = null;
try {
if (args.length < 1) {
throw new RuntimeException();
Expand All @@ -16,6 +34,35 @@ public static void main(String[] args) throws Exception {
instance.execute(commands);
}
}
} else if (args[0].startsWith("profiler-work:")) {
long expectedCpuTime = Long.parseLong(args[0].substring("profiler-work:".length()));
ThreadMXBean thrdBean = ManagementFactory.getThreadMXBean();
JavaProfiler instance = JavaProfiler.getInstance();
if (args.length == 2) {
String commands = args[1];
if (!commands.isEmpty()) {
instance.execute(commands);
worker = new Thread(() -> {
Random rnd = new Random();
LongAdder adder = new LongAdder();
long counter = 0;
long cpuTime = thrdBean.getThreadCpuTime(Thread.currentThread().getId());
while (!Thread.currentThread().isInterrupted()) {
adder.add(rnd.nextLong());
// make sure we caused some CPU load and print the progress
if (++counter % 1000000 == 0) {
if (thrdBean.getThreadCpuTime(Thread.currentThread().getId()) - cpuTime > expectedCpuTime * 1_000_000L) {
cpuTime = thrdBean.getThreadCpuTime(Thread.currentThread().getId());
System.out.println("[working]");
System.out.flush();
}
}
}
System.out.println("[async] " + adder.sum());
});
worker.start();
}
}
}
} finally {
System.out.println("[ready]");
Expand All @@ -24,5 +71,9 @@ public static void main(String[] args) throws Exception {
}
// wait for signal to exit
System.in.read();
if (worker != null) {
worker.interrupt();
worker.join();
}
}
}
10 changes: 5 additions & 5 deletions ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ void sanityInitailizationTest() throws Exception {
l -> {
initLibraryFound.set(initLibraryFound.get() | l.contains("[TEST::INFO] VM::initLibrary"));
initProfilerFound.set(initProfilerFound.get() | l.contains("[TEST::INFO] VM::initProfilerBridge"));
return true;
return LineConsumerResult.CONTINUE;
},
null
);
).inTime;

assertTrue(rslt);

Expand All @@ -57,10 +57,10 @@ void jvmVersionTest() throws Exception {
boolean rslt = launch("library", Collections.emptyList(), null, l -> {
if (l.contains("[TEST::INFO] jvm_version#")) {
foundVersion.set(l.split("#")[1]);
return false;
return LineConsumerResult.STOP;
}
return true;
}, null);
return LineConsumerResult.CONTINUE;
}, null).inTime;

assertTrue(rslt);

Expand Down
48 changes: 38 additions & 10 deletions ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.LockSupport;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
Expand All @@ -29,8 +32,8 @@ void sanityInitailizationTest() throws Exception {
initFlag.set(initFlag.get() | 2);
}
// found both expected sections; can terminate the test now
return initFlag.get() != 3;
}, null);
return initFlag.get() != 3 ? LineConsumerResult.CONTINUE : LineConsumerResult.STOP;
}, null).inTime;

assertTrue(val);
}
Expand Down Expand Up @@ -59,13 +62,13 @@ void testJ9DefaultSanity() throws Exception {
boolean val = launch("profiler", Collections.emptyList(), "start,cpu,file=" + jfr, l -> {
if (l.contains("J9[cpu]")) {
usedSampler.set(l.split("=")[1]);
return false;
return LineConsumerResult.STOP;
} else if (l.contains("J9[wall]")) {
hasWall.set(true);
return false;
return LineConsumerResult.STOP;
}
return true;
}, null);
return LineConsumerResult.CONTINUE;
}, null).inTime;
assertTrue(val);
assertEquals(sampler, usedSampler.get());
assertFalse(hasWall.get());
Expand All @@ -91,15 +94,40 @@ void testJ9ForceJvmtiSanity() throws Exception {
boolean val = launch("profiler", args, "start,cpu,file=" + jfr, l -> {
if (l.contains("J9[cpu]")) {
usedSampler.set(l.split("=")[1]);
return false;
return LineConsumerResult.STOP;
} else if (l.contains("J9[wall]")) {
hasWall.set(true);
return false;
return LineConsumerResult.STOP;
}
return true;
}, null);
return LineConsumerResult.CONTINUE;
}, null).inTime;
assertTrue(val);
assertEquals(sampler, usedSampler.get());
assertFalse(hasWall.get());
}

@Test
void vmStackwalkerCrashRecoveryTest() throws Exception {
assumeFalse(Platform.isJ9() || Platform.isZing()); // J9 and Zing do not support vmstructs
String config = System.getProperty("ddprof_test.config");
assumeTrue("debug".equals(config));

Path jfr = Files.createTempFile("work", ".jfr");
jfr.toFile().deleteOnExit();

Map<String, String> env = Collections.singletonMap("DDPROF_FORCE_STACKWALK_CRASH", "1");
// run the profiled process and generate at least 50ms of cpu activity
LaunchResult rslt = launch("profiler-work:50", Collections.emptyList(), "start,cpu=1ms,cstack=vm,file=" + jfr, env, l -> {
if (l.contains("[ready]")) {
return LineConsumerResult.IGNORE;
}
if (l.contains("[working]")) {
return LineConsumerResult.STOP;
}
return LineConsumerResult.CONTINUE;
}, null);

assertTrue(rslt.inTime);
assertEquals(0, rslt.exitCode, "exit code should be 0");
}
}
Loading