From 8d3cbb196e3c746389e5b47ff6457d5873c083cf Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 19:55:44 +0000 Subject: [PATCH 01/30] Restore priming --- ddprof-lib/src/main/cpp/context_api.cpp | 2 +- ddprof-lib/src/main/cpp/ctimer_linux.cpp | 1 + ddprof-lib/src/main/cpp/flightRecorder.cpp | 1 + ddprof-lib/src/main/cpp/guards.cpp | 2 +- .../src/main/cpp/hotspot/hotspotSupport.cpp | 23 +++--- ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | 2 +- ddprof-lib/src/main/cpp/itimer.cpp | 2 +- ddprof-lib/src/main/cpp/javaApi.cpp | 2 +- ddprof-lib/src/main/cpp/jvmSupport.cpp | 7 +- ddprof-lib/src/main/cpp/jvmThread.h | 14 ++++ ddprof-lib/src/main/cpp/perfEvents_linux.cpp | 2 +- ddprof-lib/src/main/cpp/refCountGuard.cpp | 2 +- ddprof-lib/src/main/cpp/stackWalker.cpp | 35 ++++---- ddprof-lib/src/main/cpp/threadLocalData.cpp | 15 ++-- ddprof-lib/src/main/cpp/threadLocalData.h | 34 ++++++-- .../src/main/cpp/threadLocalData.inline.h | 31 +++++++ .../src/main/cpp/threadLocalDataPool.cpp | 82 +++++++++++++++++++ ddprof-lib/src/main/cpp/threadLocalDataPool.h | 42 ++++++++++ ddprof-lib/src/main/cpp/wallClock.h | 2 +- 19 files changed, 250 insertions(+), 51 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/threadLocalData.inline.h create mode 100644 ddprof-lib/src/main/cpp/threadLocalDataPool.cpp create mode 100644 ddprof-lib/src/main/cpp/threadLocalDataPool.h diff --git a/ddprof-lib/src/main/cpp/context_api.cpp b/ddprof-lib/src/main/cpp/context_api.cpp index 082131a921..8a4f556f58 100644 --- a/ddprof-lib/src/main/cpp/context_api.cpp +++ b/ddprof-lib/src/main/cpp/context_api.cpp @@ -19,7 +19,7 @@ #include "guards.h" #include "otel_context.h" #include "profiler.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include /** diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index 11502c4883..a6a3c12ddd 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -27,6 +27,7 @@ #include "log.h" #include "profiler.h" #include "signalCookie.h" +#include "threadLocalData.inline.h" #include "threadState.inline.h" #include #include diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 6064e594cc..b71b841415 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -28,6 +28,7 @@ #include "unwindStats.h" #include "symbols.h" #include "threadFilter.h" +#include "threadLocalData.inline.h" #include "threadState.h" #include "tsc.h" #include "hotspot/vmStructs.h" diff --git a/ddprof-lib/src/main/cpp/guards.cpp b/ddprof-lib/src/main/cpp/guards.cpp index 9905182e9a..f0651d7a2c 100644 --- a/ddprof-lib/src/main/cpp/guards.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -17,7 +17,7 @@ #include "guards.h" #include "common.h" #include "os.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" // Signal-context tracking — backed by ProfiledThread::_signal_depth; see // the comment block in guards.h for the rationale (initial-exec TLS was diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 66057d141a..af045189db 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -18,6 +18,7 @@ #include "profiler.h" #include "stackWalker.inline.h" #include "threadLocal.h" +#include "threadLocalData.inline.h" using StackWalkValidation::inDeadZone; using StackWalkValidation::aligned; @@ -240,7 +241,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // VMStructs is only available for hotspot JVM assert(VM::isHotspot()); - ProfiledThread* prof_thread = ProfiledThread::current(); + ProfiledThread* prof_thread = ProfiledThread::acquire_current(); if (prof_thread == nullptr) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; @@ -1210,27 +1211,25 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { // AsyncGetCallTrace. Install a jmp ctx here too, so a SIGSEGV anywhere in // walkJavaStack, except HotSpot's AsyncGetCallTrace call, is caught by // Profiler::checkFault() and siglongjmp'd back here instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::current(); - const bool prev_unwinding_java = prof_thread != nullptr ? prof_thread->is_unwinding_Java() : false; + ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + if (prof_thread == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return 0; + } sigjmp_buf crash_protection_ctx; sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; - if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + if (sigsetjmp(crash_protection_ctx, 1) != 0) { // checkFault() does a siglongjmp from inside segvHandler, bypassing // segvHandler's SignalHandlerScope destructor. Compensate. SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); prof_thread->setJmpCtx(prev_jmp_buf); - // A recovered siglongjmp bypasses AsyncSampleMutex destructors, so restore - // the per-thread guard to its pre-walk value. - prof_thread->set_unwinding_Java(prev_unwinding_java); if (truncated) { *truncated = true; } return java_frames; } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(&crash_protection_ctx); - } + prof_thread->setJmpCtx(&crash_protection_ctx); if (features.mixed) { java_frames = walkVM(ucontext, frames, max_depth, features, eventTypeFromBCI(request.event_type), lock_index, truncated); @@ -1284,9 +1283,7 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { } } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(prev_jmp_buf); - } + prof_thread->setJmpCtx(prev_jmp_buf); return java_frames; } diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index 73d7e2aec6..c1a1c59ecf 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -15,7 +15,7 @@ #include "jvmThread.h" #include "safeAccess.h" #include "spinLock.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "threadState.h" CodeCache* VMStructs::_libjvm = nullptr; diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp index 0c1a134df3..820ba341b4 100644 --- a/ddprof-lib/src/main/cpp/itimer.cpp +++ b/ddprof-lib/src/main/cpp/itimer.cpp @@ -22,7 +22,7 @@ #include "profiler.h" #include "signalInflight.h" #include "stackWalker.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "threadState.inline.h" #include "guards.h" #include diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 3f71a1b74c..380b3058c7 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -31,7 +31,7 @@ #include "os.h" #include "otel_process_ctx.h" #include "profiler.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "tsc.h" #include "vmEntry.h" #include diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index b6277a6be4..0068d211fd 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -9,7 +9,8 @@ #include "frames.h" #include "os.h" #include "profiler.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" +#include "threadLocalDataPool.h" #include "vmEntry.h" #include "hotspot/hotspotSupport.h" @@ -38,6 +39,10 @@ bool JVMSupport::initialize() { return false; } + if (JVMThread::supportPriming()) { + ThreadLocalDataPool::initialize(); + } + // Check ProfiledThread key, it is critical for storing per-thread metadata return ProfiledThread::isThreadKeyValid(); } diff --git a/ddprof-lib/src/main/cpp/jvmThread.h b/ddprof-lib/src/main/cpp/jvmThread.h index 2f5bd69104..bbc3a79b9c 100644 --- a/ddprof-lib/src/main/cpp/jvmThread.h +++ b/ddprof-lib/src/main/cpp/jvmThread.h @@ -10,12 +10,16 @@ #include #include "threadLocal.h" +#include "os.h" /** * JVMThread represents a native JVM thread that is JVM implementation agnostic */ class JVMThread { private: + // PTHREAD_KEY_2NDLEVEL_SIZE is an internal macro set to 32 in the GNU C Library (glibc) NPTL + // implementation. Slot indexes less than PTHREAD_KEY_2NDLEVEL_SIZE are pre-allocated. + static constexpr int PTHREAD_KEY_2NDLEVEL_SIZE = 32; static jfieldID _tid; static ThreadLocal _jvm_thread; @@ -41,6 +45,16 @@ class JVMThread { return _jvm_thread.key(); } + static bool supportPriming() { + // Key must be valid + assert(_jvm_thread.isKeyValid()); + if (OS::isMusl()) { + return true; + } else { + return _jvm_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; + } + } + static int nativeThreadId(JNIEnv* jni, jthread thread); static inline jlong javaThreadId(JNIEnv* env, jthread thread) { return env->GetLongField(thread, _tid); diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp index b3e5028135..a111a5a14f 100644 --- a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp +++ b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp @@ -35,7 +35,7 @@ #include "stackFrame.h" #include "stackWalker.h" #include "symbols.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "threadState.inline.h" #include #include diff --git a/ddprof-lib/src/main/cpp/refCountGuard.cpp b/ddprof-lib/src/main/cpp/refCountGuard.cpp index 5f37d74a43..7af5a3216d 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.cpp +++ b/ddprof-lib/src/main/cpp/refCountGuard.cpp @@ -10,7 +10,7 @@ #include "log.h" #include "os.h" #include "primeProbing.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include #include diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index 4de65b5805..3912cfebd3 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -13,7 +13,7 @@ #include "symbols.h" #include "jvmSupport.inline.h" #include "jvmThread.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" // Use validation helpers from header (shared with tests) using StackWalkValidation::inDeadZone; @@ -45,11 +45,15 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::current(); + ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + if (prof_thread == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return 0; + } sigjmp_buf crash_protection_ctx; - sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + sigjmp_buf* prev_jmp_buf = prof_thread->getJmpCtx(); - if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + if (sigsetjmp(crash_protection_ctx, 1) != 0) { // checkFault() does a siglongjmp from inside segvHandler, bypassing // segvHandler's SignalHandlerScope destructor. Compensate. SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); @@ -62,9 +66,7 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S } return depth; } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(&crash_protection_ctx); - } + prof_thread->setJmpCtx(&crash_protection_ctx); // Walk until the bottom of the stack or until the first Java frame while (depth < actual_max_depth) { @@ -132,11 +134,16 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::current(); + ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + if (prof_thread == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return 0; + } + sigjmp_buf crash_protection_ctx; - sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + sigjmp_buf* prev_jmp_buf = prof_thread->getJmpCtx(); - if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + if (sigsetjmp(crash_protection_ctx, 1) != 0) { // checkFault() does a siglongjmp from inside segvHandler, bypassing // segvHandler's SignalHandlerScope destructor. Compensate. SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); @@ -149,9 +156,7 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth } return depth; } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(&crash_protection_ctx); - } + prof_thread->setJmpCtx(&crash_protection_ctx); // Walk until the bottom of the stack or until the first Java frame while (depth < actual_max_depth) { @@ -229,9 +234,7 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth } } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(prev_jmp_buf); - } + prof_thread->setJmpCtx(prev_jmp_buf); if (truncated && depth > max_depth) { *truncated = true; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 37942fe36f..2ff72b896b 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "threadLocalData.h" +#include "threadLocalData.inline.h" +#include "threadLocalDataPool.h" #include "context_api.h" #include "guards.h" #include "otel_context.h" @@ -49,11 +50,13 @@ ProfiledThread* ProfiledThread::initCurrentThreadSignalSafe() { void ProfiledThread::freeValue(void* value) { SignalBlocker blocker; ProfiledThread* pt = reinterpret_cast(value); - // Sole deletion site for a ProfiledThread (invoked by the ThreadLocal - // destructor callback), so the THREAD_LOCAL decrement belongs here. Record - // after the delete, consistent with the other decrement sites. - delete pt; - NativeMem::record(NM_THREAD_LOCAL, -(long long)sizeof(ProfiledThread)); + if (!ThreadLocalDataPool::release(pt)) { + // Sole deletion site for a ProfiledThread (invoked by the ThreadLocal + // destructor callback), so the THREAD_LOCAL decrement belongs here. Record + // after the delete, consistent with the other decrement sites. + delete pt; + NativeMem::record(NM_THREAD_LOCAL, -(long long)sizeof(ProfiledThread)); + } } void ProfiledThread::release() { diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 30f750cb15..46fce44af3 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -37,6 +37,8 @@ class ThreadLocalData { }; class ProfiledThread : public ThreadLocalData { + friend class ThreadLocalDataPool; + public: enum ThreadType : u32 { TYPE_UNKNOWN = 0, @@ -46,6 +48,8 @@ class ProfiledThread : public ThreadLocalData { }; static constexpr u32 FLAG_PARKED = 0x4u; // next free bit after TYPE_MASK (0x1|0x2) + static constexpr u32 FLAG_CLAIMED = 0x8u; // Used by ThreadLocalDataPool only + // We are allowing several levels of nesting because we can be // eg. in a crash handler when wallclock signal kicks in, @@ -74,7 +78,7 @@ class ProfiledThread : public ThreadLocalData { u32 _wall_epoch; u64 _call_trace_id; u32 _recording_epoch; - u32 _misc_flags; + volatile u32 _misc_flags; u64 _park_block_token; int _filter_slot_id; // Slot ID for thread filtering uint8_t _init_window; // Countdown for JVM thread init race window (PROF-13072) @@ -112,6 +116,24 @@ class ProfiledThread : public ThreadLocalData { }; virtual ~ProfiledThread() { } + + inline bool isClaimed() const { + return (__atomic_load_n(&_misc_flags, __ATOMIC_RELAXED) & FLAG_CLAIMED) == FLAG_CLAIMED; + } + + inline bool claim_acquire(int tid) { + if (isClaimed()) { + return false; + } + + u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); + bool rc = (flags & FLAG_CLAIMED) == 0; + if (rc) { + _tid = tid; + } + return rc; +} + public: static ProfiledThread *forTid(int tid) { ProfiledThread *pt = new ProfiledThread(tid); @@ -154,12 +176,10 @@ class ProfiledThread : public ThreadLocalData { static ProfiledThread* initCurrentThreadSignalSafe(); // Signal-handler friendly (no allocation): returns existing TLS or nullptr. - static inline ProfiledThread *current() { - if (!isThreadKeyValid()) { - return nullptr; - } - return _current_thread.get(); - } + static inline ProfiledThread *current(); + // signal-handler friendly with priming: return existing TLS or acquire and set + // ProfiledThread from ThreadLocalDataPool. + static inline ProfiledThread* acquire_current(); static int currentTid(); inline int tid() { return _tid; } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h new file mode 100644 index 0000000000..a4f60c9806 --- /dev/null +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -0,0 +1,31 @@ +/* + * Copyright 2026 Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef THREADLOCALDATA_INLINE_H +#define THREADLOCALDATA_INLINE_H + +#include "os.h" +#include "threadLocalData.h" +#include "threadLocalDataPool.h" + +ProfiledThread* ProfiledThread::current() { + if (!isThreadKeyValid()) { + return nullptr; + } + return _current_thread.get(); +} + +ProfiledThread* ProfiledThread::acquire_current() { + ProfiledThread* prof_thread = current(); + if (prof_thread == nullptr) { + prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); + if (prof_thread != nullptr) { + _current_thread.set(prof_thread); + } + } + return prof_thread; +} + +#endif // THREADLOCALDATA_INLINE_H diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp new file mode 100644 index 0000000000..7b15d12448 --- /dev/null +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -0,0 +1,82 @@ + + /* + * Copyright 2026 Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "threadLocalData.h" +#include "threadLocalDataPool.h" + +#include + +ThreadLocalDataPool* ThreadLocalDataPool::_pool = nullptr; + +ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) : _capacity(capacity), _used(0) { + size_t malloc_size = capacity * sizeof(ProfiledThread); + void* p = malloc(malloc_size); + if (p != nullptr) { + _threads = reinterpret_cast(p); + for (int index = 0; index < capacity; index++) { + new (&_threads[index])ProfiledThread(0); + } + NativeMem::record(NM_THREAD_LOCAL, malloc_size); + } +} + +ThreadLocalDataPool::~ThreadLocalDataPool() { + if (_threads != nullptr) { + free(reinterpret_cast(_threads)); + } +} + +ProfiledThread* ThreadLocalDataPool::claim(int tid) { + uint16_t used = __atomic_fetch_add(&_used, 1, __ATOMIC_RELAXED); + if (used >= _capacity) { + __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); + return nullptr; + } + + int start_pos = tid % _capacity; + int index = start_pos; + do { + if (_threads[index].claim_acquire(tid)) { + return &_threads[index]; + } + index = (index + 1) % _capacity; + } while (index != start_pos); + assert(false && "Should not reach here"); + return nullptr; +} + +bool ThreadLocalDataPool::unclaim(ProfiledThread* t) { + if (contains(t)) { + new (t)ProfiledThread(0); + uint16_t used = __atomic_fetch_add(&_used, -1, __ATOMIC_RELEASE); + assert(used > 0); + return true; + } + return false; +} + +void ThreadLocalDataPool::initialize() { + ThreadLocalDataPool* pool = new ThreadLocalDataPool(); + __atomic_store_n(&_pool, pool, __ATOMIC_RELEASE); +} + +ProfiledThread* ThreadLocalDataPool::acquire(int tid) { + ThreadLocalDataPool* pool = __atomic_load_n(&_pool, __ATOMIC_ACQUIRE); + if (pool == nullptr) { + return nullptr; + } else { + return pool->claim(tid); + } +} + +bool ThreadLocalDataPool::release(ProfiledThread* t) { + ThreadLocalDataPool* pool = __atomic_load_n(&_pool, __ATOMIC_ACQUIRE); + if (pool != nullptr) { + return pool->unclaim(t); + } else { + return false; + } +} \ No newline at end of file diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h new file mode 100644 index 0000000000..1971880074 --- /dev/null +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -0,0 +1,42 @@ +/* + * Copyright 2026 Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef THREADLOCALDATA_POOL_H +#define THREADLOCALDATA_POOL_H + +#include + +class ProfiledThread; + +class ThreadLocalDataPool { + static constexpr uint16_t DEFAULT_CAPACITY = 64; +private: + static ThreadLocalDataPool* _pool; + + const uint64_t _capacity; + volatile uint16_t _used; + ProfiledThread* _threads; + + ThreadLocalDataPool(const ThreadLocalDataPool&) = delete; + ThreadLocalDataPool& operator=(const ThreadLocalDataPool&) = delete; + + ThreadLocalDataPool(uint64_t capacity = DEFAULT_CAPACITY); + ~ThreadLocalDataPool(); + + ProfiledThread* claim(int tid); + bool unclaim(ProfiledThread* t); + + + inline bool contains(ProfiledThread* t) { + return t >= _threads && t <= &_threads[_capacity - 1]; + } + +public: + static void initialize(); + static ProfiledThread* acquire(int tid); + static bool release(ProfiledThread* t); +}; + +#endif // THREADLOCALDATA_POOL_H \ No newline at end of file diff --git a/ddprof-lib/src/main/cpp/wallClock.h b/ddprof-lib/src/main/cpp/wallClock.h index 14e3f88aa3..7e2c7fefa7 100644 --- a/ddprof-lib/src/main/cpp/wallClock.h +++ b/ddprof-lib/src/main/cpp/wallClock.h @@ -12,7 +12,7 @@ #include "os.h" #include "profiler.h" #include "reservoirSampler.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "threadFilter.h" #include "threadState.h" #include "tsc.h" From 856ee133a68fe770ffcfa01dd89dffccb4305ee2 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 22:10:41 +0200 Subject: [PATCH 02/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 1971880074..93f0ec5a0c 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -29,8 +29,12 @@ class ThreadLocalDataPool { bool unclaim(ProfiledThread* t); - inline bool contains(ProfiledThread* t) { - return t >= _threads && t <= &_threads[_capacity - 1]; + inline bool contains(ProfiledThread* t) const { + if (_threads == nullptr || t == nullptr) return false; + const uintptr_t addr = reinterpret_cast(t); + const uintptr_t base = reinterpret_cast(_threads); + const uintptr_t end = reinterpret_cast(_threads + _capacity); + return addr >= base && addr < end; } public: From 6a32799d44b394f56aabc01f849cf15929436f92 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 20:54:49 +0000 Subject: [PATCH 03/30] Fix --- .../src/main/cpp/hotspot/hotspotSupport.cpp | 6 +++++- ddprof-lib/src/main/cpp/threadLocalData.h | 19 ++++++++++--------- .../src/main/cpp/threadLocalDataPool.cpp | 8 ++++++-- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index af045189db..d26fbb56c2 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -1216,14 +1216,18 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; } + const bool prev_unwinding_java = prof_thread->is_unwinding_Java(); sigjmp_buf crash_protection_ctx; - sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + sigjmp_buf* prev_jmp_buf = prof_thread->getJmpCtx(); if (sigsetjmp(crash_protection_ctx, 1) != 0) { // checkFault() does a siglongjmp from inside segvHandler, bypassing // segvHandler's SignalHandlerScope destructor. Compensate. SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); prof_thread->setJmpCtx(prev_jmp_buf); + // A recovered siglongjmp bypasses AsyncSampleMutex destructors, so restore + // the per-thread guard to its pre-walk value. + prof_thread->set_unwinding_Java(prev_unwinding_java); if (truncated) { *truncated = true; } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 46fce44af3..e5bf9d59b0 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -99,9 +99,9 @@ class ProfiledThread : public ThreadLocalData { alignas(8) u32 _otel_tag_encodings[DD_TAGS_CAPACITY]; u64 _otel_local_root_span_id; - ProfiledThread(int tid) + ProfiledThread(int tid, bool claimed = false) : ThreadLocalData(), _jmp_buf(nullptr), _pc(0), _sp(0), _span_id(0), _crash_depth(0), _tid(tid), _cpu_epoch(0), - _wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(0), + _wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(claimed ? FLAG_CLAIMED : 0), _park_block_token(0), _filter_slot_id(-1), _init_window(0), _signal_depth(0), _otel_ctx_initialized(false), @@ -118,20 +118,21 @@ class ProfiledThread : public ThreadLocalData { virtual ~ProfiledThread() { } inline bool isClaimed() const { - return (__atomic_load_n(&_misc_flags, __ATOMIC_RELAXED) & FLAG_CLAIMED) == FLAG_CLAIMED; + return (__atomic_load_n(&_misc_flags, __ATOMIC_RELAXED) & FLAG_CLAIMED) == FLAG_CLAIMED; } - inline bool claim_acquire(int tid) { + inline void unclaim() { + assert(isClaimed() && "Slot has been claimed"); + __atomic_fetch_and(&_misc_flags, ~FLAG_CLAIMED, __ATOMIC_RELEASE); + } + + inline bool claim_acquire() { if (isClaimed()) { return false; } u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); - bool rc = (flags & FLAG_CLAIMED) == 0; - if (rc) { - _tid = tid; - } - return rc; + return (flags & FLAG_CLAIMED) == 0; } public: diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 7b15d12448..4923f04933 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -39,7 +39,7 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { int start_pos = tid % _capacity; int index = start_pos; do { - if (_threads[index].claim_acquire(tid)) { + if (_threads[index].claim_acquire()) { return &_threads[index]; } index = (index + 1) % _capacity; @@ -68,7 +68,11 @@ ProfiledThread* ThreadLocalDataPool::acquire(int tid) { if (pool == nullptr) { return nullptr; } else { - return pool->claim(tid); + ProfiledThread* t = pool->claim(tid); + if (t != nullptr) { + new (t)ProfiledThread(tid, true /* claimed */); + } + return t; } } From 918fad4709eaf259551e91f888dbef1d99116444 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 20:57:07 +0000 Subject: [PATCH 04/30] Fix --- ddprof-lib/src/main/cpp/signalSafety.h | 2 +- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/signalSafety.h b/ddprof-lib/src/main/cpp/signalSafety.h index 44d0d5d4b9..0918472260 100644 --- a/ddprof-lib/src/main/cpp/signalSafety.h +++ b/ddprof-lib/src/main/cpp/signalSafety.h @@ -18,7 +18,7 @@ #define _SIGNAL_SAFETY_H #include "guards.h" // isInSignalContext, SIGNAL_HANDLER_GUARD, ... -#include "threadLocalData.h" // ProfiledThread::current +#include "threadLocalData.inline.h" // ProfiledThread::current // Detect ASAN using compiler-provided macros so the ASAN_ENABLED guard below // works in every TU that includes this header, independent of include order. diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 4923f04933..d7b28a9620 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -7,6 +7,7 @@ #include "threadLocalData.h" #include "threadLocalDataPool.h" +#include #include ThreadLocalDataPool* ThreadLocalDataPool::_pool = nullptr; From 0e216e5e63397344ba3d981e8e96e76e0e1cae5b Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 22:57:31 +0200 Subject: [PATCH 05/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index d7b28a9620..236f78c32f 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -31,8 +31,10 @@ ThreadLocalDataPool::~ThreadLocalDataPool() { } ProfiledThread* ThreadLocalDataPool::claim(int tid) { + if (_threads == nullptr) { + return nullptr; + } uint16_t used = __atomic_fetch_add(&_used, 1, __ATOMIC_RELAXED); - if (used >= _capacity) { __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); return nullptr; } From f4e9eecb0644690a420f5638d36740a7f32c59b2 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 22:59:48 +0200 Subject: [PATCH 06/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/main/cpp/threadLocalDataPool.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 236f78c32f..6bfd8ed3a0 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -12,16 +12,19 @@ ThreadLocalDataPool* ThreadLocalDataPool::_pool = nullptr; -ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) : _capacity(capacity), _used(0) { - size_t malloc_size = capacity * sizeof(ProfiledThread); +ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) + : _capacity(capacity), _used(0), _threads(nullptr) { + const size_t malloc_size = capacity * sizeof(ProfiledThread); void* p = malloc(malloc_size); - if (p != nullptr) { - _threads = reinterpret_cast(p); - for (int index = 0; index < capacity; index++) { - new (&_threads[index])ProfiledThread(0); - } - NativeMem::record(NM_THREAD_LOCAL, malloc_size); + if (p == nullptr) { + return; } + + _threads = reinterpret_cast(p); + for (uint64_t index = 0; index < capacity; index++) { + new (&_threads[index]) ProfiledThread(0); + } + NativeMem::record(NM_THREAD_LOCAL, malloc_size); } ThreadLocalDataPool::~ThreadLocalDataPool() { From 0473e711b62c725dae19b2276d9231eeb7bb95ed Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 23:17:49 +0200 Subject: [PATCH 07/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/threadLocalData.inline.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index a4f60c9806..5274d57a3a 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -10,7 +10,7 @@ #include "threadLocalData.h" #include "threadLocalDataPool.h" -ProfiledThread* ProfiledThread::current() { +inline ProfiledThread* ProfiledThread::current() { if (!isThreadKeyValid()) { return nullptr; } From 84a5f4c7de9291cce48569eea493312b0df846f8 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 21:18:07 +0000 Subject: [PATCH 08/30] Fix merge --- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 6bfd8ed3a0..efc7621056 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -37,7 +37,9 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { if (_threads == nullptr) { return nullptr; } + uint16_t used = __atomic_fetch_add(&_used, 1, __ATOMIC_RELAXED); + if (used >= _capacity) { __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); return nullptr; } From ef0026e1f26826d6b33c575f079f5efcea5ec86c Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 00:35:43 +0200 Subject: [PATCH 09/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index efc7621056..279841bd36 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -52,7 +52,7 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { } index = (index + 1) % _capacity; } while (index != start_pos); - assert(false && "Should not reach here"); + __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); return nullptr; } From c1d60baffbae9da6e1f3728f4641fe10c4fa9462 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 3 Aug 2026 22:38:11 +0000 Subject: [PATCH 10/30] Check wrong thread for priming --- ddprof-lib/src/main/cpp/jvmSupport.cpp | 2 +- ddprof-lib/src/main/cpp/jvmThread.h | 13 ------------- ddprof-lib/src/main/cpp/threadLocal.h | 4 ++++ ddprof-lib/src/main/cpp/threadLocalData.cpp | 11 +++++++++++ ddprof-lib/src/main/cpp/threadLocalData.h | 6 ++++++ 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 0068d211fd..e2d6a9e24e 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -39,7 +39,7 @@ bool JVMSupport::initialize() { return false; } - if (JVMThread::supportPriming()) { + if (ProfiledThread::supportPriming()) { ThreadLocalDataPool::initialize(); } diff --git a/ddprof-lib/src/main/cpp/jvmThread.h b/ddprof-lib/src/main/cpp/jvmThread.h index bbc3a79b9c..d9a3d54e87 100644 --- a/ddprof-lib/src/main/cpp/jvmThread.h +++ b/ddprof-lib/src/main/cpp/jvmThread.h @@ -17,9 +17,6 @@ */ class JVMThread { private: - // PTHREAD_KEY_2NDLEVEL_SIZE is an internal macro set to 32 in the GNU C Library (glibc) NPTL - // implementation. Slot indexes less than PTHREAD_KEY_2NDLEVEL_SIZE are pre-allocated. - static constexpr int PTHREAD_KEY_2NDLEVEL_SIZE = 32; static jfieldID _tid; static ThreadLocal _jvm_thread; @@ -44,16 +41,6 @@ class JVMThread { static inline pthread_key_t key() { return _jvm_thread.key(); } - - static bool supportPriming() { - // Key must be valid - assert(_jvm_thread.isKeyValid()); - if (OS::isMusl()) { - return true; - } else { - return _jvm_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; - } - } static int nativeThreadId(JNIEnv* jni, jthread thread); static inline jlong javaThreadId(JNIEnv* env, jthread thread) { diff --git a/ddprof-lib/src/main/cpp/threadLocal.h b/ddprof-lib/src/main/cpp/threadLocal.h index ffd06713a0..764a80c685 100644 --- a/ddprof-lib/src/main/cpp/threadLocal.h +++ b/ddprof-lib/src/main/cpp/threadLocal.h @@ -86,6 +86,10 @@ class ThreadLocal { return _key != INVALID_KEY; } + pthread_key_t key() const { + return _key; + } + /** * set(nullptr) will result in the value being recreated when get() is called * when CREATE_FUNC is not nullptr. diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 2ff72b896b..8c3f89db34 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -9,6 +9,7 @@ #include "guards.h" #include "otel_context.h" #include "os.h" +#include #include #include @@ -20,6 +21,16 @@ // reads it. ThreadLocal ProfiledThread::_current_thread; +bool ProfiledThread::supportPriming() { + // Key must be valid + assert(_current_thread.isKeyValid()); + if (OS::isMusl()) { + return true; + } else { + return _current_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; + } + } + ProfiledThread* ProfiledThread::initCurrentThread() { if (!isThreadKeyValid()) { return nullptr; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index e5bf9d59b0..65955e3b03 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -39,6 +39,10 @@ class ThreadLocalData { class ProfiledThread : public ThreadLocalData { friend class ThreadLocalDataPool; + // PTHREAD_KEY_2NDLEVEL_SIZE is an internal macro set to 32 in the GNU C Library (glibc) NPTL + // implementation. Slot indexes less than PTHREAD_KEY_2NDLEVEL_SIZE are pre-allocated. + static constexpr int PTHREAD_KEY_2NDLEVEL_SIZE = 32; + public: enum ThreadType : u32 { TYPE_UNKNOWN = 0, @@ -145,6 +149,8 @@ class ProfiledThread : public ThreadLocalData { return _current_thread.isKeyValid(); } + static bool supportPriming(); + #ifdef UNIT_TEST // Simulates the moment inside release() after pthread_setspecific(NULL) but // before delete — the race window the clearCurrentThreadTLS fix covers. From 3b1f48cef146d1928737e2a1b026c6a0b55f2afe Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 00:51:43 +0000 Subject: [PATCH 11/30] Fix pooled profiledThread reset --- ddprof-lib/src/main/cpp/threadLocalData.cpp | 24 +++++++++++++++++++ ddprof-lib/src/main/cpp/threadLocalData.h | 7 ++++-- .../src/main/cpp/threadLocalDataPool.cpp | 6 ++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 8c3f89db34..5a84355acf 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -95,3 +95,27 @@ Context ProfiledThread::snapshotContext(size_t numAttrs) { } return ctx; } + +void ProfiledThread::resetClaimed(int tid) { + _jmp_buf = nullptr; + _pc = 0; + _sp = 0; + _span_id = 0; + _crash_depth = 0; + _tid = tid; + _cpu_epoch = 0; + _wall_epoch = 0; + _call_trace_id = 0; + _recording_epoch = 0; + _misc_flags = FLAG_CLAIMED; + _park_block_token = 0; + _filter_slot_id = 0; + _init_window = 0; + _signal_depth = 0; + _otel_ctx_initialized = false; + _otel_ctx_record = {}; + for (int index = 0; index < DD_TAGS_CAPACITY; index++) { + _otel_tag_encodings[index] = 0; + } + _otel_local_root_span_id = 0; +} \ No newline at end of file diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 65955e3b03..04874ff1ce 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -103,9 +103,9 @@ class ProfiledThread : public ThreadLocalData { alignas(8) u32 _otel_tag_encodings[DD_TAGS_CAPACITY]; u64 _otel_local_root_span_id; - ProfiledThread(int tid, bool claimed = false) + ProfiledThread(int tid) : ThreadLocalData(), _jmp_buf(nullptr), _pc(0), _sp(0), _span_id(0), _crash_depth(0), _tid(tid), _cpu_epoch(0), - _wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(claimed ? FLAG_CLAIMED : 0), + _wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(0), _park_block_token(0), _filter_slot_id(-1), _init_window(0), _signal_depth(0), _otel_ctx_initialized(false), @@ -121,6 +121,9 @@ class ProfiledThread : public ThreadLocalData { virtual ~ProfiledThread() { } + // Reset content of claimed slot + void resetClaimed(int tid); + inline bool isClaimed() const { return (__atomic_load_n(&_misc_flags, __ATOMIC_RELAXED) & FLAG_CLAIMED) == FLAG_CLAIMED; } diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 279841bd36..b635960d18 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -24,7 +24,7 @@ ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) for (uint64_t index = 0; index < capacity; index++) { new (&_threads[index]) ProfiledThread(0); } - NativeMem::record(NM_THREAD_LOCAL, malloc_size); + NativeMem::record(NM_THREAD_LOCAL, malloc_size + sizeof(ThreadLocalDataPool)); } ThreadLocalDataPool::~ThreadLocalDataPool() { @@ -58,7 +58,7 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { bool ThreadLocalDataPool::unclaim(ProfiledThread* t) { if (contains(t)) { - new (t)ProfiledThread(0); + t->unclaim(); uint16_t used = __atomic_fetch_add(&_used, -1, __ATOMIC_RELEASE); assert(used > 0); return true; @@ -78,7 +78,7 @@ ProfiledThread* ThreadLocalDataPool::acquire(int tid) { } else { ProfiledThread* t = pool->claim(tid); if (t != nullptr) { - new (t)ProfiledThread(tid, true /* claimed */); + t->resetClaimed(tid); } return t; } From 01d4b4214706754ad7d458291aea59c4986fd44c Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 01:38:10 +0000 Subject: [PATCH 12/30] Fixes --- ddprof-lib/src/main/cpp/faultInjection.cpp | 2 +- .../src/main/cpp/hotspot/vmStructs.inline.h | 2 +- ddprof-lib/src/main/cpp/jvmThread.h | 3 +-- ddprof-lib/src/main/cpp/profiler.cpp | 1 + ddprof-lib/src/main/cpp/safeAccess.cpp | 2 +- ddprof-lib/src/main/cpp/threadLocalData.cpp | 16 ++++++++-------- ddprof-lib/src/main/cpp/threadLocalData.h | 5 ++--- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 8 +++++--- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 2 +- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/ddprof-lib/src/main/cpp/faultInjection.cpp b/ddprof-lib/src/main/cpp/faultInjection.cpp index d61be77b68..d1389cfb49 100644 --- a/ddprof-lib/src/main/cpp/faultInjection.cpp +++ b/ddprof-lib/src/main/cpp/faultInjection.cpp @@ -22,7 +22,7 @@ #include "counters.h" // Counters::increment (FAULTS_INJECTED) #include "os.h" // OS::page_size -#include "threadLocalData.h" // ProfiledThread::current / nextFiRandom +#include "threadLocalData.inline.h" // ProfiledThread::current / nextFiRandom #include #include diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h index 5de4c94cad..100da7fb48 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h @@ -11,7 +11,7 @@ #include "hotspot/vmStructs.h" #include "jvmThread.h" #include "safeAccess.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" inline bool crashProtectionActive() { ProfiledThread* pt = ProfiledThread::current(); diff --git a/ddprof-lib/src/main/cpp/jvmThread.h b/ddprof-lib/src/main/cpp/jvmThread.h index d9a3d54e87..2f5bd69104 100644 --- a/ddprof-lib/src/main/cpp/jvmThread.h +++ b/ddprof-lib/src/main/cpp/jvmThread.h @@ -10,7 +10,6 @@ #include #include "threadLocal.h" -#include "os.h" /** * JVMThread represents a native JVM thread that is JVM implementation agnostic @@ -41,7 +40,7 @@ class JVMThread { static inline pthread_key_t key() { return _jvm_thread.key(); } - + static int nativeThreadId(JNIEnv* jni, jthread thread); static inline jlong javaThreadId(JNIEnv* env, jthread thread) { return env->GetLongField(thread, _tid); diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 60aa659d8a..24c8c5d67c 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -35,6 +35,7 @@ #include "stackFrame.h" #include "stackWalker.h" #include "symbols.h" +#include "threadLocalData.inline.h" #include "tsc.h" #include "utils.h" #include "wallClock.h" diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index bd637d051a..24d1c6ff6e 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -23,7 +23,7 @@ #include #include #ifdef DEBUG -#include "threadLocalData.h" // ProfiledThread::current / isProtected +#include "threadLocalData.inline.h" // ProfiledThread::current / isProtected #endif extern "C" int safefetch32_cont(int* adr, int errValue); diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 5a84355acf..f79f9f90aa 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -22,14 +22,14 @@ ThreadLocal ProfiledThread::_current_thread; bool ProfiledThread::supportPriming() { - // Key must be valid - assert(_current_thread.isKeyValid()); - if (OS::isMusl()) { - return true; - } else { - return _current_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; - } + // Key must be valid + assert(_current_thread.isKeyValid()); + if (OS::isMusl()) { + return true; + } else { + return _current_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; } +} ProfiledThread* ProfiledThread::initCurrentThread() { if (!isThreadKeyValid()) { @@ -118,4 +118,4 @@ void ProfiledThread::resetClaimed(int tid) { _otel_tag_encodings[index] = 0; } _otel_local_root_span_id = 0; -} \ No newline at end of file +} diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 04874ff1ce..2214b0e85a 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -52,8 +52,7 @@ class ProfiledThread : public ThreadLocalData { }; static constexpr u32 FLAG_PARKED = 0x4u; // next free bit after TYPE_MASK (0x1|0x2) - static constexpr u32 FLAG_CLAIMED = 0x8u; // Used by ThreadLocalDataPool only - + static constexpr u32 FLAG_CLAIMED = 0x8u; // Used by ThreadLocalDataPool only // We are allowing several levels of nesting because we can be // eg. in a crash handler when wallclock signal kicks in, @@ -140,7 +139,7 @@ class ProfiledThread : public ThreadLocalData { u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); return (flags & FLAG_CLAIMED) == 0; -} + } public: static ProfiledThread *forTid(int tid) { diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index b635960d18..4ec649edc9 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -1,5 +1,4 @@ - - /* +/* * Copyright 2026 Datadog, Inc. * SPDX-License-Identifier: Apache-2.0 */ @@ -29,6 +28,9 @@ ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) ThreadLocalDataPool::~ThreadLocalDataPool() { if (_threads != nullptr) { + for (uint64_t index = 0; index < _capacity; index++) { + _threads[index].~ProfiledThread(); + } free(reinterpret_cast(_threads)); } } @@ -91,4 +93,4 @@ bool ThreadLocalDataPool::release(ProfiledThread* t) { } else { return false; } -} \ No newline at end of file +} diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 93f0ec5a0c..a7ed9cf4ca 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -43,4 +43,4 @@ class ThreadLocalDataPool { static bool release(ProfiledThread* t); }; -#endif // THREADLOCALDATA_POOL_H \ No newline at end of file +#endif // THREADLOCALDATA_POOL_H From 0ef3fcf0e5315a3552529db8d0bd7b46e6befb50 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 15:23:26 +0000 Subject: [PATCH 13/30] Review comments --- ddprof-lib/src/main/cpp/counters.h | 1 + ddprof-lib/src/main/cpp/faultInjection.h | 14 ++- .../src/main/cpp/hotspot/hotspotSupport.cpp | 4 +- ddprof-lib/src/main/cpp/jvmSupport.cpp | 3 + ddprof-lib/src/main/cpp/profiler.cpp | 4 +- ddprof-lib/src/main/cpp/stackWalker.cpp | 4 +- ddprof-lib/src/main/cpp/threadLocalData.cpp | 28 +++++- ddprof-lib/src/main/cpp/threadLocalData.h | 24 ++--- .../src/main/cpp/threadLocalData.inline.h | 4 +- .../src/main/cpp/threadLocalDataPool.cpp | 13 +-- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 32 ++++++ ddprof-lib/src/main/cpp/unwindStats.cpp | 17 ++++ ddprof-lib/src/main/cpp/unwindStats.h | 15 +-- .../src/test/cpp/threadLocalDataPool_ut.cpp | 98 +++++++++++++++++++ 14 files changed, 218 insertions(+), 43 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index deb6e40d9a..4d41867ba0 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -134,6 +134,7 @@ X(SAFECOPY_FAILED, "safecopy_failed") \ X(SAFEFETCH_FAILED, "safefetch_failed") \ X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ + X(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED, "thread_local_pool_exhausted") \ DD_COUNTER_TABLE_FAULT_INJECTION(X) \ DD_COUNTER_TABLE_FI_DEBUG(X) \ DD_COUNTER_TABLE_DEBUG(X) diff --git a/ddprof-lib/src/main/cpp/faultInjection.h b/ddprof-lib/src/main/cpp/faultInjection.h index 5ac8ead3ba..9729668e97 100644 --- a/ddprof-lib/src/main/cpp/faultInjection.h +++ b/ddprof-lib/src/main/cpp/faultInjection.h @@ -34,8 +34,9 @@ // // return INJECT_FAULT_BOOL_LIKELY(dlopen(name, flags) != nullptr); // -// The three tiers name their firing frequency: RARE 0.01%, UNLIKELY 0.1%, -// LIKELY 1%. See faultInjection.cpp for the poison-address and PRNG details. +// The four tiers name their firing frequency: RARE 0.01%, UNLIKELY 0.1%, +// LIKELY 1%, HIGH 10%. See faultInjection.cpp for the poison-address and PRNG +// details. #ifndef _FAULT_INJECTION_H #define _FAULT_INJECTION_H @@ -56,6 +57,7 @@ namespace faultinj { constexpr u64 PROB_RARE = 1844674407370955ULL; // 1e-4 (0.01%) constexpr u64 PROB_UNLIKELY = 18446744073709552ULL; // 1e-3 (0.1%) constexpr u64 PROB_LIKELY = 184467440737095520ULL; // 1e-2 (1%) +constexpr u64 PROB_HIGH = 1844674407370955162ULL; // 1e-1 (10%) // Called once at profiler startup (off the signal path) to mmap the PROT_NONE // guard region used by poisonAddress(). Safe to call before any injection. @@ -106,6 +108,8 @@ inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { ::faultinj::injectAddress((ptr), ::faultinj::PROB_UNLIKELY, __func__) #define INJECT_FAULT_ADDRESS_LIKELY(ptr) \ ::faultinj::injectAddress((ptr), ::faultinj::PROB_LIKELY, __func__) +#define INJECT_FAULT_ADDRESS_HIGH(ptr) \ + ::faultinj::injectAddress((ptr), ::faultinj::PROB_HIGH, __func__) #define INJECT_FAULT_BOOL_RARE(v) \ ::faultinj::injectValue((v), false, ::faultinj::PROB_RARE, __func__) @@ -113,24 +117,30 @@ inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { ::faultinj::injectValue((v), false, ::faultinj::PROB_UNLIKELY, __func__) #define INJECT_FAULT_BOOL_LIKELY(v) \ ::faultinj::injectValue((v), false, ::faultinj::PROB_LIKELY, __func__) +#define INJECT_FAULT_BOOL_HIGH(v) \ + ::faultinj::injectValue((v), false, ::faultinj::PROB_HIGH, __func__) #else // __FAULT_INJECTION__ not defined — strict identity, zero cost. #define INJECT_FAULT_ADDRESS_RARE(ptr) (ptr) #define INJECT_FAULT_ADDRESS_UNLIKELY(ptr) (ptr) #define INJECT_FAULT_ADDRESS_LIKELY(ptr) (ptr) +#define INJECT_FAULT_ADDRESS_HIGH(ptr) (ptr) #define INJECT_FAULT_INT_RARE(v) (v) #define INJECT_FAULT_INT_UNLIKELY(v) (v) #define INJECT_FAULT_INT_LIKELY(v) (v) +#define INJECT_FAULT_INT_HIGH(v) (v) #define INJECT_FAULT_LONG_RARE(v) (v) #define INJECT_FAULT_LONG_UNLIKELY(v) (v) #define INJECT_FAULT_LONG_LIKELY(v) (v) +#define INJECT_FAULT_LONG_HIGH(v) (v) #define INJECT_FAULT_BOOL_RARE(v) (v) #define INJECT_FAULT_BOOL_UNLIKELY(v) (v) #define INJECT_FAULT_BOOL_LIKELY(v) (v) +#define INJECT_FAULT_BOOL_HIGH(v) (v) #define NO_INJECTION_ASSERT(a) (assert(a)) diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index d26fbb56c2..bb2c3233bd 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -241,7 +241,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // VMStructs is only available for hotspot JVM assert(VM::isHotspot()); - ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); if (prof_thread == nullptr) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; @@ -1211,7 +1211,7 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { // AsyncGetCallTrace. Install a jmp ctx here too, so a SIGSEGV anywhere in // walkJavaStack, except HotSpot's AsyncGetCallTrace call, is caught by // Profiler::checkFault() and siglongjmp'd back here instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); if (prof_thread == nullptr) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index e2d6a9e24e..b1e4a728d0 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -6,6 +6,7 @@ #include "jvmSupport.inline.h" #include "asyncSampleMutex.h" +#include "common.h" #include "frames.h" #include "os.h" #include "profiler.h" @@ -41,6 +42,8 @@ bool JVMSupport::initialize() { if (ProfiledThread::supportPriming()) { ThreadLocalDataPool::initialize(); + } else { + LOG_WARN("Thread priming is not supported"); } // Check ProfiledThread key, it is critical for storing per-thread metadata diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 24c8c5d67c..6f87abe7a6 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -866,10 +866,10 @@ bool Profiler::prewarmUnwinder() { // libgcc_s.so.1 has been the stable SONAME since 2002; a bump would // constitute a glibc/GCC C++ ABI break and is treated as a fixed contract. // - // INJECT_FAULT_BOOL_LIKELY lets fault-injection builds force this to + // INJECT_FAULT_BOOL_HIGH lets fault-injection builds force this to // report failure without the library actually being absent, so // checkState()'s "Missing libgcc_s.so" path can be exercised in CI. - return INJECT_FAULT_BOOL_LIKELY(dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr); + return INJECT_FAULT_BOOL_HIGH(dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr); #else return true; #endif diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index 3912cfebd3..9abcf2c61b 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -45,7 +45,7 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); if (prof_thread == nullptr) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; @@ -134,7 +134,7 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::acquire_current(); + ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); if (prof_thread == nullptr) { Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); return 0; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index f79f9f90aa..456efe9fa7 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include "faultInjection.h" #include "threadLocalData.inline.h" #include "threadLocalDataPool.h" #include "context_api.h" @@ -26,9 +27,18 @@ bool ProfiledThread::supportPriming() { assert(_current_thread.isKeyValid()); if (OS::isMusl()) { return true; - } else { - return _current_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; } +#ifdef __GLIBC__ + bool rc = _current_thread.key() < PTHREAD_KEY_2NDLEVEL_SIZE; + return INJECT_FAULT_BOOL_HIGH(rc); +#else + // Neither musl nor glibc (e.g. macOS libpthread): PTHREAD_KEY_2NDLEVEL_SIZE + // is a glibc NPTL implementation detail (see threadLocalData.h) that doesn't + // describe this libc's pthread_key_t allocation scheme. Fail safe by + // disabling signal-handler TLS priming rather than assuming glibc-compatible + // pthread_setspecific behavior. + return false; +#endif } ProfiledThread* ProfiledThread::initCurrentThread() { @@ -74,6 +84,15 @@ void ProfiledThread::release() { _current_thread.clear(); } +#ifdef UNIT_TEST +void ProfiledThread::deleteForTest(ProfiledThread* pt) { + if (!ThreadLocalDataPool::release(pt)) { + delete pt; + NativeMem::record(NM_THREAD_LOCAL, -(long long)sizeof(ProfiledThread)); + } +} +#endif + int ProfiledThread::currentTid() { ProfiledThread *tls = current(); if (tls != NULL) { @@ -109,7 +128,7 @@ void ProfiledThread::resetClaimed(int tid) { _recording_epoch = 0; _misc_flags = FLAG_CLAIMED; _park_block_token = 0; - _filter_slot_id = 0; + _filter_slot_id = -1; _init_window = 0; _signal_depth = 0; _otel_ctx_initialized = false; @@ -118,4 +137,7 @@ void ProfiledThread::resetClaimed(int tid) { _otel_tag_encodings[index] = 0; } _otel_local_root_span_id = 0; + _in_critical_section = false; + + _unwind_failures.reset(); } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 2214b0e85a..09e3665fbf 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -41,6 +41,9 @@ class ProfiledThread : public ThreadLocalData { // PTHREAD_KEY_2NDLEVEL_SIZE is an internal macro set to 32 in the GNU C Library (glibc) NPTL // implementation. Slot indexes less than PTHREAD_KEY_2NDLEVEL_SIZE are pre-allocated. + // glibc-specific: only meaningful under the __GLIBC__ branch of supportPriming() + // (threadLocalData.cpp). Other libcs (musl, macOS libpthread) don't share this + // layout and must not be routed through this constant. static constexpr int PTHREAD_KEY_2NDLEVEL_SIZE = 32; public: @@ -132,7 +135,7 @@ class ProfiledThread : public ThreadLocalData { __atomic_fetch_and(&_misc_flags, ~FLAG_CLAIMED, __ATOMIC_RELEASE); } - inline bool claim_acquire() { + inline bool claimAcquire() { if (isClaimed()) { return false; } @@ -163,16 +166,15 @@ class ProfiledThread : public ThreadLocalData { _current_thread.set(nullptr); return pt; } - // Deletes a ProfiledThread returned by clearCurrentThreadTLS(). - // Needed because the destructor is private. This stands in for the delete - // that freeValue() performs in production, so it mirrors freeValue()'s - // NM_THREAD_LOCAL decrement to keep the accounting balanced in tests. - static void deleteForTest(ProfiledThread *pt) { - delete pt; - NativeMem::record(NM_THREAD_LOCAL, -(long long)sizeof(ProfiledThread)); - } + // Releases a ProfiledThread returned by clearCurrentThreadTLS(). + // Needed because the destructor is private. Mirrors freeValue()'s + // ThreadLocalDataPool::release()-then-delete logic (and its NM_THREAD_LOCAL + // decrement) so it's safe to call on both forTid()-obtained and pool-backed + // threads. Defined in threadLocalData.cpp, where ThreadLocalDataPool's full + // declaration is visible. + static void deleteForTest(ProfiledThread *pt); #endif - // initCurrentThread() and release() are not async-signal-safe: + // initCurrentThread() and release() are not async-signal-safe: // must be called outside of a signal handler with signal blocked static ProfiledThread* initCurrentThread(); static void release(); @@ -188,7 +190,7 @@ class ProfiledThread : public ThreadLocalData { static inline ProfiledThread *current(); // signal-handler friendly with priming: return existing TLS or acquire and set // ProfiledThread from ThreadLocalDataPool. - static inline ProfiledThread* acquire_current(); + static inline ProfiledThread* acquireCurrent(); static int currentTid(); inline int tid() { return _tid; } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index 5274d57a3a..2d9b1b6c26 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -6,6 +6,7 @@ #ifndef THREADLOCALDATA_INLINE_H #define THREADLOCALDATA_INLINE_H +#include "guards.h" #include "os.h" #include "threadLocalData.h" #include "threadLocalDataPool.h" @@ -17,11 +18,12 @@ inline ProfiledThread* ProfiledThread::current() { return _current_thread.get(); } -ProfiledThread* ProfiledThread::acquire_current() { +ProfiledThread* ProfiledThread::acquireCurrent() { ProfiledThread* prof_thread = current(); if (prof_thread == nullptr) { prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); if (prof_thread != nullptr) { + SignalBlocker blocker; _current_thread.set(prof_thread); } } diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 4ec649edc9..1b25be60c0 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include "counters.h" #include "threadLocalData.h" #include "threadLocalDataPool.h" @@ -26,14 +27,6 @@ ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) NativeMem::record(NM_THREAD_LOCAL, malloc_size + sizeof(ThreadLocalDataPool)); } -ThreadLocalDataPool::~ThreadLocalDataPool() { - if (_threads != nullptr) { - for (uint64_t index = 0; index < _capacity; index++) { - _threads[index].~ProfiledThread(); - } - free(reinterpret_cast(_threads)); - } -} ProfiledThread* ThreadLocalDataPool::claim(int tid) { if (_threads == nullptr) { @@ -49,12 +42,13 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { int start_pos = tid % _capacity; int index = start_pos; do { - if (_threads[index].claim_acquire()) { + if (_threads[index].claimAcquire()) { return &_threads[index]; } index = (index + 1) % _capacity; } while (index != start_pos); __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); + Counters::increment(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); return nullptr; } @@ -69,6 +63,7 @@ bool ThreadLocalDataPool::unclaim(ProfiledThread* t) { } void ThreadLocalDataPool::initialize() { + // process-lifetime singleton ThreadLocalDataPool* pool = new ThreadLocalDataPool(); __atomic_store_n(&_pool, pool, __ATOMIC_RELEASE); } diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index a7ed9cf4ca..43e64749cd 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -7,6 +7,8 @@ #define THREADLOCALDATA_POOL_H #include +#include +#include class ProfiledThread; @@ -41,6 +43,36 @@ class ThreadLocalDataPool { static void initialize(); static ProfiledThread* acquire(int tid); static bool release(ProfiledThread* t); + +#ifdef UNIT_TEST + // Test-only: a pool isolated from the process-wide singleton (_pool), so + // contains()/boundary tests don't disturb other tests' use of + // initialize()/acquire()/release(). + static ThreadLocalDataPool* createForTest(uint64_t capacity) { + return new ThreadLocalDataPool(capacity); + } + // ThreadLocalDataPool has no destructor definition (it's a process-lifetime + // singleton in production, never freed), so `delete p` won't link. Mirror + // what a destructor would do -- destroy each placement-newed ProfiledThread + // and free() the malloc'd buffer -- then release the ThreadLocalDataPool + // object itself via the deallocation function directly, without invoking a + // (nonexistent) destructor. + static void destroyForTest(ThreadLocalDataPool* p) { + if (p->_threads != nullptr) { + for (uint64_t index = 0; index < p->_capacity; index++) { + p->_threads[index].~ProfiledThread(); + } + free(reinterpret_cast(p->_threads)); + } + ::operator delete(p); + } + + bool containsForTest(ProfiledThread* t) const { return contains(t); } + ProfiledThread* threadsForTest() const { return _threads; } + uint64_t capacityForTest() const { return _capacity; } + ProfiledThread* claimForTest(int tid) { return claim(tid); } + bool unclaimForTest(ProfiledThread* t) { return unclaim(t); } +#endif }; #endif // THREADLOCALDATA_POOL_H diff --git a/ddprof-lib/src/main/cpp/unwindStats.cpp b/ddprof-lib/src/main/cpp/unwindStats.cpp index 82a38cf173..abc8123034 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.cpp +++ b/ddprof-lib/src/main/cpp/unwindStats.cpp @@ -3,3 +3,20 @@ // initialize static members SpinLock UnwindStats::_lock; UnwindFailures UnwindStats::_unwind_failures; + +UnwindFailures::UnwindFailures() : _nameCount(0) { + _names = new char[MAX_UNWIND_FAILURE_NAMES][MAX_NAME_LENGTH]; + _counters = new u64[MAX_UNWIND_FAILURE_NAMES][UNWIND_FAILURE_ANY + 1]; + reset(); +} + +UnwindFailures::~UnwindFailures() { + delete[] _names; + delete[] _counters; + } + + +void UnwindFailures::reset() { + memset((void*)_names, 0, MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH); + memset((void*)_counters, 0, MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64)); +} diff --git a/ddprof-lib/src/main/cpp/unwindStats.h b/ddprof-lib/src/main/cpp/unwindStats.h index 1eb4eab297..5a9a75845b 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.h +++ b/ddprof-lib/src/main/cpp/unwindStats.h @@ -25,22 +25,15 @@ class UnwindFailures { volatile u64 (*_counters)[UNWIND_FAILURE_ANY + 1]; public: - UnwindFailures() : _nameCount(0) { - _names = new char[MAX_UNWIND_FAILURE_NAMES][MAX_NAME_LENGTH]; - _counters = new u64[MAX_UNWIND_FAILURE_NAMES][UNWIND_FAILURE_ANY + 1]; - memset((void*)_names, 0, MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH); - memset((void*)_counters, 0, MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64)); - } - - ~UnwindFailures() { - delete[] _names; - delete[] _counters; - } + UnwindFailures(); + ~UnwindFailures(); // Disable copy constructor and assignment operator UnwindFailures(const UnwindFailures&) = delete; UnwindFailures& operator=(const UnwindFailures&) = delete; + void reset(); + void record(UnwindFailureKind kind, const char *name) { if (!name) return; diff --git a/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp new file mode 100644 index 0000000000..038354e119 --- /dev/null +++ b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp @@ -0,0 +1,98 @@ +/* + * Copyright 2026 Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +// threadLocalData.h must precede threadLocalDataPool.h: ProfiledThread needs to +// be a complete type before contains()'s pointer arithmetic is parsed (the +// pool header only forward-declares it). +#include "threadLocalData.h" +#include "threadLocalDataPool.h" +#include "counters.h" + +// Covers ThreadLocalDataPool::contains(), whose result feeds directly into +// unclaim()'s double-release guard. Uses createForTest()/destroyForTest() to +// build a pool isolated from the process-wide singleton (_pool), so these +// boundary checks don't interact with other tests' initialize()/acquire()/ +// release() calls. +class ThreadLocalDataPoolTest : public ::testing::Test {}; + +TEST_F(ThreadLocalDataPoolTest, firstElementIsContained) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(4); + ProfiledThread* base = pool->threadsForTest(); + + EXPECT_TRUE(pool->containsForTest(base)); + + ThreadLocalDataPool::destroyForTest(pool); +} + +TEST_F(ThreadLocalDataPoolTest, lastElementIsContained) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(4); + ProfiledThread* base = pool->threadsForTest(); + uint64_t capacity = pool->capacityForTest(); + + EXPECT_TRUE(pool->containsForTest(base + (capacity - 1))); + + ThreadLocalDataPool::destroyForTest(pool); +} + +TEST_F(ThreadLocalDataPoolTest, onePastEndIsNotContained) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(4); + ProfiledThread* base = pool->threadsForTest(); + uint64_t capacity = pool->capacityForTest(); + + EXPECT_FALSE(pool->containsForTest(base + capacity)); + + ThreadLocalDataPool::destroyForTest(pool); +} + +TEST_F(ThreadLocalDataPoolTest, oneBeforeStartIsNotContained) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(4); + ProfiledThread* base = pool->threadsForTest(); + + EXPECT_FALSE(pool->containsForTest(base - 1)); + + ThreadLocalDataPool::destroyForTest(pool); +} + +TEST_F(ThreadLocalDataPoolTest, nullptrIsNotContained) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(4); + + EXPECT_FALSE(pool->containsForTest(nullptr)); + + ThreadLocalDataPool::destroyForTest(pool); +} + +// Covers claim()'s (used >= _capacity) fast-path rejection once the pool is +// full. claim()'s return value is nullptr whether that guard reads `>=` or +// (bugged) `>`, because a bugged `>` still falls through to the slot-scanning +// loop, which finds every real slot already claimed and also returns nullptr. +// The observable difference is that the buggy fallthrough additionally +// increments SAMPLES_DROPPED_TLS_POOL_EXHAUSTED, which the fast path must not +// do -- that's what this test pins down. +TEST_F(ThreadLocalDataPoolTest, claimAtCapacityRejectsWithoutExhaustionScan) { + ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(2); + + ASSERT_NE(pool->claimForTest(0), nullptr); + ASSERT_NE(pool->claimForTest(1), nullptr); + + long long before = Counters::getCounter(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); + EXPECT_EQ(pool->claimForTest(2), nullptr); + long long after = Counters::getCounter(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); + + EXPECT_EQ(after, before); + + ThreadLocalDataPool::destroyForTest(pool); +} From d02c156bb75f2bc8e89b7c99c51074ef7800a727 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 16:30:38 +0000 Subject: [PATCH 14/30] Test TLS priming test --- .../profiler/cpu/TlsPrimingTest.java | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java new file mode 100644 index 0000000000..3548b965a9 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java @@ -0,0 +1,157 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package com.datadoghq.profiler.cpu; + +import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.Platform; +import org.junitpioneer.jupiter.RetryingTest; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.openjdk.jmc.common.item.IItem; +import org.openjdk.jmc.common.item.IItemCollection; +import org.openjdk.jmc.common.item.IItemIterable; +import org.openjdk.jmc.common.item.IMemberAccessor; +import org.openjdk.jmc.flightrecorder.jdk.JdkAttributes; + +import java.lang.reflect.Method; +import java.util.TreeSet; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Validates TLS priming by checking that CPU-time profiling captures samples + * from the JVM's JIT compiler threads (HotSpot's "C1 CompilerThread*", + * OpenJ9's "JIT Compilation Thread*"). + * + * Compiler threads are started very early during JVM bootstrap, usually + * before the profiler agent has attached and initialized. Because of that, + * they never go through the normal thread-registration path the profiler + * uses for application threads, so they have no ProfiledThread attached when + * the first profiling signal reaches them. TLS priming is what covers this + * gap: on that first signal, ProfiledThread::acquireCurrent() claims a slot + * from the pool and attaches it via pthread_setspecific right there in the + * signal handler (see threadLocalData.cpp/ThreadLocalDataPool). If priming + * were broken, these early-started threads would simply never show up as + * eventThread on a sample. So seeing compiler-thread samples here is direct + * evidence that TLS priming worked. + * + * The test forces JIT compilation by loading a dynamically-generated class + * with many distinct trivial methods and invoking each one past HotSpot's/ + * OpenJ9's cold-to-compiled threshold, then leaves the CPU sampler running + * long enough for the background compiler thread(s) to actually drain the + * resulting compile queue. + */ +public class TlsPrimingTest extends AbstractProfilerTest { + + private static final String HOTSPOT_COMPILER_THREAD_PREFIX = "C1 CompilerThre"; + private static final String J9_COMPILER_THREAD_PREFIX = "JIT Compilation Thread"; + + // Distinct methods, each invoked enough times to individually cross the + // JIT's cold-to-compiled invocation threshold, so the compiler queue has + // sustained work rather than a single instantly-finished compile. + private static final int METHOD_COUNT = 300; + private static final int INVOCATIONS_PER_METHOD = 1000; + + // Compilation happens asynchronously on background compiler threads, so + // after tripping the invocation thresholds we must give them real time to + // drain the queue while the CPU sampler is still active. + private static final long COMPILE_DRAIN_WAIT_MS = 4000; + + @RetryingTest(3) + public void compilerThreadSamplesArePresent() throws Exception { + triggerJitCompilation(); + + stopProfiler(); + + IItemCollection events = verifyEvents("datadog.ExecutionSample"); + String expectedPrefix = Platform.isJ9() ? J9_COMPILER_THREAD_PREFIX : HOTSPOT_COMPILER_THREAD_PREFIX; + + Set observedThreadNames = new TreeSet<>(); + boolean sawCompilerThreadSample = false; + for (IItemIterable cpuSamples : events) { + IMemberAccessor threadNameAccessor = + JdkAttributes.EVENT_THREAD_NAME.getAccessor(cpuSamples.getType()); + for (IItem sample : cpuSamples) { + String threadName = threadNameAccessor.getMember(sample); + if (threadName == null) { + continue; + } + observedThreadNames.add(threadName); + if (threadName.startsWith(expectedPrefix)) { + sawCompilerThreadSample = true; + } + } + } + + assertTrue(sawCompilerThreadSample, + "expected a datadog.ExecutionSample with eventThread starting with \"" + expectedPrefix + + "\", but observed thread names: " + observedThreadNames); + } + + private void triggerJitCompilation() throws Exception { + Class generated = defineWorkloadClass(); + Object instance = generated.getDeclaredConstructor().newInstance(); + Method[] methods = generated.getDeclaredMethods(); + + for (int call = 0; call < INVOCATIONS_PER_METHOD; call++) { + for (Method m : methods) { + m.invoke(instance); + } + } + + Thread.sleep(COMPILE_DRAIN_WAIT_MS); + } + + /** + * Generates a class with {@value #METHOD_COUNT} distinct no-arg int-returning + * methods (each a different constant expression, so the JIT can't fold them + * into one shared compiled method) and loads it in a fresh ClassLoader. + */ + private static Class defineWorkloadClass() throws ClassNotFoundException { + String internalName = "com/datadoghq/profiler/cpu/generated/CompilerThreadWorkload"; + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, internalName, null, "java/lang/Object", null); + + MethodVisitor ctor = cw.visitMethod(Opcodes.ACC_PUBLIC, "", "()V", null, null); + ctor.visitCode(); + ctor.visitVarInsn(Opcodes.ALOAD, 0); + ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "", "()V", false); + ctor.visitInsn(Opcodes.RETURN); + ctor.visitMaxs(0, 0); + ctor.visitEnd(); + + for (int i = 0; i < METHOD_COUNT; i++) { + MethodVisitor mv = cw.visitMethod(Opcodes.ACC_PUBLIC, "method" + i, "()I", null, null); + mv.visitCode(); + mv.visitIntInsn(Opcodes.SIPUSH, i); + mv.visitIntInsn(Opcodes.SIPUSH, i + 1); + mv.visitInsn(Opcodes.IMUL); + mv.visitInsn(Opcodes.IRETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + } + cw.visitEnd(); + + IsolatedClassLoader loader = new IsolatedClassLoader(TlsPrimingTest.class.getClassLoader()); + return loader.defineClass(internalName.replace('/', '.'), cw.toByteArray()); + } + + private static final class IsolatedClassLoader extends ClassLoader { + IsolatedClassLoader(ClassLoader parent) { + super(parent); + } + + Class defineClass(String name, byte[] bytecode) { + return defineClass(name, bytecode, 0, bytecode.length); + } + } + + @Override + protected String getProfilerCommand() { + return "cpu=1ms"; + } +} From c3e0c74e8f0f9fe75ebad1655185a68df38731b8 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 19:13:30 +0000 Subject: [PATCH 15/30] Cleanup and test --- ddprof-lib/src/main/cpp/threadLocalData.cpp | 5 +++++ ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 13 ++++++++++++- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 7 +++++-- ddprof-lib/src/main/cpp/unwindStats.cpp | 3 +-- .../com/datadoghq/profiler/cpu/TlsPrimingTest.java | 2 +- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 456efe9fa7..0fc7ff377b 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -140,4 +140,9 @@ void ProfiledThread::resetClaimed(int tid) { _in_critical_section = false; _unwind_failures.reset(); + + #ifdef __FAULT_INJECTION__ + _fi_rng = ((u64)(uintptr_t)this) ^ (0x9e3779b97f4a7c15ULL * (u64)tid); + if (_fi_rng == 0) _fi_rng = 1; +#endif } diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 1b25be60c0..73f3bbf2f9 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -12,7 +12,7 @@ ThreadLocalDataPool* ThreadLocalDataPool::_pool = nullptr; -ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) +ThreadLocalDataPool::ThreadLocalDataPool(uint16_t capacity) : _capacity(capacity), _used(0), _threads(nullptr) { const size_t malloc_size = capacity * sizeof(ProfiledThread); void* p = malloc(malloc_size); @@ -27,6 +27,17 @@ ThreadLocalDataPool::ThreadLocalDataPool(uint64_t capacity) NativeMem::record(NM_THREAD_LOCAL, malloc_size + sizeof(ThreadLocalDataPool)); } +#ifdef UNIT_TEST +ThreadLocalDataPool::~ThreadLocalDataPool() { + if (_threads != nullptr) { + for (int index = 0; index < _capacity; index++) { + _threads[index].~ProfiledThread(); + } + free(reinterpret_cast(_threads)); + } +} +#endif // UNIT_TEST + ProfiledThread* ThreadLocalDataPool::claim(int tid) { if (_threads == nullptr) { diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 43e64749cd..4fb4deb765 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -24,9 +24,12 @@ class ThreadLocalDataPool { ThreadLocalDataPool(const ThreadLocalDataPool&) = delete; ThreadLocalDataPool& operator=(const ThreadLocalDataPool&) = delete; - ThreadLocalDataPool(uint64_t capacity = DEFAULT_CAPACITY); + ThreadLocalDataPool(uint16_t capacity = DEFAULT_CAPACITY); +#ifdef UNIT_TEST ~ThreadLocalDataPool(); - +#else + ~ThreadLocalDataPool() = delete; +#endif // UNIT_TEST ProfiledThread* claim(int tid); bool unclaim(ProfiledThread* t); diff --git a/ddprof-lib/src/main/cpp/unwindStats.cpp b/ddprof-lib/src/main/cpp/unwindStats.cpp index abc8123034..86c99dbcea 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.cpp +++ b/ddprof-lib/src/main/cpp/unwindStats.cpp @@ -13,8 +13,7 @@ UnwindFailures::UnwindFailures() : _nameCount(0) { UnwindFailures::~UnwindFailures() { delete[] _names; delete[] _counters; - } - +} void UnwindFailures::reset() { memset((void*)_names, 0, MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java index 3548b965a9..8ae20dc728 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java @@ -111,7 +111,7 @@ private void triggerJitCompilation() throws Exception { * methods (each a different constant expression, so the JIT can't fold them * into one shared compiled method) and loads it in a fresh ClassLoader. */ - private static Class defineWorkloadClass() throws ClassNotFoundException { + private static Class defineWorkloadClass() throws ClassFormatError { String internalName = "com/datadoghq/profiler/cpu/generated/CompilerThreadWorkload"; ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, internalName, null, "java/lang/Object", null); From 38e7faa50b81d4ef273da9c35518653ec2687bc6 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 4 Aug 2026 19:40:04 +0000 Subject: [PATCH 16/30] Cleanup includes --- ddprof-lib/src/test/cpp/faultInjection_ut.cpp | 2 +- .../src/test/cpp/hotspot_crash_protection_ut.cpp | 2 +- ddprof-lib/src/test/cpp/stackWalker_ut.cpp | 14 +++++++------- .../src/test/cpp/stress_threadLifecycle_ut.cpp | 2 +- .../src/test/cpp/threadFilter_lifecycle_ut.cpp | 2 +- .../src/test/cpp/thread_teardown_safety_ut.cpp | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index f0a49354e2..ab0102dee1 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -14,7 +14,7 @@ #include "faultInjection.h" #include "safeAccess.h" #include "os.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "profiler.h" #include "../../main/cpp/gtest_crash_handler.h" diff --git a/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp b/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp index 3739606bbb..2131ecac04 100644 --- a/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp @@ -30,7 +30,7 @@ */ #include -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "profiler.h" #include "asyncSampleMutex.h" diff --git a/ddprof-lib/src/test/cpp/stackWalker_ut.cpp b/ddprof-lib/src/test/cpp/stackWalker_ut.cpp index 1212b704d2..efe19492da 100644 --- a/ddprof-lib/src/test/cpp/stackWalker_ut.cpp +++ b/ddprof-lib/src/test/cpp/stackWalker_ut.cpp @@ -3,18 +3,18 @@ */ #include -#include "../../main/cpp/stackWalker.h" -#include "../../main/cpp/gtest_crash_handler.h" +#include "stackWalker.h" +#include "gtest_crash_handler.h" #ifdef __linux__ #include #include #include -#include "../../main/cpp/counters.h" -#include "../../main/cpp/os.h" -#include "../../main/cpp/profiler.h" -#include "../../main/cpp/stackFrame.h" -#include "../../main/cpp/threadLocalData.h" +#include "counters.h" +#include "os.h" +#include "profiler.h" +#include "stackFrame.h" +#include "threadLocalData.inline.h" [[maybe_unused]] static long long* _stackwalker_ut_counters_init = Counters::getCounters(); #endif diff --git a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp index 5615010e0a..8ad8a336e9 100644 --- a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp +++ b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp @@ -16,7 +16,7 @@ #include "callTraceStorage.h" #include "callTraceHashTable.h" #include "threadFilter.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "arch.h" #include "spinLock.h" diff --git a/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp index 9ca9b082e1..dab8333f04 100644 --- a/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadFilter_lifecycle_ut.cpp @@ -19,7 +19,7 @@ #ifdef __linux__ #include "threadFilter.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "../../main/cpp/gtest_crash_handler.h" #include 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 02f8c3f50c..9195f27ad5 100644 --- a/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp +++ b/ddprof-lib/src/test/cpp/thread_teardown_safety_ut.cpp @@ -20,7 +20,7 @@ #include "guards.h" #include "nativeMem.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include #include From bdf14979078eb0f80bc77c640769424f13f8d129 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 12:47:44 +0000 Subject: [PATCH 17/30] Fix --- ddprof-lib/src/main/cpp/threadLocalData.inline.h | 2 +- ddprof-lib/src/main/cpp/unwindStats.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index 2d9b1b6c26..0f8889f18f 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -21,9 +21,9 @@ inline ProfiledThread* ProfiledThread::current() { ProfiledThread* ProfiledThread::acquireCurrent() { ProfiledThread* prof_thread = current(); if (prof_thread == nullptr) { + SignalBlocker blocker; prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); if (prof_thread != nullptr) { - SignalBlocker blocker; _current_thread.set(prof_thread); } } diff --git a/ddprof-lib/src/main/cpp/unwindStats.cpp b/ddprof-lib/src/main/cpp/unwindStats.cpp index 86c99dbcea..5852977794 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.cpp +++ b/ddprof-lib/src/main/cpp/unwindStats.cpp @@ -18,4 +18,5 @@ UnwindFailures::~UnwindFailures() { void UnwindFailures::reset() { memset((void*)_names, 0, MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH); memset((void*)_counters, 0, MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64)); + _nameCount = 0; } From 210552553074502fc22d9ae7811e9125a196edca Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 16:53:30 +0000 Subject: [PATCH 18/30] Fix CriticalSection --- ddprof-lib/src/main/cpp/guards.cpp | 11 +++++++++++ ddprof-lib/src/main/cpp/threadLocalData.inline.h | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/ddprof-lib/src/main/cpp/guards.cpp b/ddprof-lib/src/main/cpp/guards.cpp index f0651d7a2c..b3c0c6765d 100644 --- a/ddprof-lib/src/main/cpp/guards.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -102,6 +102,17 @@ CriticalSection::~CriticalSection() { // Clear the bit atomically for fallback bitmap // Use RELEASE ordering to ensure protected data writes are visible before releasing __atomic_fetch_and(&_fallback_bitmap[_word_index], ~_bit_mask, __ATOMIC_RELEASE); + + // ProfiledThread::current() was null when we entered via the fallback + // path, but TLS may have been published during this critical section's + // lifetime (ProfiledThread::acquireCurrent(), invoked from stack walking + // inside recordSample). acquireCurrent() marks the newly-published + // ProfiledThread's critical section held before exposing it; release + // that here too, otherwise it would stay stuck at true forever. + ProfiledThread* published = ProfiledThread::current(); + if (published != nullptr) { + published->exitCriticalSection(); + } } else { // Release ProfiledThread flag using the pointer captured at construction if (_thread_ptr != nullptr) { diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index 0f8889f18f..ae7a69f533 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -24,6 +24,13 @@ ProfiledThread* ProfiledThread::acquireCurrent() { SignalBlocker blocker; prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); if (prof_thread != nullptr) { + // Claim the critical section before publishing the pointer. A signal + // that interrupts us on this thread right after publish (cross-type + // nesting isn't blocked — see os_linux.cpp's empty sa_mask) would + // otherwise see a non-null ProfiledThread whose _in_critical_section + // is still false and race into the primary path. CriticalSection's + // fallback-path destructor releases this once the outer handler exits. + prof_thread->tryEnterCriticalSection(); _current_thread.set(prof_thread); } } From 72e3d0da85d43c46ad14f077e5be3a7e32f0ae40 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 19:53:46 +0200 Subject: [PATCH 19/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/threadLocalData.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 0fc7ff377b..3229756932 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -122,11 +122,10 @@ void ProfiledThread::resetClaimed(int tid) { _span_id = 0; _crash_depth = 0; _tid = tid; - _cpu_epoch = 0; _wall_epoch = 0; _call_trace_id = 0; _recording_epoch = 0; - _misc_flags = FLAG_CLAIMED; + __atomic_store_n(&_misc_flags, FLAG_CLAIMED, __ATOMIC_RELEASE); _park_block_token = 0; _filter_slot_id = -1; _init_window = 0; From d8057ec8f7b1e877386ea6ab137ec5d376bd3822 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 18:59:56 +0000 Subject: [PATCH 20/30] Fix --- ddprof-lib/src/main/cpp/guards.cpp | 11 ++++++++--- ddprof-lib/src/main/cpp/guards.h | 4 ++-- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 7 +++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/ddprof-lib/src/main/cpp/guards.cpp b/ddprof-lib/src/main/cpp/guards.cpp index b3c0c6765d..c60b6320e8 100644 --- a/ddprof-lib/src/main/cpp/guards.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "os.h" #include "threadLocalData.inline.h" +#include "threadLocalDataPool.h" // Signal-context tracking — backed by ProfiledThread::_signal_depth; see // the comment block in guards.h for the rationale (initial-exec TLS was @@ -30,10 +31,14 @@ int getInSignalDepth() { bool isInTrackedSignalContext() { ProfiledThread *pt = ProfiledThread::current(); - // null ProfiledThread = no thread context; the SignalHandlerScope - // never ran, so we have no positive evidence of a signal frame. + // null ProfiledThread or primed ProfiledThread = no thread context; + // the SignalHandlerScope never ran, so we have no positive evidence + // of a signal frame. // See header comment for the rationale of returning false here. - return pt != nullptr && pt->signalDepth() != 0; + if (pt == nullptr || ThreadLocalDataPool::containsThread(pt)) { + return false; + } + return pt->signalDepth() != 0; } SignalHandlerScope::SignalHandlerScope() : _active(true) { diff --git a/ddprof-lib/src/main/cpp/guards.h b/ddprof-lib/src/main/cpp/guards.h index 18bc4fbeda..229ceba779 100644 --- a/ddprof-lib/src/main/cpp/guards.h +++ b/ddprof-lib/src/main/cpp/guards.h @@ -42,8 +42,8 @@ class ProfiledThread; // pthread_getspecific (POSIX guarantees it does not allocate; returns // nullptr when unset). // -// When ProfiledThread is null on a thread we don't yet have a thread -// context — uninstrumented JVM-internal threads (VM Thread, JIT, GC) fall +// When ProfiledThread is null or via thread priming on a thread +// — uninstrumented JVM-internal threads (VM Thread, JIT, GC) fall // into this bucket too, and they can receive signals. The // SignalHandlerScope guard is a no-op on those threads (nothing to // update), so isInTrackedSignalContext() returns false: production code diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 4fb4deb765..24ea581b02 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -46,6 +46,13 @@ class ThreadLocalDataPool { static void initialize(); static ProfiledThread* acquire(int tid); static bool release(ProfiledThread* t); + static inline bool containsThread(ProfiledThread* t) { + if (_pool != nullptr) { + return _pool->contains(t); + } else { + return false; + } + } #ifdef UNIT_TEST // Test-only: a pool isolated from the process-wide singleton (_pool), so From 4a20af7a37706943adbd2ce3642d95d17660712a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 19:42:27 +0000 Subject: [PATCH 21/30] Fix profiledThread and test --- ddprof-lib/src/main/cpp/mallocTracer.cpp | 3 ++ .../src/main/cpp/nativeSocketSampler.cpp | 3 ++ ddprof-lib/src/main/cpp/threadLocalDataPool.h | 5 ++-- .../profiler/cpu/TlsPrimingTest.java | 30 ++++++++++++++++--- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/mallocTracer.cpp b/ddprof-lib/src/main/cpp/mallocTracer.cpp index 18b843802e..7fe6c3a35f 100644 --- a/ddprof-lib/src/main/cpp/mallocTracer.cpp +++ b/ddprof-lib/src/main/cpp/mallocTracer.cpp @@ -337,6 +337,9 @@ void MallocTracer::recordMalloc(void* address, size_t size) { event._weight = (float)(1.0 / (1.0 - exp(-(double)size / (double)current_interval))); } + // We are not in a signal handler - take this chance to ensure ProfiledThread + // is attached to the thread cheaply. + ProfiledThread::initCurrentThreadSignalSafe(); Profiler::instance()->recordSample(NULL, size, OS::threadId(), BCI_NATIVE_MALLOC, 0, &event); u64 current_samples = __atomic_add_fetch(&_sample_count, 1, __ATOMIC_RELAXED); diff --git a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp index a8320f69d9..550608369a 100644 --- a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp +++ b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp @@ -264,6 +264,9 @@ void NativeSocketSampler::recordEvent(int fd, u64 t0, u64 t1, ssize_t bytes, u8 event._bytes = (u64)bytes; event._weight = weight; + // We are not in a signal handler - take this chance to ensure ProfiledThread + // is attached to the thread cheaply. + ProfiledThread::initCurrentThreadSignalSafe(); Profiler::instance()->recordSample(NULL, (u64)bytes, OS::threadId(), BCI_NATIVE_SOCKET, 0, &event); diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 24ea581b02..706d58c90a 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -47,8 +47,9 @@ class ThreadLocalDataPool { static ProfiledThread* acquire(int tid); static bool release(ProfiledThread* t); static inline bool containsThread(ProfiledThread* t) { - if (_pool != nullptr) { - return _pool->contains(t); + ThreadLocalDataPool* pool = __atomic_load_n(&_pool, __ATOMIC_ACQUIRE); + if (pool != nullptr) { + return pool->contains(t); } else { return false; } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java index 8ae20dc728..6345a25b4a 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java @@ -17,9 +17,11 @@ import org.openjdk.jmc.flightrecorder.jdk.JdkAttributes; import java.lang.reflect.Method; +import java.util.Map; import java.util.TreeSet; import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -34,10 +36,20 @@ * the first profiling signal reaches them. TLS priming is what covers this * gap: on that first signal, ProfiledThread::acquireCurrent() claims a slot * from the pool and attaches it via pthread_setspecific right there in the - * signal handler (see threadLocalData.cpp/ThreadLocalDataPool). If priming - * were broken, these early-started threads would simply never show up as - * eventThread on a sample. So seeing compiler-thread samples here is direct - * evidence that TLS priming worked. + * signal handler (see threadLocalData.cpp/ThreadLocalDataPool). + * + * Seeing a compiler-thread eventThread alone is not sufficient evidence: the + * CPU signal handlers resolve tid via OS::threadId() before recordSample() is + * even called, and native thread names are refreshed independently of TLS + * priming, so a sample tagged with a compiler thread's name would show up + * regardless of whether ProfiledThread::acquireCurrent() actually succeeded. + * If priming fails, recordSample() still emits an event for that tid, just + * with a synthetic "no_Java_frame" stack instead of a real unwind. The + * "samples_dropped_thread_local" debug counter is incremented exactly when + * acquireCurrent() fails (see StackWalker::walkFP/walkDwarf), so asserting it + * stayed at zero for the whole run is what actually proves every signal that + * reached a stack walker — including the ones on these never-registered + * compiler threads — found or attached a ProfiledThread. * * The test forces JIT compilation by loading a dynamically-generated class * with many distinct trivial methods and invoking each one past HotSpot's/ @@ -90,6 +102,16 @@ public void compilerThreadSamplesArePresent() throws Exception { assertTrue(sawCompilerThreadSample, "expected a datadog.ExecutionSample with eventThread starting with \"" + expectedPrefix + "\", but observed thread names: " + observedThreadNames); + + // A compiler-thread eventThread on its own doesn't prove a pool slot was + // ever attached (see class javadoc) — the tid and thread name are resolved + // independently of priming. Confirm no signal ever fell back to the + // "no_Java_frame" stack for lack of a ProfiledThread, on this or any other + // thread in the run. + Map debugCounters = profiler.getDebugCounters(); + assertEquals(0L, debugCounters.get("samples_dropped_thread_local"), + "TLS priming failed for at least one signal; compiler-thread samples " + + "may have used the no_Java_frame fallback instead of a real unwind"); } private void triggerJitCompilation() throws Exception { From 13c6c30d0571b6155b7c99421110708c4b6bfc4a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 20:36:34 +0000 Subject: [PATCH 22/30] Fix --- ddprof-lib/src/main/cpp/jvmSupport.cpp | 8 +++++--- ddprof-lib/src/main/cpp/mallocTracer.cpp | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index b1e4a728d0..5b18339115 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -40,14 +40,16 @@ bool JVMSupport::initialize() { return false; } - if (ProfiledThread::supportPriming()) { + // Check ProfiledThread key, it is critical for storing per-thread metadata + bool validKey = ProfiledThread::isThreadKeyValid(); + + if (validKey && ProfiledThread::supportPriming()) { ThreadLocalDataPool::initialize(); } else { LOG_WARN("Thread priming is not supported"); } - // Check ProfiledThread key, it is critical for storing per-thread metadata - return ProfiledThread::isThreadKeyValid(); + return validKey; } bool JVMSupport::isInitialized() { diff --git a/ddprof-lib/src/main/cpp/mallocTracer.cpp b/ddprof-lib/src/main/cpp/mallocTracer.cpp index 7fe6c3a35f..da7e14d09d 100644 --- a/ddprof-lib/src/main/cpp/mallocTracer.cpp +++ b/ddprof-lib/src/main/cpp/mallocTracer.cpp @@ -41,6 +41,10 @@ static void* (*_orig_aligned_alloc)(size_t, size_t); // because the window is short. static inline void maybeRecord(void* ret, size_t size) { if (MallocTracer::running() && ret && size) { + // We are not in a signal handler - take this chance to ensure ProfiledThread + // is attached to the thread cheaply. + ProfiledThread::initCurrentThreadSignalSafe(); + CriticalSection cs; if (cs.entered()) { MallocTracer::recordMalloc(ret, size); @@ -337,9 +341,6 @@ void MallocTracer::recordMalloc(void* address, size_t size) { event._weight = (float)(1.0 / (1.0 - exp(-(double)size / (double)current_interval))); } - // We are not in a signal handler - take this chance to ensure ProfiledThread - // is attached to the thread cheaply. - ProfiledThread::initCurrentThreadSignalSafe(); Profiler::instance()->recordSample(NULL, size, OS::threadId(), BCI_NATIVE_MALLOC, 0, &event); u64 current_samples = __atomic_add_fetch(&_sample_count, 1, __ATOMIC_RELAXED); From 55f63631ea8e481d18f16b75f590e8d38f07d5a3 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 5 Aug 2026 23:00:42 +0000 Subject: [PATCH 23/30] Fix --- ddprof-lib/src/main/cpp/mallocTracer.cpp | 7 +++---- ddprof-lib/src/main/cpp/unwindStats.cpp | 6 ++++++ ddprof-lib/src/main/cpp/unwindStats.h | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/ddprof-lib/src/main/cpp/mallocTracer.cpp b/ddprof-lib/src/main/cpp/mallocTracer.cpp index da7e14d09d..468b396e71 100644 --- a/ddprof-lib/src/main/cpp/mallocTracer.cpp +++ b/ddprof-lib/src/main/cpp/mallocTracer.cpp @@ -41,10 +41,9 @@ static void* (*_orig_aligned_alloc)(size_t, size_t); // because the window is short. static inline void maybeRecord(void* ret, size_t size) { if (MallocTracer::running() && ret && size) { - // We are not in a signal handler - take this chance to ensure ProfiledThread - // is attached to the thread cheaply. - ProfiledThread::initCurrentThreadSignalSafe(); - + // Even we are not in a signal handler, we cannot malloc or + // we may get into indefinite loop + ProfiledThread::acquireCurrent(); CriticalSection cs; if (cs.entered()) { MallocTracer::recordMalloc(ret, size); diff --git a/ddprof-lib/src/main/cpp/unwindStats.cpp b/ddprof-lib/src/main/cpp/unwindStats.cpp index 5852977794..5bd21a2f77 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.cpp +++ b/ddprof-lib/src/main/cpp/unwindStats.cpp @@ -1,3 +1,9 @@ +/* + * Copyright The async-profiler authors + * Copyright 2026 Datadog, Inc + * SPDX-License-Identifier: Apache-2.0 + */ + #include "unwindStats.h" // initialize static members diff --git a/ddprof-lib/src/main/cpp/unwindStats.h b/ddprof-lib/src/main/cpp/unwindStats.h index 5a9a75845b..bf2c49cce2 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.h +++ b/ddprof-lib/src/main/cpp/unwindStats.h @@ -1,3 +1,9 @@ +/* + * Copyright The async-profiler authors + * Copyright 2026 Datadog, Inc + * SPDX-License-Identifier: Apache-2.0 + */ + #ifndef STUB_UNWIND_STATS_H #define STUB_UNWIND_STATS_H From d0d231c4b59348d64cc45eff063f15a2bac56b0d Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 6 Aug 2026 20:05:20 +0000 Subject: [PATCH 24/30] Refactored --- AGENTS.md | 5 + ddprof-lib/src/main/cpp/ctimer_linux.cpp | 27 +-- ddprof-lib/src/main/cpp/guards.cpp | 75 ++----- ddprof-lib/src/main/cpp/guards.h | 30 +-- .../src/main/cpp/hotspot/hotspotSupport.cpp | 7 +- ddprof-lib/src/main/cpp/itimer.cpp | 27 ++- ddprof-lib/src/main/cpp/javaApi.cpp | 200 ++++++++++++------ ddprof-lib/src/main/cpp/mallocTracer.cpp | 6 +- ddprof-lib/src/main/cpp/perfEvents_linux.cpp | 18 +- ddprof-lib/src/main/cpp/profiler.cpp | 7 + ddprof-lib/src/main/cpp/stackWalker.cpp | 15 +- .../src/main/cpp/threadLocalData.inline.h | 24 ++- ddprof-lib/src/main/cpp/vmEntry.cpp | 13 +- ddprof-lib/src/main/cpp/wallClock.cpp | 28 ++- .../src/test/cpp/stress_callTraceStorage.cpp | 3 +- 15 files changed, 278 insertions(+), 207 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c69f02df49..201993abe4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -288,6 +288,11 @@ The profiler uses a sophisticated double-buffered storage system for call traces - **Atomic Operations**: Instance ID management and counter updates use atomics - **Memory Allocation**: Minimize malloc() in hot paths, use pre-allocated containers +### Sampler Safety +- **Stack walker**: `HotspotSupport::walkVM()`, `StackWalker::walkDwarf()`, and `StackWalker::walkFP()` must be protected by `sigsetjmp()`/`siglongjmp()`. +- **Samplers**: Every sampler must set up the `ProfiledThread` thread-local before sampling, and skip the sample if it isn't available. Signal-based samplers use `ProfiledThread::acquireCurrent()`; non-signal-based samplers use `ProfiledThread::initCurrentThreadSignalSafe()`. +- **JNI/JVMTI callbacks**: Use `ProfiledThread::initCurrentThreadSignalSafe()` to set up `ProfiledThread` for the thread. + ### Atomic Memory Ordering (Critical for arm64) arm64 has a weakly-ordered memory model (unlike x86 TSO). Incorrect ordering causes real lockups on arm64 that never reproduce on x86. - **Cross-thread reads**: Always use `__ATOMIC_ACQUIRE` for loads that must see stores from another thread. Never use `__ATOMIC_RELAXED` for cross-thread visibility unless you can prove no ordering dependency exists. diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index a6a3c12ddd..e3fd527f1e 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -226,19 +226,19 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { return; } int tid = 0; - ProfiledThread *current = ProfiledThread::current(); - assert(current == nullptr || !current->isDeepCrashHandler()); - if (current != nullptr && JVMThread::current() == nullptr + ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); + assert(current != nullptr); + + if (JVMThread::current() == nullptr && current->inInitWindow()) { current->tickInitWindow(); errno = saved_errno; return; } + if (current != NULL) { current->noteCPUSample(Profiler::instance()->recordingEpoch()); tid = current->tid(); - } else { - tid = OS::threadId(); } Shims::instance().setSighandlerTid(tid); @@ -268,6 +268,8 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { Counters::increment(CTIMER_SIGNAL_OWN); InflightGuard inflight; + ProfiledThread* current = SIGNAL_HANDLER_CURRENT_THREAD(); + assert(current != nullptr); // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs; @@ -281,25 +283,18 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { return; } - int tid = 0; - ProfiledThread *current = ProfiledThread::current(); - assert(current == nullptr || !current->isDeepCrashHandler()); + assert(!current->isDeepCrashHandler()); // Guard against the race window between Profiler::registerThread() and // thread_native_entry setting JVM TLS (PROF-13072): skip at most one signal // per thread. Pure native threads (where JVMThread::current() is always null) // are allowed through once the one-shot window expires. - if (current != nullptr && JVMThread::current() == nullptr - && current->inInitWindow()) { + if (JVMThread::current() == nullptr && current->inInitWindow()) { current->tickInitWindow(); errno = saved_errno; return; } - if (current != NULL) { - current->noteCPUSample(Profiler::instance()->recordingEpoch()); - tid = current->tid(); - } else { - tid = OS::threadId(); - } + current->noteCPUSample(Profiler::instance()->recordingEpoch()); + int tid = current->tid(); Shims::instance().setSighandlerTid(tid); ExecutionEvent event; diff --git a/ddprof-lib/src/main/cpp/guards.cpp b/ddprof-lib/src/main/cpp/guards.cpp index c60b6320e8..4d365c7495 100644 --- a/ddprof-lib/src/main/cpp/guards.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -18,7 +18,8 @@ #include "common.h" #include "os.h" #include "threadLocalData.inline.h" -#include "threadLocalDataPool.h" + +#include // Signal-context tracking — backed by ProfiledThread::_signal_depth; see // the comment block in guards.h for the rationale (initial-exec TLS was @@ -31,19 +32,17 @@ int getInSignalDepth() { bool isInTrackedSignalContext() { ProfiledThread *pt = ProfiledThread::current(); - // null ProfiledThread or primed ProfiledThread = no thread context; + // null ProfiledThread = no thread context; // the SignalHandlerScope never ran, so we have no positive evidence // of a signal frame. // See header comment for the rationale of returning false here. - if (pt == nullptr || ThreadLocalDataPool::containsThread(pt)) { - return false; - } - return pt->signalDepth() != 0; + return pt != nullptr && pt->signalDepth() != 0; } -SignalHandlerScope::SignalHandlerScope() : _active(true) { - ProfiledThread *pt = ProfiledThread::current(); +SignalHandlerScope::SignalHandlerScope() : _active(true), _current(nullptr) { + ProfiledThread *pt = ProfiledThread::acquireCurrent(); if (pt != nullptr) { + _current = pt; pt->enterSignalScope(); } else { // No thread context: nothing to update; mark inactive so destructor @@ -54,9 +53,8 @@ SignalHandlerScope::SignalHandlerScope() : _active(true) { SignalHandlerScope::~SignalHandlerScope() { if (!_active) return; - ProfiledThread *pt = ProfiledThread::current(); - if (pt != nullptr) { - pt->exitSignalScope(); + if (_current != nullptr) { + _current->exitSignalScope(); } } @@ -76,57 +74,20 @@ void signalHandlerUnwindAfterLongjmp() { } } -// Static bitmap storage for fallback cases -uint64_t CriticalSection::_fallback_bitmap[CriticalSection::FALLBACK_BITMAP_WORDS] = {}; -CriticalSection::CriticalSection() : _entered(false), _using_fallback(false), _word_index(0), _bit_mask(0), _thread_ptr(nullptr) { +CriticalSection::CriticalSection() : _entered(false), _thread_ptr(nullptr) { +#ifdef UNIT_TEST + _thread_ptr = ProfiledThread::initCurrentThreadSignalSafe(); +#else _thread_ptr = ProfiledThread::current(); - if (_thread_ptr != nullptr) { - // Primary path: Use ProfiledThread storage (fast and memory-efficient) - _entered = _thread_ptr->tryEnterCriticalSection(); - } else { - // Fallback path: Use hash-based bitmap for stress tests and edge cases - _using_fallback = true; - int tid = OS::threadId(); - - // Hash TID to distribute across bitmap words, reducing clustering - // We are OK with false collision for the fallback - it should be used only for testing when we don't have full profiler initialized - _word_index = hash_tid(tid) % FALLBACK_BITMAP_WORDS; - uint32_t bit_index = tid % 64; - _bit_mask = 1ULL << bit_index; - - // Use ACQUIRE ordering to ensure visibility of protected data after acquiring critical section - uint64_t old_word = __atomic_fetch_or(&_fallback_bitmap[_word_index], _bit_mask, __ATOMIC_ACQUIRE); - _entered = !(old_word & _bit_mask); // Success if bit was previously 0 - } +#endif + assert(_thread_ptr != nullptr); + _entered = _thread_ptr->tryEnterCriticalSection(); } CriticalSection::~CriticalSection() { + assert(_thread_ptr != nullptr); if (_entered) { - if (_using_fallback) { - // Clear the bit atomically for fallback bitmap - // Use RELEASE ordering to ensure protected data writes are visible before releasing - __atomic_fetch_and(&_fallback_bitmap[_word_index], ~_bit_mask, __ATOMIC_RELEASE); - - // ProfiledThread::current() was null when we entered via the fallback - // path, but TLS may have been published during this critical section's - // lifetime (ProfiledThread::acquireCurrent(), invoked from stack walking - // inside recordSample). acquireCurrent() marks the newly-published - // ProfiledThread's critical section held before exposing it; release - // that here too, otherwise it would stay stuck at true forever. - ProfiledThread* published = ProfiledThread::current(); - if (published != nullptr) { - published->exitCriticalSection(); - } - } else { - // Release ProfiledThread flag using the pointer captured at construction - if (_thread_ptr != nullptr) { - _thread_ptr->exitCriticalSection(); - } - } + _thread_ptr->exitCriticalSection(); } } - -uint32_t CriticalSection::hash_tid(int tid) { - return static_cast(tid * KNUTH_MULTIPLICATIVE_CONSTANT); -} diff --git a/ddprof-lib/src/main/cpp/guards.h b/ddprof-lib/src/main/cpp/guards.h index 229ceba779..8edf508031 100644 --- a/ddprof-lib/src/main/cpp/guards.h +++ b/ddprof-lib/src/main/cpp/guards.h @@ -22,6 +22,8 @@ #include #include +#include "counters.h" + class ProfiledThread; // --------------------------------------------------------------------------- @@ -80,14 +82,26 @@ class SignalHandlerScope { void release(); SignalHandlerScope(const SignalHandlerScope&) = delete; SignalHandlerScope& operator=(const SignalHandlerScope&) = delete; + + bool isActive() const { return _active; } + ProfiledThread* current() const { return _current; } private: + ProfiledThread* _current; bool _active; }; // Declare a scope guard local that increments the depth on entry and // decrements on scope exit. Use as the very first statement in every // installed signal handler. -#define SIGNAL_HANDLER_GUARD() SignalHandlerScope _signal_handler_scope +#define SIGNAL_HANDLER_GUARD() \ + SignalHandlerScope _signal_handler_scope; \ + if (!_signal_handler_scope.isActive()) { \ + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); \ + return; \ + } + +// Cheaper way to retrieve current ProfiledThread inside the scope +#define SIGNAL_HANDLER_CURRENT_THREAD() _signal_handler_scope.current() // Manually release the most recent SIGNAL_HANDLER_GUARD() before chaining to // another handler that may siglongjmp through us (e.g. J9's SIGSEGV null-pointer @@ -136,17 +150,7 @@ void signalHandlerUnwindAfterLongjmp(); */ class CriticalSection { private: - static constexpr size_t FALLBACK_BITMAP_WORDS = 1024; // 8KB for 64K bits - // Atomic bitmap for thread-safe critical section tracking without TLS - // Must be atomic because multiple signal handlers can run concurrently across - // different threads and attempt to set/clear bits simultaneously. Compare-and-swap - // operations ensure race-free bit manipulation even during signal interruption. - static uint64_t _fallback_bitmap[FALLBACK_BITMAP_WORDS]; - bool _entered; // Track if this instance successfully entered - bool _using_fallback; // Track which storage mechanism we're using - uint32_t _word_index; // For fallback bitmap cleanup - uint64_t _bit_mask; // For fallback bitmap cleanup ProfiledThread* _thread_ptr; // ProfiledThread captured at construction public: @@ -161,10 +165,6 @@ class CriticalSection { // Check if this instance successfully entered the critical section bool entered() const { return _entered; } - -private: - // Hash function to distribute thread IDs across bitmap words - static uint32_t hash_tid(int tid); }; /** diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index bb2c3233bd..1404425339 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -241,11 +241,8 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // VMStructs is only available for hotspot JVM assert(VM::isHotspot()); - ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); - if (prof_thread == nullptr) { - Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); - return 0; - } + ProfiledThread* prof_thread = ProfiledThread::current(); + assert(prof_thread != nullptr && "Should have been setup at signal handler entery"); HotspotStackFrame frame(ucontext); uintptr_t bottom = (uintptr_t)&frame + MAX_WALK_SIZE; diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp index 820ba341b4..eeff35c2e6 100644 --- a/ddprof-lib/src/main/cpp/itimer.cpp +++ b/ddprof-lib/src/main/cpp/itimer.cpp @@ -16,6 +16,7 @@ */ #include "itimer.h" +#include "counters.h" #include "debugSupport.h" #include "jvmThread.h" #include "os.h" @@ -42,20 +43,16 @@ void ITimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { InflightGuard inflight; if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) return; - + + ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); + // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs; if (!cs.entered()) { return; // Another critical section is active, defer profiling } - int tid = 0; - ProfiledThread *current = ProfiledThread::current(); - if (current != NULL) { - current->noteCPUSample(Profiler::instance()->recordingEpoch()); - tid = current->tid(); - } else { - tid = OS::threadId(); - } + current->noteCPUSample(Profiler::instance()->recordingEpoch()); + int tid = current->tid(); Shims::instance().setSighandlerTid(tid); ExecutionEvent event; @@ -106,6 +103,9 @@ long ITimerJvmti::_interval = 0; void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { SIGNAL_HANDLER_GUARD(); + ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); + assert(current != nullptr); + InflightGuard inflight; CriticalSection cs; if (!cs.entered()) { @@ -116,17 +116,14 @@ void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { errno = saved_errno; return; } - ProfiledThread *current = ProfiledThread::current(); - if (current != nullptr && JVMThread::current() == nullptr + if (JVMThread::current() == nullptr && current->inInitWindow()) { current->tickInitWindow(); errno = saved_errno; return; } - int tid = current ? current->tid() : OS::threadId(); - if (current) { - current->noteCPUSample(Profiler::instance()->recordingEpoch()); - } + int tid = current->tid(); + current->noteCPUSample(Profiler::instance()->recordingEpoch()); Shims::instance().setSighandlerTid(tid); ExecutionEvent event; diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 380b3058c7..6654b84b7a 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -75,8 +75,17 @@ Java_com_datadoghq_profiler_JavaProfiler_init0(JNIEnv *env, jclass unused) { return JNI_FALSE; } + // JavaVM* has already been stored when the native library was loaded so we can pass nullptr here - return VM::initProfilerBridge(nullptr, true); + if (VM::initProfilerBridge(nullptr, true)) { + // Attach ProfiledThread + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + + return JNI_TRUE; + } else { + return JNI_FALSE; + } } extern "C" DLLEXPORT void JNICALL @@ -90,6 +99,10 @@ Java_com_datadoghq_profiler_JavaProfiler_stop0(JNIEnv *env, jobject unused) { extern "C" DLLEXPORT jint JNICALL Java_com_datadoghq_profiler_JavaProfiler_getTid0(JNIEnv *env, jclass unused) { + // Attach ProfiledThread + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + return OS::threadId(); } @@ -108,6 +121,11 @@ Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused, Log::open(args); std::ostringstream out; + + // Attach ProfiledThread + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + error = Profiler::instance()->runInternal(args, out); if (!error) { if (out.tellp() >= 0x3fffffff) { @@ -126,6 +144,10 @@ extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env, jclass unused) { char msg[2048]; + // Attach ProfiledThread + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + Profiler::instance()->status((char*)msg, sizeof(msg) - 1); return env->NewStringUTF(msg); } @@ -133,6 +155,10 @@ Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env, extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env, jclass unused) { + // Attach ProfiledThread + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + return (jlong)Profiler::instance()->total_samples(); } @@ -146,9 +172,7 @@ extern "C" DLLEXPORT void JNICALL JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0() { // Initialize thread TLS if it has not yet done ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if(current == nullptr) { - return; - } + assert(current != nullptr && "Out of order initialization"); int tid = current->tid(); if (unlikely(tid < 0)) { @@ -180,10 +204,7 @@ extern "C" DLLEXPORT void JNICALL JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0() { // Initialize thread TLS if it has not yet done ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if(current == nullptr) { - return; - } - + assert(current != nullptr && "Out of order initialization"); int tid = current->tid(); if (unlikely(tid < 0)) { return; @@ -221,7 +242,8 @@ Java_com_datadoghq_profiler_JavaProfiler_recordTrace0( JniString endpoint_str(env, endpoint); // Initialize thread TLS if it has not yet done - ProfiledThread::initCurrentThreadSignalSafe(); + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); u32 endpointLabel = Profiler::instance()->stringLabelMap()->bounded_lookup( endpoint_str.c_str(), endpoint_str.length(), sizeLimit); @@ -244,6 +266,10 @@ Java_com_datadoghq_profiler_JavaProfiler_recordTrace0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_dump0(JNIEnv *env, jclass unused, jstring path) { + // Initialize thread TLS if it has not yet done + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString path_str(env, path); Profiler::instance()->dump(path_str.c_str(), path_str.length()); } @@ -263,6 +289,10 @@ extern "C" DLLEXPORT jobjectArray JNICALL Java_com_datadoghq_profiler_JavaProfiler_describeDebugCounters0( JNIEnv *env, jclass unused) { #ifdef COUNTERS + // Initialize thread TLS if it has not yet done + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + std::vector counter_names = Counters::describeCounters(); jobjectArray array = (jobjectArray)env->NewObjectArray( counter_names.size(), env->FindClass("java/lang/String"), @@ -281,7 +311,8 @@ extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_recordSettingEvent0( JNIEnv *env, jclass unused, jstring name, jstring value, jstring unit) { // Initialize thread TLS if it has not yet done - ProfiledThread::initCurrentThreadSignalSafe(); + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); int tid = ProfiledThread::currentTid(); if (tid < 0) { @@ -310,6 +341,8 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0( // Initialize thread TLS if it has not yet done ProfiledThread::initCurrentThreadSignalSafe(); + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); int tid = ProfiledThread::currentTid(); if (tid < 0) { @@ -346,9 +379,8 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(JNIEnv *env, jclass unused) { ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) { - return; - } + assert(current != nullptr && "Out of order initialization"); + bool first_park = current->parkEnter(); ThreadFilter *tf = Profiler::instance()->threadFilter(); if (first_park && tf->enabled()) { @@ -364,9 +396,7 @@ extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkExit0( JNIEnv *env, jclass unused, jlong blocker, jlong unblockingSpanId) { ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) { - return; - } + assert(current != nullptr && "Out of order initialization"); u64 park_block_token = 0; if (!current->parkExit(park_block_token) || park_block_token == 0) { @@ -393,14 +423,13 @@ static bool decodeJavaBlockState(jint state, OSThreadState &decoded) { extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( JNIEnv *env, jclass unused, jint state) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + OSThreadState decoded; if (!decodeJavaBlockState(state, decoded)) { return 0; } - ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) { - return 0; - } ThreadFilter *tf = Profiler::instance()->threadFilter(); if (!tf->enabled()) { return 0; @@ -415,14 +444,13 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0( JNIEnv *env, jclass unused, jlong token) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + u64 block_token = static_cast(token); if (block_token == 0) { return; } - ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) { - return; - } ThreadFilter::SlotID slot_id = ThreadFilter::tokenSlotId(block_token); if (current->filterSlotId() != slot_id) { return; @@ -436,12 +464,17 @@ Java_com_datadoghq_profiler_JavaProfiler_blockExit0( extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_currentTicks0(JNIEnv *env, jclass unused) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + return TSC::ticks(); } extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_tscFrequency0(JNIEnv *env, jclass unused) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); return TSC::frequency(); } @@ -449,6 +482,8 @@ extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_mallocArenaMax0(JNIEnv *env, jclass unused, jint maxArenas) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); OS::mallocArenaMax(maxArenas); } @@ -456,6 +491,9 @@ extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JVMAccess_findStringJVMFlag0(JNIEnv *env, jobject unused, jstring flagName) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::String, VMFlag::Type::Stringlist}); if (f) { @@ -472,6 +510,9 @@ Java_com_datadoghq_profiler_JVMAccess_setStringJVMFlag0(JNIEnv *env, jobject unused, jstring flagName, jstring flagValue) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); JniString value_str(env, flagValue); VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::String, VMFlag::Type::Stringlist}); @@ -487,6 +528,9 @@ extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JVMAccess_findBooleanJVMFlag0(JNIEnv *env, jobject unused, jstring flagName) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Bool}); if (f) { @@ -503,6 +547,9 @@ Java_com_datadoghq_profiler_JVMAccess_setBooleanJVMFlag0(JNIEnv *env, jobject unused, jstring flagName, jboolean flagValue) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Bool}); if (f) { @@ -517,6 +564,9 @@ extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JVMAccess_findIntJVMFlag0(JNIEnv *env, jobject unused, jstring flagName) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Int, VMFlag::Type::Uint, VMFlag::Type::Intx, VMFlag::Type::Uintx, VMFlag::Type::Uint64_t, VMFlag::Type::Size_t}); if (f) { @@ -532,6 +582,9 @@ extern "C" DLLEXPORT jdouble JNICALL Java_com_datadoghq_profiler_JVMAccess_findFloatJVMFlag0(JNIEnv *env, jobject unused, jstring flagName) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString flag_str(env, flagName); VMFlag *f = VMFlag::find(flag_str.c_str(),{ VMFlag::Type::Double}); if (f) { @@ -546,6 +599,9 @@ Java_com_datadoghq_profiler_JVMAccess_findFloatJVMFlag0(JNIEnv *env, extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JVMAccess_healthCheck0(JNIEnv *env, jobject unused) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + return true; } @@ -560,6 +616,9 @@ Java_com_datadoghq_profiler_OTelContext_setProcessCtx0(JNIEnv *env, jstring tracer_version, jobjectArray attribute_keys ) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + JniString env_str(env, env_data); JniString hostname_str(env, hostname); JniString runtime_id_str(env, runtime_id); @@ -634,6 +693,9 @@ Java_com_datadoghq_profiler_OTelContext_setProcessCtx0(JNIEnv *env, extern "C" DLLEXPORT jobject JNICALL Java_com_datadoghq_profiler_OTelContext_readProcessCtx0(JNIEnv *env, jclass unused) { #ifndef OTEL_PROCESS_CTX_NO_READ + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); + otel_process_ctx_read_result result = otel_process_ctx_read(); if (!result.success) { @@ -829,10 +891,9 @@ extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_setTraceContext0(JNIEnv* env, jclass unused, jlong localRootSpanId, jlong spanId, jlong traceIdHigh, jlong traceIdLow, jint slot0, jint enc0, jbyteArray utf0, jint slot1, jint enc1, jbyteArray utf1) { - ProfiledThread* thrd = ProfiledThread::initCurrentThreadSignalSafe(); - if (thrd == nullptr) { - return; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + // Contract: this is the activation path and requires a non-zero span; clearing is // clearTraceContext0. The public setTraceContext wrapper enforces this by throwing // IllegalArgumentException, so a zero span reaching here is a direct-JNI/contract violation. @@ -885,10 +946,9 @@ Java_com_datadoghq_profiler_JavaProfiler_setTraceContext0(JNIEnv* env, jclass un // detached (valid=0). extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_clearTraceContext0(JNIEnv* env, jclass unused) { - ProfiledThread* thrd = ProfiledThread::initCurrentThreadSignalSafe(); - if (thrd == nullptr) { - return; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + OtelThreadContextRecord* record = thrd->getOtelContextRecord(); u32* enc = thrd->getOtelTagEncodingsPtr(); u64* lrs = reinterpret_cast(enc + DD_TAGS_CAPACITY); @@ -910,8 +970,10 @@ Java_com_datadoghq_profiler_JavaProfiler_clearTraceContext0(JNIEnv* env, jclass extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_setContextValue0(JNIEnv* env, jclass unused, jint slot, jint encoding, jbyteArray utf8) { - ProfiledThread* thrd = ProfiledThread::initCurrentThreadSignalSafe(); - if (thrd == nullptr || slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + + if (slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { return JNI_FALSE; } // See setTraceContext0: publish the OTEP TLS pointer on first native write so the sampler and @@ -949,8 +1011,10 @@ Java_com_datadoghq_profiler_JavaProfiler_setContextValue0(JNIEnv* env, jclass un // Clears a single attribute slot (zeros the sidecar encoding, compacts it out of attrs_data). extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_clearContextValue0(JNIEnv* env, jclass unused, jint slot) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr || slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + + if (slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { return; } OtelThreadContextRecord* record = thrd->getOtelContextRecord(); @@ -978,19 +1042,21 @@ Java_com_datadoghq_profiler_JavaProfiler_copyContextTags0(JNIEnv* env, jclass un if (out == nullptr) { return; } + + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + jint len = env->GetArrayLength(out); int n = len < (jint)DD_TAGS_CAPACITY ? (int)len : (int)DD_TAGS_CAPACITY; jint tmp[DD_TAGS_CAPACITY]; for (int i = 0; i < n; i++) { tmp[i] = 0; } - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd != nullptr) { - u32* enc = thrd->getOtelTagEncodingsPtr(); - for (int i = 0; i < n; i++) { - tmp[i] = (jint)enc[i]; - } + u32* enc = thrd->getOtelTagEncodingsPtr(); + for (int i = 0; i < n; i++) { + tmp[i] = (jint)enc[i]; } + if (n > 0) { env->SetIntArrayRegion(out, 0, n, tmp); } @@ -998,6 +1064,9 @@ Java_com_datadoghq_profiler_JavaProfiler_copyContextTags0(JNIEnv* env, jclass un extern "C" DLLEXPORT jint JNICALL Java_com_datadoghq_profiler_ContextValueCache_registerConstant0(JNIEnv* env, jclass unused, jstring value) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + JniString value_str(env, value); u32 encoding = Profiler::instance()->contextValueMap()->bounded_lookup( value_str.c_str(), value_str.length(), 1 << 16); @@ -1008,6 +1077,9 @@ Java_com_datadoghq_profiler_ContextValueCache_registerConstant0(JNIEnv* env, jcl // MAX_CONTEXT_SLOTS constant has not drifted from DD_TAGS_CAPACITY (see MaxContextSlotsTest). extern "C" DLLEXPORT jint JNICALL Java_com_datadoghq_profiler_JavaProfiler_maxContextSlots0(JNIEnv* env, jclass unused) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + return (jint)DD_TAGS_CAPACITY; } @@ -1018,12 +1090,18 @@ Java_com_datadoghq_profiler_JavaProfiler_maxContextSlots0(JNIEnv* env, jclass un // re-parsing in Java. extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_consumeContextDictionaryReset0(JNIEnv* env, jclass unused) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + return Profiler::instance()->consumeContextValueDictReset() ? JNI_TRUE : JNI_FALSE; } // ---- test and debug utilities extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_testlog(JNIEnv* env, jclass unused, jstring msg) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + JniString msg_str(env, msg); TEST_LOG("%s", msg_str.c_str()); @@ -1031,6 +1109,9 @@ Java_com_datadoghq_profiler_JavaProfiler_testlog(JNIEnv* env, jclass unused, jst extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_dumpContext(JNIEnv* env, jclass unused) { + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + u64 spanId = 0, rootSpanId = 0; ContextApi::get(spanId, rootSpanId); TEST_LOG("===> Context: tid:%lu, spanId=%lu, rootSpanId=%lu", OS::threadId(), spanId, rootSpanId); @@ -1042,10 +1123,9 @@ Java_com_datadoghq_profiler_JavaProfiler_dumpContext(JNIEnv* env, jclass unused) extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_testGetSpanId0(JNIEnv* env, jclass unused) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr) { - return 0; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + OtelThreadContextRecord* record = thrd->getOtelContextRecord(); uint64_t beSpan; memcpy(&beSpan, record->span_id, 8); @@ -1054,10 +1134,9 @@ Java_com_datadoghq_profiler_JavaProfiler_testGetSpanId0(JNIEnv* env, jclass unus extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_testGetRootSpanId0(JNIEnv* env, jclass unused) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr) { - return 0; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + u32* enc = thrd->getOtelTagEncodingsPtr(); u64* lrs = reinterpret_cast(enc + DD_TAGS_CAPACITY); return (jlong)*lrs; @@ -1065,10 +1144,9 @@ Java_com_datadoghq_profiler_JavaProfiler_testGetRootSpanId0(JNIEnv* env, jclass extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JavaProfiler_testReadTraceId0(JNIEnv* env, jclass unused) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr) { - return nullptr; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + OtelThreadContextRecord* record = thrd->getOtelContextRecord(); static const char HEXD[16] = {'0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f'}; @@ -1084,10 +1162,9 @@ Java_com_datadoghq_profiler_JavaProfiler_testReadTraceId0(JNIEnv* env, jclass un extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JavaProfiler_testReadContextAttribute0(JNIEnv* env, jclass unused, jint slot) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr || slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { - return nullptr; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + OtelThreadContextRecord* record = thrd->getOtelContextRecord(); int targetKey = slot + 1; int size = record->attrs_data_size; @@ -1112,10 +1189,9 @@ Java_com_datadoghq_profiler_JavaProfiler_testReadContextAttribute0(JNIEnv* env, extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_testIsContextValid0(JNIEnv* env, jclass unused) { - ProfiledThread* thrd = ProfiledThread::current(); - if (thrd == nullptr) { - return JNI_FALSE; - } + ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thrd != nullptr && "Out of order initialization"); + OtelThreadContextRecord* record = thrd->getOtelContextRecord(); return __atomic_load_n(&record->valid, __ATOMIC_ACQUIRE) ? JNI_TRUE : JNI_FALSE; } diff --git a/ddprof-lib/src/main/cpp/mallocTracer.cpp b/ddprof-lib/src/main/cpp/mallocTracer.cpp index 468b396e71..1b87060f52 100644 --- a/ddprof-lib/src/main/cpp/mallocTracer.cpp +++ b/ddprof-lib/src/main/cpp/mallocTracer.cpp @@ -10,6 +10,7 @@ #include #include #include "codeCache.h" +#include "counters.h" #include "guards.h" #include "libraries.h" #include "mallocTracer.h" @@ -43,7 +44,10 @@ static inline void maybeRecord(void* ret, size_t size) { if (MallocTracer::running() && ret && size) { // Even we are not in a signal handler, we cannot malloc or // we may get into indefinite loop - ProfiledThread::acquireCurrent(); + if (ProfiledThread::acquireCurrent() == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return; + } CriticalSection cs; if (cs.entered()) { MallocTracer::recordMalloc(ret, size); diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp index a111a5a14f..61f964d0fd 100644 --- a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp +++ b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp @@ -20,6 +20,7 @@ #include "arch.h" #include "arguments.h" #include "context.h" +#include "counters.h" #include "guards.h" #include "debugSupport.h" #include "jvmSupport.inline.h" @@ -742,16 +743,23 @@ void PerfEvents::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { return; } InflightGuard inflight; + + // A thread with no ProfiledThread attached must never enter the critical + // section below. acquireCurrent() is the only thing that can attach one; it + // must fully succeed or fail before we try to claim exclusivity, not while + // we're holding it -- otherwise a signal that interrupts us right after + // publish could observe a ProfiledThread whose critical-section state + // doesn't yet reflect reality. Drop the sample instead. + ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); + assert(current != nullptr); + // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs; if (!cs.entered()) { return; // Another critical section is active, defer profiling } - ProfiledThread *current = ProfiledThread::current(); - if (current != NULL) { - current->noteCPUSample(Profiler::instance()->recordingEpoch()); - } - int tid = current != NULL ? current->tid() : OS::threadId(); + current->noteCPUSample(Profiler::instance()->recordingEpoch()); + int tid = current->tid(); if (__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { Shims::instance().setSighandlerTid(tid); diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 6f87abe7a6..5cdf13e105 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -531,6 +531,13 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, } u64 Profiler::recordJVMTISample(u64 counter, int tid, jthread thread, jint event_type, Event *event, bool deferred) { + // Called from none signal based sampler + ProfiledThread* prof_thread = ProfiledThread::initCurrentThreadSignalSafe(); + if (prof_thread == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return 0; + } + // Protect JVMTI sampling operations to prevent signal handler interference CriticalSection cs; atomicIncRelaxed(_total_samples); diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index 9abcf2c61b..b7a30abe10 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -45,11 +45,9 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); - if (prof_thread == nullptr) { - Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); - return 0; - } + ProfiledThread* prof_thread = ProfiledThread::current(); + assert(prof_thread != nullptr && "Should have been setup at signal handler entery"); + sigjmp_buf crash_protection_ctx; sigjmp_buf* prev_jmp_buf = prof_thread->getJmpCtx(); @@ -134,11 +132,8 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth // falls inside this library while a jmp ctx is installed gets caught by // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, // instead of crashing the process. - ProfiledThread* prof_thread = ProfiledThread::acquireCurrent(); - if (prof_thread == nullptr) { - Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); - return 0; - } + ProfiledThread* prof_thread = ProfiledThread::current(); + assert(prof_thread != nullptr && "Should have been setup at signal handler entery"); sigjmp_buf crash_protection_ctx; sigjmp_buf* prev_jmp_buf = prof_thread->getJmpCtx(); diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index ae7a69f533..3513176180 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -22,16 +22,20 @@ ProfiledThread* ProfiledThread::acquireCurrent() { ProfiledThread* prof_thread = current(); if (prof_thread == nullptr) { SignalBlocker blocker; - prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); - if (prof_thread != nullptr) { - // Claim the critical section before publishing the pointer. A signal - // that interrupts us on this thread right after publish (cross-type - // nesting isn't blocked — see os_linux.cpp's empty sa_mask) would - // otherwise see a non-null ProfiledThread whose _in_critical_section - // is still false and race into the primary path. CriticalSection's - // fallback-path destructor releases this once the outer handler exits. - prof_thread->tryEnterCriticalSection(); - _current_thread.set(prof_thread); + // Check again, in case the call is interrupted by another signal + prof_thread = current(); + if (prof_thread == nullptr) { + prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); + if (prof_thread != nullptr) { + // Claim the critical section before publishing the pointer. A signal + // that interrupts us on this thread right after publish (cross-type + // nesting isn't blocked — see os_linux.cpp's empty sa_mask) would + // otherwise see a non-null ProfiledThread whose _in_critical_section + // is still false and race into the primary path. CriticalSection's + // fallback-path destructor releases this once the outer handler exits. + prof_thread->tryEnterCriticalSection(); + _current_thread.set(prof_thread); + } } } return prof_thread; diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index f6b6946ad7..38534cba16 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -626,21 +626,26 @@ void *VM::getLibraryHandle(const char *name) { void JNICALL VM::ClassPrepare(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread, jclass klass) { - ProfiledThread::initCurrentThreadSignalSafe(); + ProfiledThread* thr = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thr != nullptr); JVMSupport::loadMethodIDsIfNeeded(jvmti, jni, klass); } void JNICALL VM::ClassLoad(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jclass klass) { // Needed only for AsyncGetCallTrace support - ProfiledThread::initCurrentThreadSignalSafe(); + ProfiledThread* thr = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thr != nullptr); } void JNICALL VM::VMInit(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread) { ready(jvmti, jni); - // initialize the heap usage tracking only after the VM is ready + ProfiledThread* thr = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thr != nullptr); + + // initialize the heap usage tracking only after the VM is ready HeapUsage::initJMXUsage(VM::jni()); // Delayed start of profiler if agent has been loaded at VM bootstrap @@ -655,6 +660,8 @@ Arguments& VM::arguments() { } void JNICALL VM::VMDeath(jvmtiEnv *jvmti, JNIEnv *jni) { + ProfiledThread* thr = ProfiledThread::initCurrentThreadSignalSafe(); + assert(thr != nullptr); Profiler::instance()->shutdown(_agent_args); } diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp index 798da6c3d3..a41029ebab 100644 --- a/ddprof-lib/src/main/cpp/wallClock.cpp +++ b/ddprof-lib/src/main/cpp/wallClock.cpp @@ -21,6 +21,7 @@ #include "threadState.inline.h" #include "guards.h" #include "wallClockCounters.h" +#include #include #include #include @@ -231,18 +232,28 @@ void WallClockASGCT::sharedSignalHandler(int signo, siginfo_t *siginfo, void WallClockASGCT::signalHandler(int signo, siginfo_t *siginfo, void *ucontext, u64 last_sample) { + // A thread with no ProfiledThread attached must never enter the critical + // section below. acquireCurrent() is the only thing that can attach one; it + // must fully succeed or fail before we try to claim exclusivity, not while + // we're holding it -- otherwise a signal that interrupts us right after + // publish could observe a ProfiledThread whose critical-section state + // doesn't yet reflect reality. Drop the sample instead. + ProfiledThread *current = ProfiledThread::acquireCurrent(); + if (current == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return; + } + // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs; if (!cs.entered()) { return; // Another critical section is active, defer profiling } - ProfiledThread *current = ProfiledThread::current(); // Guard against the race window between Profiler::registerThread() and // thread_native_entry setting JVM TLS (PROF-13072): skip at most one signal // per thread. Pure native threads (where JVMThread::current() is always null) // are allowed through once the one-shot window expires. - if (current != nullptr && JVMThread::current() == nullptr - && current->inInitWindow()) { + if (JVMThread::current() == nullptr && current->inInitWindow()) { current->tickInitWindow(); return; } @@ -256,10 +267,10 @@ void WallClockASGCT::signalHandler(int signo, siginfo_t *siginfo, void *ucontext if (precheck.suppress) { return; } - int tid = current != NULL ? current->tid() : OS::threadId(); + int tid = current->tid(); Shims::instance().setSighandlerTid(tid); u64 call_trace_id = 0; - if (current != NULL && _collapsing) { + if (_collapsing) { StackFrame frame(ucontext); u64 spanId = 0, rootSpanId = 0; // contextValid is not redundant with (spanId==0 && rootSpanId==0): a cleared @@ -447,18 +458,21 @@ void WallClockJvmti::signalHandler(int signo, siginfo_t *siginfo, } int saved_errno = errno; ProfiledThread *current = ProfiledThread::current(); - if (current != nullptr && JVMThread::current() == nullptr + assert(current != nullptr && "Should have been setup at signal handler entery"); + + if (JVMThread::current() == nullptr && current->inInitWindow()) { current->tickInitWindow(); errno = saved_errno; return; } + WallPrecheckResult precheck = prepareWallPrecheck(current, _precheck); if (precheck.suppress) { errno = saved_errno; return; } - int tid = current != NULL ? current->tid() : OS::threadId(); + int tid = current->tid(); Shims::instance().setSighandlerTid(tid); ExecutionEvent event; diff --git a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp index a8706967d9..4cd5690a69 100644 --- a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp @@ -8,6 +8,7 @@ #include "callTraceHashTable.h" #include "guards.h" #include "common.h" // TSAN_ENABLED (toolchain-agnostic sanitizer detection) +#include "threadLocalData.h" #include #include #include @@ -30,7 +31,7 @@ #include #include #include -#include "../../main/cpp/gtest_crash_handler.h" +#include "gtest_crash_handler.h" // Test name for crash handler static constexpr const char STRESS_TEST_NAME[] = "StressCallTraceStorage"; From bd5bd61c4716bbce25b39e2f484f991d9d562da1 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 02:00:05 +0000 Subject: [PATCH 25/30] Fix --- ddprof-lib/src/main/cpp/guards.cpp | 4 - ddprof-lib/src/main/cpp/guards.h | 8 +- ddprof-lib/src/main/cpp/javaApi.cpp | 1 - .../src/main/cpp/nativeSocketSampler.cpp | 9 +- ddprof-lib/src/main/cpp/profiler.cpp | 4 +- ddprof-lib/src/main/cpp/threadLocalData.cpp | 2 + .../src/main/cpp/threadLocalData.inline.h | 7 -- ddprof-lib/src/main/cpp/unwindStats.cpp | 19 +++- ddprof-lib/src/test/cpp/nativeMem_ut.cpp | 27 ++++- .../src/test/cpp/stress_callTraceStorage.cpp | 45 +++++++- .../test/cpp/stress_threadLifecycle_ut.cpp | 2 + .../src/test/cpp/test_callTraceStorage.cpp | 26 +++++ .../profiler/cpu/TlsPrimingTest.java | 100 ++++++++++-------- 13 files changed, 186 insertions(+), 68 deletions(-) diff --git a/ddprof-lib/src/main/cpp/guards.cpp b/ddprof-lib/src/main/cpp/guards.cpp index 4d365c7495..3638076943 100644 --- a/ddprof-lib/src/main/cpp/guards.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -76,11 +76,7 @@ void signalHandlerUnwindAfterLongjmp() { CriticalSection::CriticalSection() : _entered(false), _thread_ptr(nullptr) { -#ifdef UNIT_TEST - _thread_ptr = ProfiledThread::initCurrentThreadSignalSafe(); -#else _thread_ptr = ProfiledThread::current(); -#endif assert(_thread_ptr != nullptr); _entered = _thread_ptr->tryEnterCriticalSection(); } diff --git a/ddprof-lib/src/main/cpp/guards.h b/ddprof-lib/src/main/cpp/guards.h index 8edf508031..95cc5faa5f 100644 --- a/ddprof-lib/src/main/cpp/guards.h +++ b/ddprof-lib/src/main/cpp/guards.h @@ -92,7 +92,7 @@ class SignalHandlerScope { // Declare a scope guard local that increments the depth on entry and // decrements on scope exit. Use as the very first statement in every -// installed signal handler. +// installed sampler signal handler #define SIGNAL_HANDLER_GUARD() \ SignalHandlerScope _signal_handler_scope; \ if (!_signal_handler_scope.isActive()) { \ @@ -100,6 +100,12 @@ class SignalHandlerScope { return; \ } +// Declare a scope guard local that increments the depth on entry and +// decrements on scope exit. Use as the very first statement in every +// installed non-sampler signal handler +#define SIGNAL_HANDLER_GUARD_NO_SAMPLE() \ + SignalHandlerScope _signal_handler_scope; + // Cheaper way to retrieve current ProfiledThread inside the scope #define SIGNAL_HANDLER_CURRENT_THREAD() _signal_handler_scope.current() diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 6654b84b7a..c02de5d0c1 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -340,7 +340,6 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0( jstring scheduler, jthread origin, jstring queueType, jint queueLength) { // Initialize thread TLS if it has not yet done - ProfiledThread::initCurrentThreadSignalSafe(); ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); assert(current != nullptr && "Out of order initialization"); diff --git a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp index 550608369a..c3154c7d0d 100644 --- a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp +++ b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp @@ -266,9 +266,12 @@ void NativeSocketSampler::recordEvent(int fd, u64 t0, u64 t1, ssize_t bytes, u8 // We are not in a signal handler - take this chance to ensure ProfiledThread // is attached to the thread cheaply. - ProfiledThread::initCurrentThreadSignalSafe(); - Profiler::instance()->recordSample(NULL, (u64)bytes, OS::threadId(), - BCI_NATIVE_SOCKET, 0, &event); + if (ProfiledThread::initCurrentThreadSignalSafe() == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + } else { + Profiler::instance()->recordSample(NULL, (u64)bytes, OS::threadId(), + BCI_NATIVE_SOCKET, 0, &event); + } _rate_limiter.recordFire(); } diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 81dc6fc6bd..25d0993d0d 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -977,7 +977,7 @@ void Profiler::segvHandler(int signo, siginfo_t *siginfo, void *ucontext) { // the lesser of two evils — leaking depth on siglongjmp would silently // break the production deferred-refresh gate, while the sanitizer gap // is bounded to third-party signal handler code we don't own. - SIGNAL_HANDLER_GUARD(); + SIGNAL_HANDLER_GUARD_NO_SAMPLE(); if (crashHandlerInternal(signo, siginfo, ucontext)) { return; // Handled — destructor decrements depth } @@ -994,7 +994,7 @@ void Profiler::segvHandler(int signo, siginfo_t *siginfo, void *ucontext) { void Profiler::busHandler(int signo, siginfo_t *siginfo, void *ucontext) { // See segvHandler: release before chaining in case the chained handler // siglongjmps through us. - SIGNAL_HANDLER_GUARD(); + SIGNAL_HANDLER_GUARD_NO_SAMPLE(); if (crashHandlerInternal(signo, siginfo, ucontext)) { return; // Handled — destructor decrements depth } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 3229756932..fe57260e90 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -116,12 +116,14 @@ Context ProfiledThread::snapshotContext(size_t numAttrs) { } void ProfiledThread::resetClaimed(int tid) { + _unwinding_Java = false; _jmp_buf = nullptr; _pc = 0; _sp = 0; _span_id = 0; _crash_depth = 0; _tid = tid; + _cpu_epoch = 0; _wall_epoch = 0; _call_trace_id = 0; _recording_epoch = 0; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index 3513176180..c10ed2c41f 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -27,13 +27,6 @@ ProfiledThread* ProfiledThread::acquireCurrent() { if (prof_thread == nullptr) { prof_thread = ThreadLocalDataPool::acquire(OS::threadId()); if (prof_thread != nullptr) { - // Claim the critical section before publishing the pointer. A signal - // that interrupts us on this thread right after publish (cross-type - // nesting isn't blocked — see os_linux.cpp's empty sa_mask) would - // otherwise see a non-null ProfiledThread whose _in_critical_section - // is still false and race into the primary path. CriticalSection's - // fallback-path destructor releases this once the outer handler exits. - prof_thread->tryEnterCriticalSection(); _current_thread.set(prof_thread); } } diff --git a/ddprof-lib/src/main/cpp/unwindStats.cpp b/ddprof-lib/src/main/cpp/unwindStats.cpp index 5bd21a2f77..11a02dfdb5 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.cpp +++ b/ddprof-lib/src/main/cpp/unwindStats.cpp @@ -5,20 +5,35 @@ */ #include "unwindStats.h" +#include "nativeMem.h" // initialize static members SpinLock UnwindStats::_lock; UnwindFailures UnwindStats::_unwind_failures; +// Heap footprint of the two arrays each instance owns. The ctor records it +// under NM_THREAD_LOCAL and the dtor gives it back, so it lives in one constant +// to keep the two sides from drifting. +static constexpr size_t UNWIND_FAILURES_ARRAYS_SIZE = + MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH * sizeof(char) + + MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64); + UnwindFailures::UnwindFailures() : _nameCount(0) { _names = new char[MAX_UNWIND_FAILURE_NAMES][MAX_NAME_LENGTH]; - _counters = new u64[MAX_UNWIND_FAILURE_NAMES][UNWIND_FAILURE_ANY + 1]; - reset(); + _counters = new u64[MAX_UNWIND_FAILURE_NAMES][UNWIND_FAILURE_ANY + 1]; + NativeMem::record(NM_THREAD_LOCAL, (long long)UNWIND_FAILURES_ARRAYS_SIZE); + reset(); } UnwindFailures::~UnwindFailures() { delete[] _names; delete[] _counters; + // ProfiledThread holds one of these by value, so without this the gauge + // grows by UNWIND_FAILURES_ARRAYS_SIZE for every thread that comes and + // goes -- forTid()/freeValue() only balance sizeof(ProfiledThread) itself. + // Recorded after the deletes, matching the other NM_THREAD_LOCAL + // decrement sites in threadLocalData.cpp. + NativeMem::record(NM_THREAD_LOCAL, -(long long)UNWIND_FAILURES_ARRAYS_SIZE); } void UnwindFailures::reset() { diff --git a/ddprof-lib/src/test/cpp/nativeMem_ut.cpp b/ddprof-lib/src/test/cpp/nativeMem_ut.cpp index d2f3bf1822..3470740267 100644 --- a/ddprof-lib/src/test/cpp/nativeMem_ut.cpp +++ b/ddprof-lib/src/test/cpp/nativeMem_ut.cpp @@ -9,8 +9,31 @@ class NativeMemTest : public ::testing::Test { protected: - void SetUp() override { NativeMem::reset(); } - void TearDown() override { NativeMem::reset(); } + // reset() zeroes the process-wide gauges, but objects constructed before this + // suite ran have already accounted their allocations there and will decrement + // again when they are destroyed -- UnwindStats::_unwind_failures, a static + // whose UnwindFailures dtor gives back NM_THREAD_LOCAL at process exit, is + // one. Leaving the gauges at zero makes those later decrements underflow and + // trip record()'s `updated >= 0` invariant after the last test has passed, so + // put the pre-test baseline back on the way out. + void SetUp() override { + for (int c = 0; c < NM_NUM_CATEGORIES; c++) { + _saved_live[c] = NativeMem::live((NativeMemCategory)c); + } + NativeMem::reset(); + } + + void TearDown() override { + NativeMem::reset(); + for (int c = 0; c < NM_NUM_CATEGORIES; c++) { + if (_saved_live[c] != 0) { + NativeMem::setLive((NativeMemCategory)c, _saved_live[c]); + } + } + } + +private: + long long _saved_live[NM_NUM_CATEGORIES]; }; // record() adds to and subtracts from the per-category live gauge, and the diff --git a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp index 4cd5690a69..e3f85b94b0 100644 --- a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp @@ -9,6 +9,7 @@ #include "guards.h" #include "common.h" // TSAN_ENABLED (toolchain-agnostic sanitizer detection) #include "threadLocalData.h" +#include "threadLocalDataPool.h" #include #include #include @@ -162,13 +163,29 @@ class StressTestSuite : public ::testing::Test { void SetUp() override { // Install crash handler for detailed debugging installGtestCrashHandler(); - + + // Signal-handler TLS priming is the only way a thread this suite spawns + // (a plain std::thread that never registers with the profiler) can end up + // with a ProfiledThread: ProfiledThread::acquireCurrent() claims a pool + // slot without allocating, which is what SignalHandlerScope relies on. + // That pool is normally created by JVMSupport during agent startup, and + // this binary has no JVM -- leaving _pool null, so acquireCurrent() + // returns null and CriticalSection's ctor trips its + // `_thread_ptr != nullptr` assertion (gtest builds keep asserts live). + // Create it here instead. initialize() is not idempotent -- it news a + // fresh pool and overwrites the singleton -- and SetUp() runs per test, + // so it must happen exactly once for the process. + static std::once_flag pool_once; + std::call_once(pool_once, [] { ThreadLocalDataPool::initialize(); }); + // Initialize shared storage if not already done if (!shared_storage) { shared_storage = std::make_unique(); } - + // Clear any traces from previous tests to start fresh + ProfiledThread* pt = ProfiledThread::initCurrentThreadSignalSafe(); + assert(pt != nullptr); shared_storage->clear(); } @@ -251,6 +268,7 @@ TEST_F(StressTestSuite, SwapStormTest) { // Use mutex to ensure single-threaded processTraces access - matches production { std::lock_guard lock(process_traces_mutex); + ProfiledThread::initCurrentThreadSignalSafe(); storage->processTraces([](const std::unordered_set& traces) { // Process traces (simulating JFR serialization) (void)traces.size(); @@ -879,6 +897,7 @@ TEST_F(StressTestSuite, TLSOverrunCanaryTest) { try { { + ProfiledThread::initCurrentThreadSignalSafe(); std::lock_guard lock(process_traces_mutex); storage->processTraces([](const std::unordered_set& traces) { // Aggressive processing to stress TLS during swaps @@ -1067,12 +1086,16 @@ static CallTraceStorage* realistic_shared_storage = nullptr; // Signal handler for pressure test void pressure_signal_handler(int sig) { + SIGNAL_HANDLER_GUARD_NO_SAMPLE(); + if (!signal_pressure_active.load()) { + SIGNAL_HANDLER_GUARD_RELEASE(); return; } CriticalSection cs; if (!cs.entered()) { + SIGNAL_HANDLER_GUARD_RELEASE(); // behave like the real-life signal handler return; } @@ -1096,11 +1119,17 @@ void pressure_signal_handler(int sig) { // Realistic signal handler for profiler stress test void realistic_profiler_signal_handler(int sig) { - if (!realistic_test_running.load()) return; + SIGNAL_HANDLER_GUARD_NO_SAMPLE(); + + if (!realistic_test_running.load()) { + SIGNAL_HANDLER_GUARD_RELEASE(); + return; + } CriticalSection cs; // Critical: Check if critical section is active (storage swap in progress) if (!cs.entered()) { + SIGNAL_HANDLER_GUARD_RELEASE(); return; // Skip this signal - storage operation in progress } @@ -1553,6 +1582,9 @@ static void realProfilerSignalStressImpl(int signal_barrage_count, int num_worke std::vector workers; for (int t = 0; t < num_worker_threads; ++t) { workers.emplace_back([&, t]() { + // Needed before any put()/processTraces() call -- see the worker threads + // in InstanceIdTraceIdStressTest for why. + ProfiledThread::initCurrentThreadSignalSafe(); while (test_running.load()) { try { // Simulate normal application work that profiler samples @@ -1580,6 +1612,7 @@ static void realProfilerSignalStressImpl(int signal_barrage_count, int num_worke // Single dump thread - represents realistic JFR dump operations // In production, this would be protected by mutex and only one thread does dumps std::thread dump_thread([&]() { + ProfiledThread::initCurrentThreadSignalSafe(); int dump_count = 0; while (test_running.load() && dump_count < 3) { // Only do a few dumps try { @@ -1680,6 +1713,11 @@ TEST_F(StressTestSuite, InstanceIdTraceIdStressTest) { std::vector workers; for (int t = 0; t < NUM_THREADS; ++t) { workers.emplace_back([&, t]() { + // put()/processTraces() build a CriticalSection, which requires this + // thread to already have a ProfiledThread. Only signal handlers get one + // implicitly (via SignalHandlerScope -> acquireCurrent()), so a plain + // worker thread has to ask for one up front. + ProfiledThread::initCurrentThreadSignalSafe(); for (int op = 0; op < OPERATIONS_PER_THREAD && !test_failed.load(); ++op) { try { // Use the single shared storage instance @@ -1774,6 +1812,7 @@ TEST_F(StressTestSuite, InstanceIdTraceIdStressTest) { // Additional thread that does rapid processTraces() calls to stress instance ID assignment std::thread rapid_swapper([&]() { + ProfiledThread::initCurrentThreadSignalSafe(); for (int swap = 0; swap < RAPID_SWAPS_COUNT && !test_failed.load(); ++swap) { try { // Use single shared storage instance for swap diff --git a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp index 8ad8a336e9..50d9da0367 100644 --- a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp +++ b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp @@ -139,6 +139,7 @@ static void churn_worker(ThreadFilter* filter, bool with_dump) { // dump path where Profiler::rotateDictsAndRun() holds all shard locks while // writeStackTraces() calls processCallTraces(). static void dump_thread() { + ProfiledThread::initCurrentThreadSignalSafe(); while (g_run.load(std::memory_order_relaxed)) { lock_all(); g_storage.processTraces([](const std::unordered_set& traces) { @@ -155,6 +156,7 @@ static void dump_thread() { } TEST(StressThreadLifecycle, Smoke) { + ProfiledThread::initCurrentThreadSignalSafe(); CallTraceStorage storage; storage.clear(); SUCCEED(); diff --git a/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp index 3483461fda..25f1b766c3 100644 --- a/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp @@ -16,6 +16,7 @@ #include "gtest_crash_handler.h" #include "arch.h" #include "counters.h" +#include "threadLocalData.h" // Test name for crash handler static constexpr char TEST_NAME[] = "CallTraceStorageTest"; @@ -48,6 +49,8 @@ class CallTraceStorageTest : public ::testing::Test { }; TEST_F(CallTraceStorageTest, BasicFunctionality) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Create a simple call frame ASGCT_CallFrame frame; frame.bci = 10; @@ -66,6 +69,8 @@ TEST_F(CallTraceStorageTest, BasicFunctionality) { } TEST_F(CallTraceStorageTest, LivenessCheckerRegistration) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Store multiple traces first ASGCT_CallFrame frames[4]; frames[0].bci = 10; frames[0].method_id = (jmethodID)0x1111; @@ -118,6 +123,8 @@ TEST_F(CallTraceStorageTest, LivenessCheckerRegistration) { } TEST_F(CallTraceStorageTest, MultipleLivenessCheckers) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Store multiple traces with more variety ASGCT_CallFrame frames[5]; frames[0].bci = 10; frames[0].method_id = (jmethodID)0x1111; @@ -174,6 +181,8 @@ TEST_F(CallTraceStorageTest, MultipleLivenessCheckers) { } TEST_F(CallTraceStorageTest, TraceIdPreservation) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Create a simple frame ASGCT_CallFrame frame; frame.bci = 10; @@ -218,6 +227,8 @@ TEST_F(CallTraceStorageTest, TraceIdPreservation) { } TEST_F(CallTraceStorageTest, ClearMethod) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Store a trace ASGCT_CallFrame frame; frame.bci = 10; @@ -242,6 +253,8 @@ TEST_F(CallTraceStorageTest, ClearMethod) { } TEST_F(CallTraceStorageTest, ConcurrentClearAndPut) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Test concurrent access patterns that might cause NULL dereferences ASGCT_CallFrame frame; frame.bci = 10; @@ -268,6 +281,8 @@ TEST_F(CallTraceStorageTest, ConcurrentClearAndPut) { } TEST_F(CallTraceStorageTest, ConcurrentTableExpansionRegression) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Regression test for the crash during table expansion in CallTraceHashTable::put // The crash occurred at __sync_bool_compare_and_swap(&_current_table, table, new_table) // when multiple threads triggered table expansion simultaneously @@ -365,6 +380,8 @@ TEST_F(CallTraceStorageTest, ConcurrentTableExpansionRegression) { * to the original_active after the active area swap. */ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Synchronization primitives for coordinating the test std::atomic swap_can_proceed{false}; std::atomic put_threads_ready{false}; @@ -428,6 +445,7 @@ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { put_threads_ready_count.fetch_add(1); } ready_cv.notify_all(); + ProfiledThread::initCurrentThreadSignalSafe(); // Wait for permission to proceed std::unique_lock lock(swap_mutex); @@ -458,6 +476,8 @@ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { // Perform processTraces in separate thread to trigger the storage swap std::thread process_thread([&]() { + ProfiledThread::initCurrentThreadSignalSafe(); + // Start processing - this will swap storage storage->processTraces([&](const std::unordered_set& traces) { collection_started = true; @@ -562,6 +582,8 @@ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { * This test should FAIL (crash or ASan error) before the fix and PASS after. */ TEST_F(CallTraceStorageTest, UseAfterFreeInProcessTraces) { + ProfiledThread::initCurrentThreadSignalSafe(); + // Create multiple traces with varying frame counts to increase memory footprint const int NUM_TRACES = 100; const int MAX_FRAMES = 20; @@ -743,6 +765,7 @@ TEST_F(CallTraceStorageTest, PutWithExistingIdNoInfiniteLoopWhenFull) { TEST_F(CallTraceStorageTest, LivenessPreservationAcrossMultipleCycles) { const int N = 200; + ProfiledThread::initCurrentThreadSignalSafe(); // Insert N unique traces and record their IDs and frame values for later checks. std::vector ids; std::vector bcis; @@ -767,6 +790,7 @@ TEST_F(CallTraceStorageTest, LivenessPreservationAcrossMultipleCycles) { for (int cycle = 0; cycle < CYCLES; cycle++) { std::atomic done{false}; std::thread t([&] { + ProfiledThread::initCurrentThreadSignalSafe(); storage->processTraces([&](const std::unordered_set& traces) { // All N preserved traces must be present, plus the dropped sentinel. EXPECT_GE(traces.size(), static_cast(N + 1)) @@ -817,6 +841,7 @@ TEST_F(CallTraceStorageTest, ClearTableOnlyDisconnectsFullChain) { const int NUM_TRACES = 50000; std::vector ids; ids.reserve(NUM_TRACES); + ProfiledThread::initCurrentThreadSignalSafe(); for (int i = 0; i < NUM_TRACES; i++) { ASGCT_CallFrame frame; @@ -852,6 +877,7 @@ TEST_F(CallTraceStorageTest, ClearTableOnlyDisconnectsFullChain) { TEST_F(CallTraceStorageTest, CollectFindsAllTracesAcrossExpandedChain) { const int NUM_TRACES = 50000; std::unordered_set inserted_ids; + ProfiledThread::initCurrentThreadSignalSafe(); for (int i = 0; i < NUM_TRACES; i++) { ASGCT_CallFrame frame; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java index 6345a25b4a..2dc75160b3 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/cpu/TlsPrimingTest.java @@ -5,23 +5,18 @@ package com.datadoghq.profiler.cpu; import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.JfrEvent; +import com.datadoghq.profiler.JfrEvents; import com.datadoghq.profiler.Platform; import org.junitpioneer.jupiter.RetryingTest; import org.objectweb.asm.ClassWriter; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; -import org.openjdk.jmc.common.item.IItem; -import org.openjdk.jmc.common.item.IItemCollection; -import org.openjdk.jmc.common.item.IItemIterable; -import org.openjdk.jmc.common.item.IMemberAccessor; -import org.openjdk.jmc.flightrecorder.jdk.JdkAttributes; import java.lang.reflect.Method; -import java.util.Map; import java.util.TreeSet; import java.util.Set; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -38,18 +33,17 @@ * from the pool and attaches it via pthread_setspecific right there in the * signal handler (see threadLocalData.cpp/ThreadLocalDataPool). * - * Seeing a compiler-thread eventThread alone is not sufficient evidence: the - * CPU signal handlers resolve tid via OS::threadId() before recordSample() is - * even called, and native thread names are refreshed independently of TLS - * priming, so a sample tagged with a compiler thread's name would show up - * regardless of whether ProfiledThread::acquireCurrent() actually succeeded. - * If priming fails, recordSample() still emits an event for that tid, just - * with a synthetic "no_Java_frame" stack instead of a real unwind. The - * "samples_dropped_thread_local" debug counter is incremented exactly when - * acquireCurrent() fails (see StackWalker::walkFP/walkDwarf), so asserting it - * stayed at zero for the whole run is what actually proves every signal that - * reached a stack walker — including the ones on these never-registered - * compiler threads — found or attached a ProfiledThread. + * The presence of compiler-thread samples is itself the proof that priming + * worked on those threads. CTimer::signalHandler (the engine behind "cpu=") + * opens with SIGNAL_HANDLER_GUARD(), which returns before recordSample() is + * ever reached whenever ProfiledThread::acquireCurrent() comes back null (see + * ctimer_linux.cpp and guards.h). A compiler thread whose priming failed + * therefore contributes no datadog.ExecutionSample at all, so counting those + * samples is a direct, per-thread observation rather than a proxy. + * + * Note that these samples legitimately carry an empty frame list -- a compiler + * thread has no Java stack to unwind -- so the stack contents cannot be used + * as a priming signal, only the samples' existence. * * The test forces JIT compilation by loading a dynamically-generated class * with many distinct trivial methods and invoking each one past HotSpot's/ @@ -73,45 +67,65 @@ public class TlsPrimingTest extends AbstractProfilerTest { // drain the queue while the CPU sampler is still active. private static final long COMPILE_DRAIN_WAIT_MS = 4000; + // Floor for the proportional drop allowance, so a run that happened to record + // very few samples doesn't turn a couple of incidental pool-slot losses into a + // failure. Mirrors NativeThreadPrimingTest's MAX_SYNTHETIC_NATIVE_THREAD_SAMPLES. + private static final long MIN_DROPPED_ALLOWANCE = 10; + @RetryingTest(3) public void compilerThreadSamplesArePresent() throws Exception { triggerJitCompilation(); stopProfiler(); - IItemCollection events = verifyEvents("datadog.ExecutionSample"); + JfrEvents events = verifyEvents("datadog.ExecutionSample"); String expectedPrefix = Platform.isJ9() ? J9_COMPILER_THREAD_PREFIX : HOTSPOT_COMPILER_THREAD_PREFIX; Set observedThreadNames = new TreeSet<>(); - boolean sawCompilerThreadSample = false; - for (IItemIterable cpuSamples : events) { - IMemberAccessor threadNameAccessor = - JdkAttributes.EVENT_THREAD_NAME.getAccessor(cpuSamples.getType()); - for (IItem sample : cpuSamples) { - String threadName = threadNameAccessor.getMember(sample); - if (threadName == null) { - continue; - } - observedThreadNames.add(threadName); - if (threadName.startsWith(expectedPrefix)) { - sawCompilerThreadSample = true; - } + long totalSamples = 0; + long compilerThreadSamples = 0; + for (JfrEvent sample : events) { + String threadName = sample.getThreadName("eventThread"); + if (threadName == null) { + continue; + } + totalSamples++; + observedThreadNames.add(threadName); + if (threadName.startsWith(expectedPrefix)) { + compilerThreadSamples++; } } - assertTrue(sawCompilerThreadSample, + assertTrue(compilerThreadSamples > 0, "expected a datadog.ExecutionSample with eventThread starting with \"" + expectedPrefix + "\", but observed thread names: " + observedThreadNames); - // A compiler-thread eventThread on its own doesn't prove a pool slot was - // ever attached (see class javadoc) — the tid and thread name are resolved - // independently of priming. Confirm no signal ever fell back to the - // "no_Java_frame" stack for lack of a ProfiledThread, on this or any other - // thread in the run. - Map debugCounters = profiler.getDebugCounters(); - assertEquals(0L, debugCounters.get("samples_dropped_thread_local"), - "TLS priming failed for at least one signal; compiler-thread samples " - + "may have used the no_Java_frame fallback instead of a real unwind"); + // Secondary, whole-process signal only -- see the class javadoc for why the + // assertion above is the actual proof. samples_dropped_thread_local cannot be + // asserted to be exactly zero here: + // * it is absent entirely from builds without -DCOUNTERS, where + // getDebugCounters() hands back an empty map (javaApi.cpp/JavaProfiler); + // * it is shared with the mallocTracer, wallClock, nativeSocketSampler and + // JVMTI recording paths, and with every SIGNAL_HANDLER_GUARD() -- including + // the non-sampling wakeupHandler in vmEntry.cpp; and + // * it also counts threads that merely lost the race for one of the pool's + // 64 slots (ThreadLocalDataPool::DEFAULT_CAPACITY) rather than hitting a + // priming bug, and plain capacity exhaustion does not set the + // thread_local_pool_exhausted counter that would let us tell the two apart. + // A systematic priming failure drops samples on the order of the sample count, + // so bound it proportionally instead: that still catches a real regression + // without failing on a machine whose JVM simply runs more threads than the pool + // has slots. + Long droppedThreadLocal = profiler.getDebugCounters().get("samples_dropped_thread_local"); + System.out.println("compilerThreadSamples=" + compilerThreadSamples + "/" + totalSamples + + ", samples_dropped_thread_local=" + droppedThreadLocal); + if (droppedThreadLocal != null) { + long maxDrops = Math.max(MIN_DROPPED_ALLOWANCE, totalSamples / 20); + assertTrue(droppedThreadLocal <= maxDrops, + "TLS priming failed for an implausible number of signals: " + + droppedThreadLocal + " drops for " + totalSamples + + " recorded samples (allowed at most " + maxDrops + ")"); + } } private void triggerJitCompilation() throws Exception { From 884ca9a151c49877c00a8f84b1730386853bff10 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 16:00:43 +0200 Subject: [PATCH 26/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/javaApi.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index c02de5d0c1..cd6ae12e5c 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -79,8 +79,11 @@ Java_com_datadoghq_profiler_JavaProfiler_init0(JNIEnv *env, jclass unused) { // JavaVM* has already been stored when the native library was loaded so we can pass nullptr here if (VM::initProfilerBridge(nullptr, true)) { // Attach ProfiledThread - ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); - assert(current != nullptr && "Out of order initialization"); + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + if (current == nullptr) { + throwNew(env, "java/lang/IllegalStateException", "Failed to initialize profiler thread-local state"); + return JNI_FALSE; + } return JNI_TRUE; } else { From bdc6e3edd7e751fce703e0e980ed336681c1bda5 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 16:37:41 +0000 Subject: [PATCH 27/30] Fix --- ddprof-lib/src/main/cpp/ctimer_linux.cpp | 2 +- ddprof-lib/src/main/cpp/javaApi.cpp | 6 ++++ ddprof-lib/src/main/cpp/mallocTracer.cpp | 9 ++++-- ddprof-lib/src/main/cpp/perfEvents_linux.cpp | 2 +- .../src/main/cpp/threadLocalDataPool.cpp | 1 + .../src/test/cpp/threadLocalDataPool_ut.cpp | 28 +++++++++++-------- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index e3fd527f1e..70c40c3bf0 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -255,7 +255,6 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { } void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD(); // Reject signals that did not originate from our timer_create timers. // This guards against Go's process-wide setitimer(ITIMER_PROF) and other // foreign SIGPROF sources that would otherwise drive our handler onto @@ -267,6 +266,7 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { } Counters::increment(CTIMER_SIGNAL_OWN); + SIGNAL_HANDLER_GUARD(); InflightGuard inflight; ProfiledThread* current = SIGNAL_HANDLER_CURRENT_THREAD(); assert(current != nullptr); diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index c02de5d0c1..f8fab31dd2 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -90,6 +90,9 @@ Java_com_datadoghq_profiler_JavaProfiler_init0(JNIEnv *env, jclass unused) { extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_stop0(JNIEnv *env, jobject unused) { + // Attach ProfiledThread + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + assert(current != nullptr && "Out of order initialization"); Error error = Profiler::instance()->stop(); if (error) { @@ -1163,6 +1166,9 @@ extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JavaProfiler_testReadContextAttribute0(JNIEnv* env, jclass unused, jint slot) { ProfiledThread *thrd = ProfiledThread::initCurrentThreadSignalSafe(); assert(thrd != nullptr && "Out of order initialization"); + if (slot < 0 || slot >= (jint)DD_TAGS_CAPACITY) { + return nullptr; + } OtelThreadContextRecord* record = thrd->getOtelContextRecord(); int targetKey = slot + 1; diff --git a/ddprof-lib/src/main/cpp/mallocTracer.cpp b/ddprof-lib/src/main/cpp/mallocTracer.cpp index 1b87060f52..2e4340ac67 100644 --- a/ddprof-lib/src/main/cpp/mallocTracer.cpp +++ b/ddprof-lib/src/main/cpp/mallocTracer.cpp @@ -44,9 +44,12 @@ static inline void maybeRecord(void* ret, size_t size) { if (MallocTracer::running() && ret && size) { // Even we are not in a signal handler, we cannot malloc or // we may get into indefinite loop - if (ProfiledThread::acquireCurrent() == nullptr) { - Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); - return; + { + SignalBlocker blocker; + if (ProfiledThread::acquireCurrent() == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return; + } } CriticalSection cs; if (cs.entered()) { diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp index 61f964d0fd..4b61c15f9e 100644 --- a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp +++ b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp @@ -737,11 +737,11 @@ u64 PerfEvents::readCounter(siginfo_t *siginfo, void *ucontext) { } void PerfEvents::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD(); if (siginfo->si_code <= 0) { // Looks like an external signal; don't treat as a profiling event return; } + SIGNAL_HANDLER_GUARD(); InflightGuard inflight; // A thread with no ProfiledThread attached must never enter the critical diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 73f3bbf2f9..bacb28d38f 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -47,6 +47,7 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { uint16_t used = __atomic_fetch_add(&_used, 1, __ATOMIC_RELAXED); if (used >= _capacity) { __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); + Counters::increment(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); return nullptr; } diff --git a/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp index 038354e119..fdd622ea63 100644 --- a/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp @@ -75,24 +75,30 @@ TEST_F(ThreadLocalDataPoolTest, nullptrIsNotContained) { ThreadLocalDataPool::destroyForTest(pool); } -// Covers claim()'s (used >= _capacity) fast-path rejection once the pool is -// full. claim()'s return value is nullptr whether that guard reads `>=` or -// (bugged) `>`, because a bugged `>` still falls through to the slot-scanning -// loop, which finds every real slot already claimed and also returns nullptr. -// The observable difference is that the buggy fallthrough additionally -// increments SAMPLES_DROPPED_TLS_POOL_EXHAUSTED, which the fast path must not -// do -- that's what this test pins down. -TEST_F(ThreadLocalDataPoolTest, claimAtCapacityRejectsWithoutExhaustionScan) { +// Covers claim() once every slot is taken. Two things must hold, and neither is +// visible in the return value alone (which is nullptr on every failure path): +// +// 1. The drop is recorded in SAMPLES_DROPPED_TLS_POOL_EXHAUSTED. This counter +// is the only signal that priming is silently failing in the field; an +// earlier revision incremented it solely on the scan fallthrough, so the +// real (used >= _capacity) path dropped samples untracked. +// 2. The rejected claim rolls back the speculative _used increment it took +// before the capacity check. Without the rollback, _used never falls back +// below _capacity and the pool stays permanently full even after slots are +// returned -- which the reclaim at the end of this test exercises. +TEST_F(ThreadLocalDataPoolTest, claimAtCapacityRejectsAndRecordsExhaustion) { ThreadLocalDataPool* pool = ThreadLocalDataPool::createForTest(2); - ASSERT_NE(pool->claimForTest(0), nullptr); + ProfiledThread* first = pool->claimForTest(0); + ASSERT_NE(first, nullptr); ASSERT_NE(pool->claimForTest(1), nullptr); long long before = Counters::getCounter(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); EXPECT_EQ(pool->claimForTest(2), nullptr); - long long after = Counters::getCounter(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED); + EXPECT_EQ(Counters::getCounter(SAMPLES_DROPPED_TLS_POOL_EXHAUSTED), before + 1); - EXPECT_EQ(after, before); + EXPECT_TRUE(pool->unclaimForTest(first)); + EXPECT_NE(pool->claimForTest(0), nullptr); ThreadLocalDataPool::destroyForTest(pool); } From 7ff0b393f66414de1bf6fabede10d791a334044b Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 18:16:39 +0000 Subject: [PATCH 28/30] Reset pooled ProfiledThread at release, out of signal handler --- ddprof-lib/src/main/cpp/threadLocalData.cpp | 14 ++++++++------ ddprof-lib/src/main/cpp/threadLocalData.h | 16 +++++++--------- ddprof-lib/src/main/cpp/threadLocalDataPool.cpp | 12 ++++-------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index fe57260e90..73939b732a 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -115,35 +115,37 @@ Context ProfiledThread::snapshotContext(size_t numAttrs) { return ctx; } -void ProfiledThread::resetClaimed(int tid) { +void ProfiledThread::unclaimAndReset() { _unwinding_Java = false; _jmp_buf = nullptr; _pc = 0; _sp = 0; _span_id = 0; _crash_depth = 0; - _tid = tid; + _tid = 0; _cpu_epoch = 0; _wall_epoch = 0; _call_trace_id = 0; _recording_epoch = 0; - __atomic_store_n(&_misc_flags, FLAG_CLAIMED, __ATOMIC_RELEASE); _park_block_token = 0; _filter_slot_id = -1; _init_window = 0; _signal_depth = 0; + _in_critical_section = false; + _otel_ctx_initialized = false; _otel_ctx_record = {}; + _otel_local_root_span_id = 0; for (int index = 0; index < DD_TAGS_CAPACITY; index++) { _otel_tag_encodings[index] = 0; } - _otel_local_root_span_id = 0; - _in_critical_section = false; - _unwind_failures.reset(); + _unwind_failures.reset(); #ifdef __FAULT_INJECTION__ _fi_rng = ((u64)(uintptr_t)this) ^ (0x9e3779b97f4a7c15ULL * (u64)tid); if (_fi_rng == 0) _fi_rng = 1; #endif + + __atomic_store_n(&_misc_flags, 0, __ATOMIC_RELEASE); } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 09e3665fbf..1c4fd1be68 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -123,25 +123,23 @@ class ProfiledThread : public ThreadLocalData { virtual ~ProfiledThread() { } - // Reset content of claimed slot - void resetClaimed(int tid); - inline bool isClaimed() const { return (__atomic_load_n(&_misc_flags, __ATOMIC_RELAXED) & FLAG_CLAIMED) == FLAG_CLAIMED; } - inline void unclaim() { - assert(isClaimed() && "Slot has been claimed"); - __atomic_fetch_and(&_misc_flags, ~FLAG_CLAIMED, __ATOMIC_RELEASE); - } + void unclaimAndReset(); - inline bool claimAcquire() { + inline bool claimAcquire(int tid) { if (isClaimed()) { return false; } u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); - return (flags & FLAG_CLAIMED) == 0; + if((flags & FLAG_CLAIMED) == 0) { + _tid = tid; + return true; + } + return false; } public: diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index bacb28d38f..6961af2c59 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -54,7 +54,7 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { int start_pos = tid % _capacity; int index = start_pos; do { - if (_threads[index].claimAcquire()) { + if (_threads[index].claimAcquire(tid)) { return &_threads[index]; } index = (index + 1) % _capacity; @@ -66,8 +66,8 @@ ProfiledThread* ThreadLocalDataPool::claim(int tid) { bool ThreadLocalDataPool::unclaim(ProfiledThread* t) { if (contains(t)) { - t->unclaim(); - uint16_t used = __atomic_fetch_add(&_used, -1, __ATOMIC_RELEASE); + t->unclaimAndReset(); + uint16_t used = __atomic_fetch_add(&_used, -1, __ATOMIC_RELAXED); assert(used > 0); return true; } @@ -85,11 +85,7 @@ ProfiledThread* ThreadLocalDataPool::acquire(int tid) { if (pool == nullptr) { return nullptr; } else { - ProfiledThread* t = pool->claim(tid); - if (t != nullptr) { - t->resetClaimed(tid); - } - return t; + return pool->claim(tid); } } From f7f7e4ab5f35a2eb04f6622caf790132a4585897 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 19:47:19 +0000 Subject: [PATCH 29/30] Fix --- ddprof-lib/src/main/cpp/ctimer_linux.cpp | 2 +- ddprof-lib/src/main/cpp/itimer.cpp | 2 +- ddprof-lib/src/main/cpp/profiler.cpp | 3 +++ ddprof-lib/src/main/cpp/stackWalker.cpp | 4 +--- ddprof-lib/src/main/cpp/threadLocalData.cpp | 5 ++--- ddprof-lib/src/main/cpp/threadLocalData.h | 14 +------------ .../src/main/cpp/threadLocalData.inline.h | 20 ++++++++++++++++++- .../src/main/cpp/threadLocalDataPool.cpp | 2 +- ddprof-lib/src/main/cpp/threadLocalDataPool.h | 1 + ddprof-lib/src/main/cpp/wallClock.cpp | 7 +++---- 10 files changed, 33 insertions(+), 27 deletions(-) diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index 70c40c3bf0..9fa5bbafec 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -206,7 +206,6 @@ Error CTimerJvmti::start(Arguments &args) { } void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD(); if (!OS::shouldProcessSignal(siginfo, SI_TIMER, SignalCookie::cpu())) { Counters::increment(CTIMER_SIGNAL_FOREIGN); OS::forwardForeignSignal(signo, siginfo, ucontext); @@ -214,6 +213,7 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { } Counters::increment(CTIMER_SIGNAL_OWN); + SIGNAL_HANDLER_GUARD(); InflightGuard inflight; CriticalSection cs; diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp index eeff35c2e6..a9a873ded6 100644 --- a/ddprof-lib/src/main/cpp/itimer.cpp +++ b/ddprof-lib/src/main/cpp/itimer.cpp @@ -102,6 +102,7 @@ bool ITimerJvmti::_enabled = false; long ITimerJvmti::_interval = 0; void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { + int saved_errno = errno; SIGNAL_HANDLER_GUARD(); ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); assert(current != nullptr); @@ -111,7 +112,6 @@ void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { if (!cs.entered()) { return; } - int saved_errno = errno; if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { errno = saved_errno; return; diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 50c7b28463..c3dcf55131 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -784,6 +784,9 @@ void Profiler::recordQueueTime(int tid, QueueTimeEvent *event) { void Profiler::recordExternalSample(u64 weight, int tid, int num_frames, ASGCT_CallFrame *frames, bool truncated, jint event_type, Event *event) { + // This is a non-signal based sampler + ProfiledThread::initCurrentThreadSignalSafe(); + // Protect external sampling operations to prevent signal handler interference CriticalSection cs; atomicIncRelaxed(_total_samples); diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index b7a30abe10..95a9870017 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -95,9 +95,7 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S fp = (uintptr_t)SafeAccess::load(INJECT_FAULT_ADDRESS_LIKELY((void**)fp)); } - if (prof_thread != nullptr) { - prof_thread->setJmpCtx(prev_jmp_buf); - } + prof_thread->setJmpCtx(prev_jmp_buf); if (truncated && depth > max_depth) { *truncated = true; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 73939b732a..98544989f1 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -142,9 +142,8 @@ void ProfiledThread::unclaimAndReset() { _unwind_failures.reset(); - #ifdef __FAULT_INJECTION__ - _fi_rng = ((u64)(uintptr_t)this) ^ (0x9e3779b97f4a7c15ULL * (u64)tid); - if (_fi_rng == 0) _fi_rng = 1; +#ifdef __FAULT_INJECTION__ + _fi_rng = 0; #endif __atomic_store_n(&_misc_flags, 0, __ATOMIC_RELEASE); diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 1c4fd1be68..d16b6d9210 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -129,19 +129,7 @@ class ProfiledThread : public ThreadLocalData { void unclaimAndReset(); - inline bool claimAcquire(int tid) { - if (isClaimed()) { - return false; - } - - u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); - if((flags & FLAG_CLAIMED) == 0) { - _tid = tid; - return true; - } - return false; - } - + inline bool claimAcquire(int tid); public: static ProfiledThread *forTid(int tid) { ProfiledThread *pt = new ProfiledThread(tid); diff --git a/ddprof-lib/src/main/cpp/threadLocalData.inline.h b/ddprof-lib/src/main/cpp/threadLocalData.inline.h index c10ed2c41f..856b7facc7 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.inline.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.inline.h @@ -18,7 +18,7 @@ inline ProfiledThread* ProfiledThread::current() { return _current_thread.get(); } -ProfiledThread* ProfiledThread::acquireCurrent() { +inline ProfiledThread* ProfiledThread::acquireCurrent() { ProfiledThread* prof_thread = current(); if (prof_thread == nullptr) { SignalBlocker blocker; @@ -34,4 +34,22 @@ ProfiledThread* ProfiledThread::acquireCurrent() { return prof_thread; } +inline bool ProfiledThread::claimAcquire(int tid) { + if (isClaimed()) { + return false; + } + + u32 flags = __atomic_fetch_or(&_misc_flags, FLAG_CLAIMED, __ATOMIC_ACQUIRE); + if((flags & FLAG_CLAIMED) == 0) { + _tid = tid; +#ifdef __FAULT_INJECTION__ + _fi_rng = ((u64)(uintptr_t)this) ^ (0x9e3779b97f4a7c15ULL * (u64)tid); + if (_fi_rng == 0) _fi_rng = 1; +#endif + return true; + } + return false; +} + + #endif // THREADLOCALDATA_INLINE_H diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp index 6961af2c59..4f59f6da07 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.cpp @@ -4,7 +4,7 @@ */ #include "counters.h" -#include "threadLocalData.h" +#include "threadLocalData.inline.h" #include "threadLocalDataPool.h" #include diff --git a/ddprof-lib/src/main/cpp/threadLocalDataPool.h b/ddprof-lib/src/main/cpp/threadLocalDataPool.h index 706d58c90a..a93c6b883e 100644 --- a/ddprof-lib/src/main/cpp/threadLocalDataPool.h +++ b/ddprof-lib/src/main/cpp/threadLocalDataPool.h @@ -13,6 +13,7 @@ class ProfiledThread; class ThreadLocalDataPool { + static constexpr uint16_t DEFAULT_CAPACITY = 64; private: static ThreadLocalDataPool* _pool; diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp index a41029ebab..7362a9593f 100644 --- a/ddprof-lib/src/main/cpp/wallClock.cpp +++ b/ddprof-lib/src/main/cpp/wallClock.cpp @@ -205,18 +205,18 @@ bool BaseWallClock::inSyscall(void *ucontext) { void WallClockASGCT::sharedSignalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD(); // Reject any SIGVTALRM that did not originate from our rt_tgsigqueueinfo // send. Defends against stray in-process tgkill / external sigqueue that // would otherwise drive our wallclock sampling path. if (!OS::shouldProcessSignal(siginfo, SI_QUEUE, SignalCookie::wallclock())) { Counters::increment(WALLCLOCK_SIGNAL_FOREIGN); - SIGNAL_HANDLER_GUARD_RELEASE(); OS::forwardForeignSignal(signo, siginfo, ucontext); return; } Counters::increment(WALLCLOCK_SIGNAL_OWN); + SIGNAL_HANDLER_GUARD(); + WallClockASGCT *engine = reinterpret_cast(Profiler::instance()->wallEngine()); // Past the foreign-signal filter: any work below this point can write JFR. // Participate in SignalInflight::drain() so Profiler::stop() does not tear @@ -424,17 +424,16 @@ void WallClockASGCT::timerLoop() { void WallClockJvmti::sharedSignalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD(); // Reject any SIGVTALRM that did not originate from our rt_tgsigqueueinfo // send (mirrors WallClockASGCT). Defends against stray in-process tgkill or // external sigqueue driving the JVMTI RequestStackTrace path. if (!OS::shouldProcessSignal(siginfo, SI_QUEUE, SignalCookie::wallclock())) { Counters::increment(WALLCLOCK_SIGNAL_FOREIGN); - SIGNAL_HANDLER_GUARD_RELEASE(); OS::forwardForeignSignal(signo, siginfo, ucontext); return; } Counters::increment(WALLCLOCK_SIGNAL_OWN); + SIGNAL_HANDLER_GUARD(); WallClockJvmti *engine = reinterpret_cast(Profiler::instance()->wallEngine()); From 1eede8ff5a07e8fa7fec9b6a0eb873a3d69cbd81 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 7 Aug 2026 22:51:26 +0200 Subject: [PATCH 30/30] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ddprof-lib/src/main/cpp/profiler.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index c3dcf55131..eacb2ff942 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -785,8 +785,12 @@ void Profiler::recordExternalSample(u64 weight, int tid, int num_frames, ASGCT_CallFrame *frames, bool truncated, jint event_type, Event *event) { // This is a non-signal based sampler - ProfiledThread::initCurrentThreadSignalSafe(); - + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + if (current == nullptr) { + Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL); + return; + } + // Protect external sampling operations to prevent signal handler interference CriticalSection cs; atomicIncRelaxed(_total_samples);