From 4769512510eb35ef05fedf756c4c4024d9bdd746 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 30 Jun 2026 14:04:39 +0200 Subject: [PATCH 1/9] =?UTF-8?q?test:=20add=20thread=20lifecycle=20=C3=97?= =?UTF-8?q?=20profiler-state=20coverage=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover three previously-untested cells in the thread-lifecycle × profiler-state matrix: - Cell 1 (Disabling+teardown): stop() races concurrent thread end - Cell 2 (Enabling+startup): execute() races active thread churn - Cell 3 (Disabled+teardown): thread lifecycle with idle profiler Also adds C++ unit tests for ThreadFilter races (unregister vs clearActive, register vs init toggle, unregister with no init). Co-Authored-By: Claude Sonnet 4.6 --- .../test/cpp/threadFilter_lifecycle_ut.cpp | 151 ++++++++++++++++++ .../lifecycle/IdleThreadLifecycleTest.java | 61 +++++++ .../StartWithChurningThreadsTest.java | 120 ++++++++++++++ .../StopWithChurningThreadsTest.java | 115 +++++++++++++ 4 files changed, 447 insertions(+) create mode 100644 ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java diff --git a/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp new file mode 100644 index 0000000000..f75a8f84e3 --- /dev/null +++ b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp @@ -0,0 +1,151 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + * + * Coverage for three previously-untested cells in the thread-lifecycle × + * profiler-state matrix, exercised at the native layer (no JVM required): + * + * Cell 1 – Disabling + teardown: ThreadFilter::unregisterThread races the + * clearActive() drain that stop() performs before disabling. + * Cell 2 – Enabling + startup: ThreadFilter::registerThread races a + * concurrent init() call that alternately enables/disables the filter. + * Cell 3 – Disabled + teardown: unregisterThread(-1) called when the filter + * was never initialised (idle profiler path). + * + * All three test cases are runnable under TSan and ASan. + */ +#include "gtest/gtest.h" + +#ifdef __linux__ + +#include "threadFilter.h" +#include "thread.h" +#include "../../main/cpp/gtest_crash_handler.h" + +#include +#include +#include + +static constexpr const char TEST_NAME[] = "ThreadFilterLifecycle"; +static constexpr int kWorkers = 16; +static constexpr int kIterations = 2000; + +// ── Cell 1: unregisterThread racing clearActive (simulated stop() drain) ────── + +TEST(ThreadFilterLifecycle, UnregisterRacesClearActive) { + installGtestCrashHandler(); + + ThreadFilter filter; + filter.init("*"); + ASSERT_TRUE(filter.enabled()); + + std::atomic run{true}; + + // Drain thread: simulates what stop() does — clearActive() empties the active + // slots without disabling the filter, to flush in-flight samples. + std::thread drainer([&]() { + while (run.load(std::memory_order_relaxed)) { + filter.clearActive(); + } + }); + + std::vector workers; + workers.reserve(kWorkers); + for (int i = 0; i < kWorkers; i++) { + workers.emplace_back([&]() { + for (int j = 0; j < kIterations; j++) { + ProfiledThread::initCurrentThread(); + ProfiledThread* self = ProfiledThread::current(); + if (!self) { + ProfiledThread::release(); + continue; + } + ThreadFilter::SlotID slot = filter.registerThread(); + if (slot >= 0) { + filter.add(self->tid(), slot); + filter.remove(slot); + filter.unregisterThread(slot); + self->setFilterSlotId(-1); + } + ProfiledThread::release(); + } + }); + } + for (auto& t : workers) t.join(); + run.store(false, std::memory_order_relaxed); + drainer.join(); + + restoreDefaultSignalHandlers(); + SUCCEED(); +} + +// ── Cell 2: registerThread racing concurrent init() enable/disable toggles ──── + +TEST(ThreadFilterLifecycle, RegisterRacesInit) { + installGtestCrashHandler(); + + ThreadFilter filter; + + std::atomic run{true}; + + // Init thread: alternates between enabling ("*") and disabling (nullptr) the + // filter, mirroring start()/stop() engine cycling under test conditions. + std::thread initer([&]() { + int toggle = 0; + while (run.load(std::memory_order_relaxed)) { + // Even: enable ("*"), odd: disable (nullptr / empty string) + filter.init(toggle++ % 2 == 0 ? "*" : nullptr); + } + }); + + std::vector workers; + workers.reserve(kWorkers); + for (int i = 0; i < kWorkers; i++) { + workers.emplace_back([&]() { + for (int j = 0; j < kIterations; j++) { + // Mirror the production onThreadStart guard: skip if filter disabled. + if (!filter.enabled()) continue; + + ProfiledThread::initCurrentThread(); + ProfiledThread* self = ProfiledThread::current(); + if (!self) { + ProfiledThread::release(); + continue; + } + ThreadFilter::SlotID slot = filter.registerThread(); + if (slot >= 0) { + // filter may have been disabled between the enabled() check and + // registerThread(); the slot is still valid and must be released. + filter.unregisterThread(slot); + self->setFilterSlotId(-1); + } + ProfiledThread::release(); + } + }); + } + for (auto& t : workers) t.join(); + run.store(false, std::memory_order_relaxed); + initer.join(); + + restoreDefaultSignalHandlers(); + SUCCEED(); +} + +// ── Cell 3: unregisterThread with sentinel slot when filter never init'd ─────── + +TEST(ThreadFilterLifecycle, UnregisterWithNoInitDoesNotCrash) { + // No crash handler — single-threaded, expected outcome is "no crash". + ThreadFilter filter; + + // enabled() must be false without init(). + EXPECT_FALSE(filter.enabled()); + + // unregisterThread(-1) is what onThreadEnd calls when filterSlotId() == -1 + // (thread was registered before profiler started, so slot was never assigned). + // Must not crash, assert, or invoke UB. + filter.unregisterThread(-1); + + SUCCEED(); +} + +#endif // __linux__ diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java new file mode 100644 index 0000000000..020acf78ba --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java @@ -0,0 +1,61 @@ +package com.datadoghq.profiler.lifecycle; + +import com.datadoghq.profiler.JavaProfiler; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.junit.RetryTest; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression test for the "Disabled + teardown" coverage cell. + * + *

