diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 20dad38cd..2242d4b31 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -103,19 +103,28 @@ void Profiler::onThreadEnd(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) { _thread_filter.unregisterThread(slot_id); current->setFilterSlotId(-1); } - - ProfiledThread::release(); - } else { - // ProfiledThread already cleaned up - try to get tid from JVMTI as fallback - tid = JVMThread::nativeThreadId(jni, thread); - if (tid < 0) { - // No ProfiledThread AND can't get tid from JVMTI - nothing we can do - return; + + updateThreadName(jvmti, jni, thread, false); + // Block profiling signals around engine unregistration + TLS release to + // close the window where a wall-clock/CPU signal could sample a + // partially-torn-down thread (PROF-14674). + { + SignalBlocker blocker; + _cpu_engine->unregisterThread(tid); + _wall_engine->unregisterThread(tid); + ProfiledThread::release(); } + return; } - - // These can run if we have a valid tid - updateThreadName(jvmti, jni, thread, false); // false = not self + + // ProfiledThread already cleaned up - try to get tid from JVMTI as fallback + tid = JVMThread::nativeThreadId(jni, thread); + if (tid < 0) { + // No ProfiledThread AND can't get tid from JVMTI - nothing we can do + return; + } + + updateThreadName(jvmti, jni, thread, false); _cpu_engine->unregisterThread(tid); _wall_engine->unregisterThread(tid); } diff --git a/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp b/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp index 80ef531e0..a0dc97934 100644 --- a/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp +++ b/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp @@ -175,6 +175,13 @@ TEST(ThreadTeardownSafetyTest, SignalSafeAccessorReturnsNullWithoutInit) { // ── T-05: Concurrent signals during teardown stress ────────────────────────── +static std::atomic g_t05_signal_count{0}; + +static void t05_handler(int) { + (void)ProfiledThread::currentSignalSafe(); + g_t05_signal_count.fetch_add(1, std::memory_order_relaxed); +} + static void *t05_worker(void *) { ProfiledThread::initCurrentThread(); for (int i = 0; i < 10; ++i) { @@ -184,14 +191,14 @@ static void *t05_worker(void *) { return nullptr; } -// 20 threads × 5 rounds with signal spray; no crash expected. -// Signal dispositions are process-wide — set SIG_IGN once from the test body -// before spawning workers so concurrent SigGuard restore races cannot re-expose -// the default (terminate) disposition while a worker is mid-flight. +// 20 threads × 5 rounds with signal spray; handler invocation is verified. +// Signal dispositions are process-wide — install once from the test body so +// no worker can race to restore SIG_DFL (terminate) mid-flight. TEST(ThreadTeardownSafetyTest, ConcurrentSignalsDuringTeardownStress) { SigGuard gVtalrm(SIGVTALRM); SigGuard gProf(SIGPROF); - install_handler(SIGVTALRM, SIG_IGN); + g_t05_signal_count.store(0, std::memory_order_relaxed); + install_handler(SIGVTALRM, t05_handler); install_handler(SIGPROF, SIG_IGN); for (int round = 0; round < 5; ++round) { std::vector threads(20); @@ -202,6 +209,8 @@ TEST(ThreadTeardownSafetyTest, ConcurrentSignalsDuringTeardownStress) { pthread_join(tid, nullptr); } } + EXPECT_GT(g_t05_signal_count.load(std::memory_order_relaxed), 0) + << "SIGVTALRM handler must have been invoked at least once"; } // ── T-06: SignalBlocker masks SIGPROF + SIGVTALRM and restores on exit ──────── @@ -335,6 +344,12 @@ TEST(ThreadTeardownSafetyTest, DoubleInitIsIdempotent) { // ── T-09: High-frequency signals during thread churn ───────────────────────── static std::atomic g_t09_stop{false}; +static std::atomic g_t09_signal_count{0}; + +static void t09_handler(int) { + (void)ProfiledThread::currentSignalSafe(); + g_t09_signal_count.fetch_add(1, std::memory_order_relaxed); +} static void *t09_injector(void *) { while (!g_t09_stop.load(std::memory_order_relaxed)) { @@ -345,8 +360,6 @@ static void *t09_injector(void *) { } static void *t09_worker(void *) { - SigGuard guard(SIGVTALRM); - install_handler(SIGVTALRM, SIG_IGN); ProfiledThread::initCurrentThread(); usleep(100); ProfiledThread::release(); @@ -354,10 +367,12 @@ static void *t09_worker(void *) { } // Mirrors DumpWhileChurningThreadsTest at native level: 100 short-lived threads -// under continuous SIGVTALRM injection must complete without crash. +// under continuous SIGVTALRM injection must complete without crash; handler +// invocation is verified. TEST(ThreadTeardownSafetyTest, HighFrequencySignalsDuringThreadChurn) { SigGuard testGuard(SIGVTALRM); - install_handler(SIGVTALRM, SIG_IGN); + g_t09_signal_count.store(0, std::memory_order_relaxed); + install_handler(SIGVTALRM, t09_handler); g_t09_stop.store(false, std::memory_order_relaxed); pthread_t injector; @@ -371,6 +386,8 @@ TEST(ThreadTeardownSafetyTest, HighFrequencySignalsDuringThreadChurn) { g_t09_stop.store(true, std::memory_order_relaxed); pthread_join(injector, nullptr); + EXPECT_GT(g_t09_signal_count.load(std::memory_order_relaxed), 0) + << "SIGVTALRM handler must have been invoked at least once"; } // ── T-10: CriticalSection reentrancy guard prevents double-entry ──────────────