Verifies that thread lifecycle events (ThreadStart/ThreadEnd JVMTI callbacks, + * ProfiledThread::release) are safe when the profiler has never been started. + * The JVMTI callbacks registered in JNI_OnLoad delegate to onThreadStart / + * onThreadEnd unconditionally; those paths must tolerate a never-started profiler. + * + *

Does NOT call profiler.execute() — the profiler stays idle throughout. + */ +public class IdleThreadLifecycleTest { + + private static final int THREAD_COUNT = 200; + + @RetryTest(2) + @Timeout(30) + @Test + public void threadLifecycleWhileProfilerIdle() throws Exception { + Assumptions.assumeTrue(!Platform.isJ9(), "J9 has a different JVMTI implementation"); + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11)); + + JavaProfiler profiler = JavaProfiler.getInstance(); + // Profiler must NOT be started — verify it is idle before we begin. + String statusBefore = profiler.getStatus(); + assertFalse(statusBefore.contains("Running : true"), + "Profiler must be idle at test start; status: " + statusBefore); + + ExecutorService pool = Executors.newFixedThreadPool(16); + CountDownLatch done = new CountDownLatch(THREAD_COUNT); + for (int i = 0; i < THREAD_COUNT; i++) { + pool.submit(done::countDown); + } + assertTrue(done.await(10, TimeUnit.SECONDS), + "Not all " + THREAD_COUNT + " thread tasks completed"); + pool.shutdown(); + pool.awaitTermination(5, TimeUnit.SECONDS); + + // Profiler must still be idle — thread lifecycle events must not trigger a start. + String statusAfter = profiler.getStatus(); + assertFalse(statusAfter.contains("Running : true"), + "Profiler must remain idle after thread churn; status: " + statusAfter); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java new file mode 100644 index 0000000000..907cd6c854 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -0,0 +1,120 @@ +package com.datadoghq.profiler.lifecycle; + +import com.datadoghq.profiler.JavaProfiler; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.junit.RetryTest; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Queue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Regression test for the "Enabling + startup/teardown" coverage cell. + * + *

Races profiler.execute() against threads that are already generating JVMTI + * ThreadStart/ThreadEnd events. The hazard window: execute() installs signal + * handlers, initialises the thread filter, and registers JVMTI callbacks one by + * one; a thread starting or ending during this sequence may call onThreadStart / + * onThreadEnd on a half-initialised engine. + * + *

Does NOT extend AbstractProfilerTest — needs direct control over + * execute()/stop() timing; uses the same pattern as ShutdownTest. + */ +public class StartWithChurningThreadsTest { + + private static final int CHURN_THREADS = 64; + private static final int CYCLES = 50; + private static final String PROFILER_CMD = "start,wall=5ms,filter=*,jfr,file="; + + @RetryTest(2) + @Timeout(60) + @Test + public void startRacesThreadStartEnd() throws Exception { + Assumptions.assumeTrue(!Platform.isJ9(), "J9 has a different JVMTI implementation"); + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11), "wall-clock requires Java 11+"); + Assumptions.assumeTrue(!Platform.isZing(), "Zing forces cstack=no"); + + JavaProfiler profiler = JavaProfiler.getInstance(); + Path recordings = Files.createTempDirectory("lifecycle-cell2-"); + Queue errors = new LinkedBlockingQueue<>(); + + AtomicBoolean running = new AtomicBoolean(true); + // Latch: churn threads signal they are running before execute() is called, + // ensuring JVMTI events fire during profiler initialisation. + CountDownLatch churnRunning = new CountDownLatch(CHURN_THREADS); + ExecutorService churn = Executors.newFixedThreadPool(CHURN_THREADS); + + for (int i = 0; i < CHURN_THREADS; i++) { + churn.submit(() -> { + churnRunning.countDown(); + while (running.get()) { + Thread t = new Thread(() -> {}); + t.start(); + try { + t.join(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + }); + } + // Wait for churn to be active BEFORE calling execute() + assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); + + try { + for (int cycle = 0; cycle < CYCLES; cycle++) { + Path jfr = Files.createTempFile(recordings, "c2-" + cycle + "-", ".jfr"); + try { + // execute() races with active thread churn + profiler.execute(PROFILER_CMD + jfr.toAbsolutePath()); + Thread.sleep(20); + profiler.stop(); + } catch (Throwable t) { + errors.offer(t); + break; + } finally { + Files.deleteIfExists(jfr); + } + } + } finally { + running.set(false); + churn.shutdown(); + if (!churn.awaitTermination(5, TimeUnit.SECONDS)) { + churn.shutdownNow(); + } + try { + profiler.stop(); + } catch (IOException ignored) { + // already stopped + } + deleteDirectory(recordings); + } + + if (!errors.isEmpty()) { + fail(errors.poll()); + } + } + + private static void deleteDirectory(Path dir) { + try { + Files.walk(dir) + .sorted(java.util.Comparator.reverseOrder()) + .forEach(p -> { try { Files.deleteIfExists(p); } catch (IOException ignored) {} }); + } catch (IOException ignored) {} + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java new file mode 100644 index 0000000000..30370abf30 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -0,0 +1,115 @@ +package com.datadoghq.profiler.lifecycle; + +import com.datadoghq.profiler.JavaProfiler; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.junit.RetryTest; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Queue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Regression test for the "Disabling + teardown" coverage cell. + * + *

Races profiler.stop() against concurrent thread creation and destruction. + * The hazard window: a thread calling JVMTI ThreadEnd (or ProfiledThread::release) + * concurrently with stop() can hit the ThreadFilter or signal handler after they + * have been torn down. + * + *

Does NOT extend AbstractProfilerTest — needs direct control over + * execute()/stop() timing; uses the same pattern as ShutdownTest. + */ +public class StopWithChurningThreadsTest { + + private static final int CHURN_THREADS = 64; + private static final int CYCLES = 50; + private static final String PROFILER_CMD = "start,wall=5ms,filter=*,jfr,file="; + + @RetryTest(2) + @Timeout(60) + @Test + public void stopRacesThreadEnd() throws Exception { + Assumptions.assumeTrue(!Platform.isJ9(), "J9 has a different JVMTI implementation"); + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11), "wall-clock requires Java 11+"); + Assumptions.assumeTrue(!Platform.isZing(), "Zing forces cstack=no"); + + JavaProfiler profiler = JavaProfiler.getInstance(); + Path recordings = Files.createTempDirectory("lifecycle-cell1-"); + Queue errors = new LinkedBlockingQueue<>(); + + AtomicBoolean running = new AtomicBoolean(true); + CountDownLatch churnRunning = new CountDownLatch(CHURN_THREADS); + ExecutorService churn = Executors.newFixedThreadPool(CHURN_THREADS); + + for (int i = 0; i < CHURN_THREADS; i++) { + churn.submit(() -> { + churnRunning.countDown(); + while (running.get()) { + Thread t = new Thread(() -> {}); + t.start(); + try { + t.join(5); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + }); + } + assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); + + try { + for (int cycle = 0; cycle < CYCLES; cycle++) { + Path jfr = Files.createTempFile(recordings, "c1-" + cycle + "-", ".jfr"); + try { + profiler.execute(PROFILER_CMD + jfr.toAbsolutePath()); + Thread.sleep(20); + profiler.stop(); + } catch (Throwable t) { + errors.offer(t); + break; + } finally { + Files.deleteIfExists(jfr); + } + } + } finally { + running.set(false); + churn.shutdown(); + if (!churn.awaitTermination(5, TimeUnit.SECONDS)) { + churn.shutdownNow(); + } + try { + profiler.stop(); + } catch (IOException ignored) { + // already stopped + } + deleteDirectory(recordings); + } + + if (!errors.isEmpty()) { + fail(errors.poll()); + } + } + + private static void deleteDirectory(Path dir) { + try { + Files.walk(dir) + .sorted(java.util.Comparator.reverseOrder()) + .forEach(p -> { try { Files.deleteIfExists(p); } catch (IOException ignored) {} }); + } catch (IOException ignored) {} + } +} From b31f27050ddabe821a34a3bc3a97b4b6f1b42954 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 30 Jun 2026 15:13:29 +0200 Subject: [PATCH 2/9] review: fix catch type and clarify synthetic test comment - Catch Exception instead of IOException in finally-block stop() so IllegalStateException (already-stopped) is silenced correctly - Clarify RegisterRacesInit comment: the init(nullptr) toggle is a synthetic stress, not a replay of production stop() semantics Co-Authored-By: Claude Sonnet 4.6 --- ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp | 3 ++- .../profiler/lifecycle/StartWithChurningThreadsTest.java | 2 +- .../profiler/lifecycle/StopWithChurningThreadsTest.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp index f75a8f84e3..a8e1cd3d5e 100644 --- a/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp @@ -89,7 +89,8 @@ TEST(ThreadFilterLifecycle, RegisterRacesInit) { std::atomic run{true}; // Init thread: alternates between enabling ("*") and disabling (nullptr) the - // filter, mirroring start()/stop() engine cycling under test conditions. + // filter; this is a synthetic enable/disable stress on _enabled, not a + // faithful replay of start()/stop() (production stop() never calls init()). std::thread initer([&]() { int toggle = 0; while (run.load(std::memory_order_relaxed)) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index 907cd6c854..e35cc85a60 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -99,7 +99,7 @@ public void startRacesThreadStartEnd() throws Exception { } try { profiler.stop(); - } catch (IOException ignored) { + } catch (Exception ignored) { // already stopped } deleteDirectory(recordings); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index 30370abf30..1937275fd5 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -94,7 +94,7 @@ public void stopRacesThreadEnd() throws Exception { } try { profiler.stop(); - } catch (IOException ignored) { + } catch (Exception ignored) { // already stopped } deleteDirectory(recordings); From 319a5187d806bfd39dfcb49d175e20fe472c1445 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 30 Jun 2026 16:40:15 +0200 Subject: [PATCH 3/9] Potential fix for pull request finding 'Ignored error status of call' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../profiler/lifecycle/StopWithChurningThreadsTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index 1937275fd5..a356f06352 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -80,7 +80,9 @@ public void stopRacesThreadEnd() throws Exception { Thread.sleep(20); profiler.stop(); } catch (Throwable t) { - errors.offer(t); + if (!errors.offer(t)) { + fail("Failed to record test error", t); + } break; } finally { Files.deleteIfExists(jfr); From c7046666bca1d8fcd011d0db1495f9992bb7e6fc Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 30 Jun 2026 16:40:27 +0200 Subject: [PATCH 4/9] Potential fix for pull request finding 'Ignored error status of call' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../profiler/lifecycle/StartWithChurningThreadsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index e35cc85a60..89808a0662 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -85,7 +85,7 @@ public void startRacesThreadStartEnd() throws Exception { Thread.sleep(20); profiler.stop(); } catch (Throwable t) { - errors.offer(t); + errors.add(t); break; } finally { Files.deleteIfExists(jfr); From 76b61abaea0cf9ede5d30a16b89e2471589040c5 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 30 Jun 2026 17:18:27 +0200 Subject: [PATCH 5/9] test: make lifecycle fuzz tests more aggressive - 500 cycles instead of 50 - wall=1ms to maximise signals in-flight during teardown - No thread join: ThreadEnd fires unpredictably relative to stop() - sleep(cycle % 3) instead of sleep(20): hits the 0-2ms window where signals are still being installed/removed - Timeout 120s to accommodate thread backpressure Co-Authored-By: Claude Sonnet 4.6 --- .../StartWithChurningThreadsTest.java | 17 ++++--------- .../StopWithChurningThreadsTest.java | 24 +++++++++---------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index 89808a0662..ea11df5d04 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -37,11 +37,11 @@ public class StartWithChurningThreadsTest { private static final int CHURN_THREADS = 64; - private static final int CYCLES = 50; - private static final String PROFILER_CMD = "start,wall=5ms,filter=*,jfr,file="; + private static final int CYCLES = 500; + private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; @RetryTest(2) - @Timeout(60) + @Timeout(120) @Test public void startRacesThreadStartEnd() throws Exception { Assumptions.assumeTrue(!Platform.isJ9(), "J9 has a different JVMTI implementation"); @@ -62,14 +62,7 @@ public void startRacesThreadStartEnd() throws Exception { churn.submit(() -> { churnRunning.countDown(); while (running.get()) { - Thread t = new Thread(() -> {}); - t.start(); - try { - t.join(5); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return; - } + new Thread(() -> {}).start(); } }); } @@ -82,7 +75,7 @@ public void startRacesThreadStartEnd() throws Exception { try { // execute() races with active thread churn profiler.execute(PROFILER_CMD + jfr.toAbsolutePath()); - Thread.sleep(20); + Thread.sleep(cycle % 3); profiler.stop(); } catch (Throwable t) { errors.add(t); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index a356f06352..bae7de9800 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -36,11 +36,13 @@ public class StopWithChurningThreadsTest { private static final int CHURN_THREADS = 64; - private static final int CYCLES = 50; - private static final String PROFILER_CMD = "start,wall=5ms,filter=*,jfr,file="; + private static final int CYCLES = 500; + // wall=1ms maximises signals in-flight during teardown; no filter so all + // threads receive signals, increasing collision probability with ThreadEnd. + private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; @RetryTest(2) - @Timeout(60) + @Timeout(120) @Test public void stopRacesThreadEnd() throws Exception { Assumptions.assumeTrue(!Platform.isJ9(), "J9 has a different JVMTI implementation"); @@ -59,14 +61,9 @@ public void stopRacesThreadEnd() throws Exception { churn.submit(() -> { churnRunning.countDown(); while (running.get()) { - Thread t = new Thread(() -> {}); - t.start(); - try { - t.join(5); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return; - } + // No join — thread dies on its own schedule, so ThreadEnd + // fires at an unpredictable time relative to stop(). + new Thread(() -> {}).start(); } }); } @@ -77,7 +74,10 @@ public void stopRacesThreadEnd() throws Exception { Path jfr = Files.createTempFile(recordings, "c1-" + cycle + "-", ".jfr"); try { profiler.execute(PROFILER_CMD + jfr.toAbsolutePath()); - Thread.sleep(20); + // Minimal delay: hit stop() while signals are still in-flight + // and threads are actively dying. Varies per cycle to avoid + // always landing in the same phase of the signal period. + Thread.sleep(cycle % 3); profiler.stop(); } catch (Throwable t) { if (!errors.offer(t)) { From 4c6a5e10c113f9b7fb711ea445871d88ffb715c8 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 1 Jul 2026 09:25:54 +0200 Subject: [PATCH 6/9] fix: copyright headers, latch-timeout cleanup, remove no-op @RetryTest --- .../profiler/lifecycle/IdleThreadLifecycleTest.java | 7 +++++-- .../lifecycle/StartWithChurningThreadsTest.java | 11 +++++++---- .../lifecycle/StopWithChurningThreadsTest.java | 9 ++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java index 020acf78ba..43a96aa83e 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/IdleThreadLifecycleTest.java @@ -1,8 +1,12 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.profiler.lifecycle; import com.datadoghq.profiler.JavaProfiler; import com.datadoghq.profiler.Platform; -import com.datadoghq.profiler.junit.RetryTest; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -30,7 +34,6 @@ public class IdleThreadLifecycleTest { private static final int THREAD_COUNT = 200; - @RetryTest(2) @Timeout(30) @Test public void threadLifecycleWhileProfilerIdle() throws Exception { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index ea11df5d04..c7452cba8b 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -1,8 +1,12 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.profiler.lifecycle; import com.datadoghq.profiler.JavaProfiler; import com.datadoghq.profiler.Platform; -import com.datadoghq.profiler.junit.RetryTest; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -40,7 +44,6 @@ public class StartWithChurningThreadsTest { private static final int CYCLES = 500; private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; - @RetryTest(2) @Timeout(120) @Test public void startRacesThreadStartEnd() throws Exception { @@ -66,10 +69,10 @@ public void startRacesThreadStartEnd() throws Exception { } }); } - // Wait for churn to be active BEFORE calling execute() - assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); try { + // Wait for churn to be active BEFORE calling execute() + assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); for (int cycle = 0; cycle < CYCLES; cycle++) { Path jfr = Files.createTempFile(recordings, "c2-" + cycle + "-", ".jfr"); try { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index bae7de9800..0a91bc6ecb 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -1,8 +1,12 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.profiler.lifecycle; import com.datadoghq.profiler.JavaProfiler; import com.datadoghq.profiler.Platform; -import com.datadoghq.profiler.junit.RetryTest; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -41,7 +45,6 @@ public class StopWithChurningThreadsTest { // threads receive signals, increasing collision probability with ThreadEnd. private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; - @RetryTest(2) @Timeout(120) @Test public void stopRacesThreadEnd() throws Exception { @@ -67,9 +70,9 @@ public void stopRacesThreadEnd() throws Exception { } }); } - assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); try { + assertTrue(churnRunning.await(10, TimeUnit.SECONDS), "Churn threads did not start"); for (int cycle = 0; cycle < CYCLES; cycle++) { Path jfr = Files.createTempFile(recordings, "c1-" + cycle + "-", ".jfr"); try { From d4debf917e7590695622930f21f2e2da4fd51834 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 1 Jul 2026 14:56:38 +0200 Subject: [PATCH 7/9] fix: disable thread filter and propagate churn worker failures in lifecycle tests Co-Authored-By: Claude Sonnet 4.6 --- .../StartWithChurningThreadsTest.java | 21 ++++++++++++--- .../StopWithChurningThreadsTest.java | 26 +++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index c7452cba8b..fd8984ed35 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -15,10 +15,15 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.Queue; +import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -42,7 +47,9 @@ public class StartWithChurningThreadsTest { private static final int CHURN_THREADS = 64; private static final int CYCLES = 500; - private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; + // filter= disables thread filtering so all threads receive SIGVTALRM, + // maximising the chance of racing execute() with active thread churn. + private static final String PROFILER_CMD = "start,wall=1ms,filter=,jfr,file="; @Timeout(120) @Test @@ -60,14 +67,15 @@ public void startRacesThreadStartEnd() throws Exception { // ensuring JVMTI events fire during profiler initialisation. CountDownLatch churnRunning = new CountDownLatch(CHURN_THREADS); ExecutorService churn = Executors.newFixedThreadPool(CHURN_THREADS); + List> churnFutures = new ArrayList<>(CHURN_THREADS); for (int i = 0; i < CHURN_THREADS; i++) { - churn.submit(() -> { + churnFutures.add(churn.submit(() -> { churnRunning.countDown(); while (running.get()) { new Thread(() -> {}).start(); } - }); + })); } try { @@ -99,6 +107,13 @@ public void startRacesThreadStartEnd() throws Exception { // already stopped } deleteDirectory(recordings); + for (Future f : churnFutures) { + try { + f.get(); + } catch (ExecutionException e) { + errors.add(e.getCause()); + } catch (CancellationException ignored) {} + } } if (!errors.isEmpty()) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index 0a91bc6ecb..00f399828d 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -15,10 +15,15 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.Queue; +import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -41,9 +46,10 @@ public class StopWithChurningThreadsTest { private static final int CHURN_THREADS = 64; private static final int CYCLES = 500; - // wall=1ms maximises signals in-flight during teardown; no filter so all - // threads receive signals, increasing collision probability with ThreadEnd. - private static final String PROFILER_CMD = "start,wall=1ms,jfr,file="; + // wall=1ms maximises signals in-flight during teardown; filter= disables + // thread filtering so all threads receive SIGVTALRM, increasing collision + // probability with ThreadEnd. + private static final String PROFILER_CMD = "start,wall=1ms,filter=,jfr,file="; @Timeout(120) @Test @@ -59,16 +65,17 @@ public void stopRacesThreadEnd() throws Exception { AtomicBoolean running = new AtomicBoolean(true); CountDownLatch churnRunning = new CountDownLatch(CHURN_THREADS); ExecutorService churn = Executors.newFixedThreadPool(CHURN_THREADS); + List> churnFutures = new ArrayList<>(CHURN_THREADS); for (int i = 0; i < CHURN_THREADS; i++) { - churn.submit(() -> { + churnFutures.add(churn.submit(() -> { churnRunning.countDown(); while (running.get()) { // No join — thread dies on its own schedule, so ThreadEnd // fires at an unpredictable time relative to stop(). new Thread(() -> {}).start(); } - }); + })); } try { @@ -103,6 +110,15 @@ public void stopRacesThreadEnd() throws Exception { // already stopped } deleteDirectory(recordings); + for (Future f : churnFutures) { + try { + f.get(); + } catch (ExecutionException e) { + if (!errors.offer(e.getCause())) { + fail("Failed to record churn worker error", e.getCause()); + } + } catch (CancellationException ignored) {} + } } if (!errors.isEmpty()) { From 5216effa6033741045f254f65682a9e3b83bf0ac Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 2 Jul 2026 16:32:56 +0200 Subject: [PATCH 8/9] fix: report all churn/profiler race errors via composite AssertionError --- .../profiler/lifecycle/StartWithChurningThreadsTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java index fd8984ed35..7e82a0125e 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StartWithChurningThreadsTest.java @@ -29,7 +29,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; /** * Regression test for the "Enabling + startup/teardown" coverage cell. @@ -117,7 +116,12 @@ public void startRacesThreadStartEnd() throws Exception { } if (!errors.isEmpty()) { - fail(errors.poll()); + AssertionError composite = new AssertionError( + errors.size() + " error(s) occurred during churn/profiler race"); + for (Throwable t : errors) { + composite.addSuppressed(t); + } + throw composite; } } From 52864e427cc57f92c771702198ae65a7912d6196 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 2 Jul 2026 16:33:33 +0200 Subject: [PATCH 9/9] fix: apply composite AssertionError to StopWithChurningThreadsTest too --- .../profiler/lifecycle/StopWithChurningThreadsTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java index 00f399828d..8ccf123215 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/lifecycle/StopWithChurningThreadsTest.java @@ -122,7 +122,12 @@ public void stopRacesThreadEnd() throws Exception { } if (!errors.isEmpty()) { - fail(errors.poll()); + AssertionError composite = new AssertionError( + errors.size() + " error(s) occurred during churn/profiler race"); + for (Throwable t : errors) { + composite.addSuppressed(t); + } + throw composite; } }