From da986728d06fdfeb2620282252d9d6d2662111fb Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Wed, 19 Aug 2026 15:25:51 +0200 Subject: [PATCH 1/9] feat: squash commit for PR 664 rebase --- .../native/config/ConfigurationPresets.kt | 15 + ddprof-lib/src/main/cpp/javaApi.cpp | 89 ++-- ddprof-lib/src/main/cpp/jvmSupport.cpp | 16 +- ddprof-lib/src/main/cpp/profiler.cpp | 31 +- ddprof-lib/src/main/cpp/profiler.h | 5 + ddprof-lib/src/main/cpp/threadLocalData.h | 96 +++- ddprof-lib/src/main/cpp/vmEntry.cpp | 231 +++++++- ddprof-lib/src/main/cpp/vmEntry.h | 25 +- .../com/datadoghq/profiler/JavaProfiler.java | 62 ++- ddprof-lib/src/test/cpp/park_state_ut.cpp | 73 +++ .../src/test/cpp/taskBlockRecorder_ut.cpp | 101 ++++ ddprof-lib/src/test/cpp/vmEntry_ut.cpp | 500 ++++++++++++++++++ .../datadoghq/profiler/ExternalLauncher.java | 136 +++++ .../profiler/JavaProfilerApiSurfaceTest.java | 10 + .../datadoghq/profiler/JavaProfilerTest.java | 197 ++++++- .../JvmtiBasedMonitorTaskBlockTest.java | 30 ++ .../JvmtiBasedParkTaskBlockTest.java | 30 ++ .../wallclock/MonitorTaskBlockTest.java | 240 +++++++++ .../profiler/wallclock/ParkTaskBlockTest.java | 169 ++++++ .../wallclock/TaskBlockAssertions.java | 9 + 20 files changed, 1998 insertions(+), 67 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/vmEntry_ut.cpp create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedParkTaskBlockTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt index ea4a9c7c62..9201177860 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt @@ -1,3 +1,18 @@ +/* + * 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. + */ package com.datadoghq.native.config diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 25b74bb48b..7911c0caac 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -70,7 +70,8 @@ class JniString { }; extern "C" DLLEXPORT jboolean JNICALL -Java_com_datadoghq_profiler_JavaProfiler_init0(JNIEnv *env, jclass unused) { +Java_com_datadoghq_profiler_JavaProfiler_init0( + JNIEnv *env, jclass unused, jboolean delegateMonitorWaitEvents) { Error error = Profiler::instance()->init(); if (error) { throwNew(env, "java/lang/IllegalStateException", error.message()); @@ -79,13 +80,22 @@ 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::initCurrentThreadSignalSafe(); - return JNI_TRUE; - } else { + ProfilerBridgeInitResult result = + VM::initProfilerBridge(nullptr, true, delegateMonitorWaitEvents); + if (result == ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT) { + throwNew(env, "java/lang/IllegalStateException", + "Monitor-event ownership conflicts with the profiler's " + "process-wide initialization"); return JNI_FALSE; } + if (result != ProfilerBridgeInitResult::SUCCESS) { + throwNew(env, "java/lang/IllegalStateException", + "Failed to initialize the profiler bridge"); + return JNI_FALSE; + } + // Attach ProfiledThread + ProfiledThread::initCurrentThreadSignalSafe(); + return JNI_TRUE; } extern "C" DLLEXPORT void JNICALL @@ -110,6 +120,12 @@ Java_com_datadoghq_profiler_JavaProfiler_getTid0(JNIEnv *env, jclass unused) { return OS::threadId(); } +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_monitorWaitEventsDelegated0( + JNIEnv *env, jclass unused) { + return VM::monitorWaitEventsDelegated(); +} + extern "C" DLLEXPORT jstring JNICALL Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused, jstring command) { @@ -428,15 +444,24 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0( } extern "C" DLLEXPORT jboolean JNICALL -Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(JNIEnv *env, jclass unused) { +Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + JNIEnv *env, jclass unused, jthread thread) { + if (!JVMSupport::isPlatformThread(env, thread)) { + return JNI_FALSE; + } ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); if (current == nullptr) { return JNI_FALSE; } + Context context = ContextApi::snapshot(); + if (!current->parkEnter(TSC::ticks(), context)) { + return JNI_FALSE; + } - bool first_park = current->parkEnter(); - ThreadFilter *tf = Profiler::instance()->threadFilter(); - if (first_park && tf->registryActive()) { + Profiler *profiler = Profiler::instance(); + ThreadFilter *tf = profiler->threadFilter(); + if (context.spanId == 0 && tf->registryActive() && + (profiler->taskBlockEnabled() || tf->enabled())) { ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); if (slot_id >= 0) { WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); @@ -444,31 +469,32 @@ Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(JNIEnv *env, jclass unused) tracker->enterBlockedRun(tf, slot_id, OSThreadState::CONDVAR_WAIT)); } } - return first_park ? JNI_TRUE : JNI_FALSE; + return JNI_TRUE; } extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkExit0( - JNIEnv *env, jclass unused, jlong blocker, jlong unblockingSpanId) { + JNIEnv *env, jclass unused, jthread thread, jlong blocker, + jlong unblockingSpanId) { + if (!JVMSupport::isPlatformThread(env, thread)) { + return; + } ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); if (current == nullptr) { return; } - + u64 start_ticks = 0; u64 park_block_token = 0; - if (!current->parkExit(park_block_token) || park_block_token == 0) { + Context context{}; + if (!current->parkExit(start_ticks, context, park_block_token) || + park_block_token == 0) { return; } - ThreadFilter *tf = Profiler::instance()->threadFilter(); - if (tf->registryActive()) { - ThreadFilter::SlotID slot_id = WallClockBlockTracker::tokenSlotId(park_block_token); - ThreadFilter::SlotID cached_id = current->filterSlotId(); - if (cached_id == slot_id && - tf->activeSlotForId(cached_id, current->tid()) != nullptr) { - WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); - tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(park_block_token)); - } - } + finishTaskBlockAtExit(current, Profiler::instance()->threadFilter(), + Profiler::instance()->blockTracker(), thread, + 1, park_block_token, start_ticks, context, + static_cast(blocker), + static_cast(unblockingSpanId)); } static bool decodeJavaBlockState(jint state, OSThreadState &decoded) { @@ -496,13 +522,14 @@ static bool isCurrentJniThread(JNIEnv* env, jthread thread) { extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( - JNIEnv *env, jclass unused, jint state) { - ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) { + JNIEnv *env, jclass unused, jthread thread, jint state) { + OSThreadState decoded; + if (!decodeJavaBlockState(state, decoded) || + !JVMSupport::isPlatformThread(env, thread)) { return 0; } - OSThreadState decoded; - if (!decodeJavaBlockState(state, decoded)) { + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + if (current == nullptr) { return 0; } u64 span_id = 0, root_span_id = 0; @@ -525,9 +552,9 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0( - JNIEnv *env, jclass unused, jlong token) { + JNIEnv *env, jclass unused, jthread thread, jlong token) { u64 block_token = static_cast(token); - if (block_token == 0) { + if (block_token == 0 || !JVMSupport::isPlatformThread(env, thread)) { return; } diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 7e7e38fbca..26addfe36e 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -18,6 +18,8 @@ #include +#include + using JniFunction = void (JNICALL*)(); using IsVirtualThreadFunction = jboolean (JNICALL*)(JNIEnv*, jobject); @@ -44,11 +46,21 @@ bool JVMSupport::isPlatformThread(JNIEnv* jni, jthread thread) { const JniFunction* functions = reinterpret_cast(jni->functions); + if (functions == nullptr) return false; IsVirtualThreadFunction is_virtual_thread = reinterpret_cast( functions[IS_VIRTUAL_THREAD_INDEX]); - return is_virtual_thread != nullptr && - is_virtual_thread(jni, thread) == JNI_FALSE; + if (is_virtual_thread == nullptr) { + static std::atomic warning_emitted{false}; + bool expected = false; + if (warning_emitted.compare_exchange_strong(expected, true, + std::memory_order_relaxed)) { + LOG_WARN("JNI version 19 or later does not expose IsVirtualThread; " + "JVM producer callbacks will be ignored"); + } + return false; + } + return is_virtual_thread(jni, thread) == JNI_FALSE; } bool JVMSupport::initialize() { diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index a38862a9ee..2cf2e28a81 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1623,6 +1623,26 @@ Error Profiler::init() { return Error::OK; } +void Profiler::setTaskBlockEnabled(bool enabled) { + if (enabled) { + // Keep callback admission closed until native setup has either completed + // or rolled back, so partial event enablement cannot create paired state. + bool monitor_events_enabled = + VM::nativeMonitorEventsAvailable() && + VM::setNativeMonitorEventsEnabled(true); + _task_block_monitor_events_enabled.store(monitor_events_enabled, + std::memory_order_release); + _task_block_enabled.store(true, std::memory_order_release); + return; + } + + _task_block_enabled.store(false, std::memory_order_release); + if (_task_block_monitor_events_enabled.exchange( + false, std::memory_order_acq_rel)) { + VM::setNativeMonitorEventsEnabled(false); + } +} + Error Profiler::start(Arguments &args, bool reset) { MutexLocker ml(_state_lock); Error error = checkState(); @@ -1987,9 +2007,8 @@ Error Profiler::start(Arguments &args, bool reset) { // Paired with drainInflight() on the stop side. _cpu_engine->enableEvents(true); - _task_block_enabled.store( - (activated & EM_WALL) && args._wall_precheck && track_unfiltered_wall, - std::memory_order_release); + setTaskBlockEnabled( + (activated & EM_WALL) && args._wall_precheck && track_unfiltered_wall); _state.store(RUNNING, std::memory_order_release); _start_time = time(NULL); __atomic_add_fetch(&_epoch, 1, __ATOMIC_RELAXED); @@ -2014,7 +2033,7 @@ Error Profiler::stop() { if (state() != RUNNING) { return Error("Profiler is not active"); } - _task_block_enabled.store(false, std::memory_order_release); + setTaskBlockEnabled(false); // Order matters: disable engines first so the _enabled check inside signal // handlers will fail for any new signal delivered from now on. drain() then @@ -2289,7 +2308,9 @@ Error Profiler::dump(const char *path, const int length) { // rotateDictsAndRun rotates the dictionaries, takes lockAll() around the // dump (fences ASGCT/JNI writers to CallTraceStorage), then clearStandby()s // the rotated buffers. StringDictionary's RefCountGuard protocol handles - // its own writer/reader coordination. + // its own writer/reader coordination; #527's classMapSharedGuard readers + // (deferred vtable receiver resolution) are coordinated through + // _class_map_lock. if (beginTaskBlockRotation()) { rotateDictsAndRun([&]{ err = _jfr.dump(path, length); diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index f433032724..4e46c8c54d 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -148,6 +148,7 @@ class alignas(alignof(SpinLock)) Profiler { // onThreadStart()/onThreadEnd() on JVMTI thread-lifecycle callbacks. std::atomic _reference_chains_active{false}; std::atomic _task_block_enabled{false}; + std::atomic _task_block_monitor_events_enabled{false}; std::atomic _task_block_rotation{false}; std::atomic _task_block_inflight{0}; @@ -213,6 +214,7 @@ class alignas(alignof(SpinLock)) Profiler { bool recordSampleImpl(void *ucontext, u64 weight, int tid, jint event_type, u64 call_trace_id, Event *event, WallClockBlockTracker::BlockState *unowned_slot); + void setTaskBlockEnabled(bool enabled); bool beginTaskBlockRotation(); void endTaskBlockRotation(); @@ -552,6 +554,9 @@ class alignas(alignof(SpinLock)) Profiler { bool taskBlockEnabled() const { return _task_block_enabled.load(std::memory_order_acquire); } + bool nativeMonitorTaskBlockEnabled() const { + return _task_block_monitor_events_enabled.load(std::memory_order_acquire); + } void writeLog(LogLevel level, const char *message); void writeLog(LogLevel level, const char *message, size_t len); void writeDatadogProfilerSetting(int tid, int length, const char *name, diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 66fb0450cb..efe6861ebd 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -57,7 +57,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 + static constexpr u32 FLAG_CLAIMED = 0x8u; // Used by ThreadLocalDataPool only + static constexpr u32 FLAG_MONITOR_BLOCKED = 0x10u; // We are allowing several levels of nesting because we can be // eg. in a crash handler when wallclock signal kicks in, @@ -87,10 +88,17 @@ class ProfiledThread : public ThreadLocalData { u64 _call_trace_id; u32 _recording_epoch; volatile u32 _misc_flags; + u64 _park_start_ticks; u64 _park_block_token; + Context _park_context; u64 _task_block_start_ticks; u64 _task_block_token; Context _task_block_context; + u64 _monitor_start_ticks; + Context _monitor_context; + u64 _monitor_blocker; + u64 _monitor_block_token; + OSThreadState _monitor_block_state; int _filter_slot_id; // Slot ID for thread filtering uint8_t _init_window; // Countdown for JVM thread init race window (PROF-13072) volatile int _signal_depth; // Nested signal-handler depth (see SignalHandlerScope) @@ -113,8 +121,11 @@ class ProfiledThread : public ThreadLocalData { 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(0), - _park_block_token(0), _task_block_start_ticks(0), - _task_block_token(0), _task_block_context{}, _filter_slot_id(-1), + _park_start_ticks(0), _park_block_token(0), _park_context{}, + _task_block_start_ticks(0), _task_block_token(0), _task_block_context{}, + _monitor_start_ticks(0), _monitor_context{}, _monitor_blocker(0), + _monitor_block_token(0), _monitor_block_state(OSThreadState::UNKNOWN), + _filter_slot_id(-1), _init_window(0), _signal_depth(0), _otel_ctx_initialized(false), @@ -418,11 +429,24 @@ class ProfiledThread : public ThreadLocalData { _otel_local_root_span_id = 0; } - inline bool parkEnter() { - u32 prev = __atomic_fetch_or(&_misc_flags, FLAG_PARKED, __ATOMIC_RELEASE); - return (prev & FLAG_PARKED) == 0; + inline bool parkEnter(u64 start_ticks, const Context& context) { + u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); + while ((flags & FLAG_PARKED) == 0) { + _park_start_ticks = start_ticks; + _park_context = context; + if (__atomic_compare_exchange_n(&_misc_flags, &flags, + flags | FLAG_PARKED, true, + __ATOMIC_RELEASE, __ATOMIC_ACQUIRE)) { + return true; + } + } + return false; } +#ifdef UNIT_TEST + inline bool parkEnter() { return parkEnter(0, Context{}); } +#endif + inline void setParkBlockToken(u64 token) { _park_block_token = token; } @@ -445,16 +469,74 @@ class ProfiledThread : public ThreadLocalData { } // Returns false if the thread was not parked (idempotent). - inline bool parkExit(u64 &park_block_token) { + inline bool parkExit(u64& start_ticks, Context& context, + u64& park_block_token) { u32 prev = __atomic_fetch_and(&_misc_flags, ~FLAG_PARKED, __ATOMIC_ACQ_REL); if ((prev & FLAG_PARKED) == 0) { return false; } + start_ticks = _park_start_ticks; + context = _park_context; park_block_token = _park_block_token; _park_block_token = 0; return true; } +#ifdef UNIT_TEST + inline bool parkExit(u64& park_block_token) { + u64 start_ticks = 0; + Context context{}; + return parkExit(start_ticks, context, park_block_token); + } +#endif + + // Object.wait owns its interval until MonitorWaited, including monitor + // reacquisition. A nested contention callback must not overwrite that state. + inline bool monitorEnter(u64 start_ticks, const Context& context, u64 blocker, + OSThreadState state) { + u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); + if ((flags & FLAG_MONITOR_BLOCKED) != 0) return false; + _monitor_start_ticks = start_ticks; + _monitor_context = context; + _monitor_blocker = blocker; + _monitor_block_token = 0; + _monitor_block_state = state; + __atomic_fetch_or(&_misc_flags, FLAG_MONITOR_BLOCKED, __ATOMIC_RELEASE); + return true; + } + + inline void setMonitorBlockToken(u64 token) { + _monitor_block_token = token; + } + + inline u64 monitorBlockToken() const { return _monitor_block_token; } + + inline void clearMonitorBlock() { + __atomic_fetch_and(&_misc_flags, ~FLAG_MONITOR_BLOCKED, __ATOMIC_ACQ_REL); + _monitor_block_token = 0; + _monitor_block_state = OSThreadState::UNKNOWN; + } + + inline bool monitorExit(OSThreadState expected_state, u64& start_ticks, + Context& context, u64& blocker, + u64& monitor_block_token) { + u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); + if ((flags & FLAG_MONITOR_BLOCKED) == 0 || + _monitor_block_state != expected_state) { + return false; + } + u32 prev = __atomic_fetch_and(&_misc_flags, ~FLAG_MONITOR_BLOCKED, + __ATOMIC_ACQ_REL); + if ((prev & FLAG_MONITOR_BLOCKED) == 0) return false; + start_ticks = _monitor_start_ticks; + context = _monitor_context; + blocker = _monitor_blocker; + monitor_block_token = _monitor_block_token; + _monitor_block_token = 0; + _monitor_block_state = OSThreadState::UNKNOWN; + return true; + } + Context snapshotContext(size_t numAttrs); private: diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index acf8382322..4b481cde50 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -8,6 +8,7 @@ #include "vmEntry.h" #include "arguments.h" #include "context.h" +#include "context_api.h" #include "counters.h" #include "j9/j9Support.h" #include "jniHelper.h" @@ -15,12 +16,15 @@ #include "jvmThread.h" #include "libraries.h" #include "log.h" +#include "mutex.h" #include "os.h" #include "profiler.h" #include "referenceChains.h" #include "safeAccess.h" #include "samplerPerf.h" #include "threadLocalData.h" +#include "taskBlockRecorder.h" +#include "tsc.h" // Pulls in vmStructs.h plus the definitions of crashProtectionActive()/cast_to() that its inline // accessors odr-use here; the light vmStructs.h alone leaves those unresolved in assertion-enabled // builds (see the note in hotspotStackFrame_aarch64.cpp). @@ -50,8 +54,16 @@ bool VM::_hotspot = false; bool VM::_zing = false; bool VM::_can_sample_objects = false; bool VM::_can_intercept_binding = false; +bool VM::_monitor_wait_events_delegated = false; +bool VM::_native_monitor_events_available = false; +bool VM::_profiler_bridge_initialized = false; bool VM::_is_adaptive_gc_boundary_flag_set = false; +// Serializes the one-time bridge installation and ownership negotiation. +// Callback readers need no synchronization because ownership is assigned +// before callbacks can be enabled and is never changed afterward. +static Mutex profiler_bridge_init_lock; + jvmtiExtensionFunction VM::_request_stack_trace = nullptr; jvmtiExtensionFunction VM::_init_request_stack_trace = nullptr; @@ -69,6 +81,118 @@ static void wakeupHandler(int signo) { // Dummy handler for interrupting syscalls } +static u64 monitorBlockerHash(jvmtiEnv *jvmti, jobject object) { + if (object == NULL) return 0; + jint hash = 0; + if (jvmti->GetObjectHashCode(object, &hash) != JVMTI_ERROR_NONE) return 0; + return static_cast(static_cast(hash)); +} + +static void monitorBlockEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, OSThreadState state) { + Profiler *profiler = Profiler::instance(); + if (!profiler->taskBlockEnabled() || + !profiler->nativeMonitorTaskBlockEnabled() || + !JVMSupport::isPlatformThread(jni, thread)) { + return; + } + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + if (current == nullptr) return; + Context context = ContextApi::snapshot(); + if (context.spanId != 0) { + Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); + return; + } + + if (!current->monitorEnter(TSC::ticks(), context, + monitorBlockerHash(jvmti, object), state)) { + u64 token = current->monitorBlockToken(); + ThreadFilter *tf = profiler->threadFilter(); + bool current_owner = false; + if (token != 0) { + ThreadFilter::SlotID slot_id = ThreadFilter::tokenSlotId(token); + ThreadFilter::Slot *slot = current->filterSlotId() == slot_id + ? tf->activeSlotForId(slot_id, current->tid()) + : nullptr; + if (slot != nullptr) { + BlockRunSnapshot snapshot = slot->snapshotBlockRun(); + current_owner = snapshot.active && + snapshot.owner == BlockRunOwner::JVMTI && + snapshot.generation == ThreadFilter::tokenGeneration(token); + } + } + if (current_owner) { + return; + } + current->clearMonitorBlock(); + if (!current->monitorEnter(TSC::ticks(), context, + monitorBlockerHash(jvmti, object), state)) { + return; + } + } + + ThreadFilter *tf = profiler->threadFilter(); + ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); + if (!tf->unfilteredWallTrackingActive() || slot_id < 0) { + current->clearMonitorBlock(); + return; + } + u64 token = + tf->enterBlockedRun(slot_id, state, BlockRunOwner::JVMTI); + if (token == 0) { + ThreadFilter::Slot *slot = tf->slotForId(slot_id); + if (slot != nullptr && slot->inContextWindow()) { + Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); + } + current->clearMonitorBlock(); + return; + } + current->setMonitorBlockToken(token); +} + +static void monitorBlockExit(JNIEnv *jni, jthread thread, OSThreadState state) { + if (!JVMSupport::isPlatformThread(jni, thread)) return; + ProfiledThread *current = ProfiledThread::current(); + if (current == nullptr) return; + + u64 start_ticks = 0; + Context context{}; + u64 blocker = 0; + u64 token = 0; + if (!current->monitorExit(state, start_ticks, context, blocker, token) || + token == 0) { + return; + } + + Profiler *profiler = Profiler::instance(); + finishTaskBlockAtExit(current, profiler->threadFilter(), thread, 0, token, + start_ticks, context, blocker, 0); +} + +static void JNICALL MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *jni, + jthread thread, jobject object) { + monitorBlockEnter(jvmti, jni, thread, object, OSThreadState::MONITOR_WAIT); +} + +static void JNICALL MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *jni, + jthread thread, jobject object) { + monitorBlockExit(jni, thread, OSThreadState::MONITOR_WAIT); +} + +static void JNICALL MonitorWait(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jlong timeout) { + if (!VM::monitorWaitEventsDelegated()) { + monitorBlockEnter(jvmti, jni, thread, object, OSThreadState::OBJECT_WAIT); + } +} + +static void JNICALL MonitorWaited(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jboolean timed_out) { + if (!VM::monitorWaitEventsDelegated()) { + monitorBlockExit(jni, thread, OSThreadState::OBJECT_WAIT); + } +} + static bool isVmRuntimeEntry(const char* blob_name) { return strcmp(blob_name, "_ZNK12MemAllocator8allocateEv") == 0 || strncmp(blob_name, "_Z22post_allocation_notify", 26) == 0 @@ -387,6 +511,11 @@ bool VM::initShared(JavaVM* vm) { } bool VM::initLibrary(JavaVM *vm) { + MutexLocker init_locker(profiler_bridge_init_lock); + if (_profiler_bridge_initialized) { + return true; + } + TEST_LOG("VM::initLibrary"); if (!initShared(vm)) { return false; @@ -456,15 +585,31 @@ void JNICALL ForwardedGarbageCollectionFinish(jvmtiEnv *jvmti_env) { ReferenceChainTracker::GarbageCollectionFinish(jvmti_env); } -bool VM::initProfilerBridge(JavaVM *vm, bool attach) { +void VM::configureMonitorEvents(bool delegateMonitorWaitEvents) { + jvmtiCapabilities actual_capabilities = {0}; + _jvmti->GetCapabilities(&actual_capabilities); + _native_monitor_events_available = + actual_capabilities.can_generate_monitor_events; + _monitor_wait_events_delegated = delegateMonitorWaitEvents; +} + +ProfilerBridgeInitResult VM::initProfilerBridge(JavaVM *vm, bool attach, + bool delegateMonitorWaitEvents) { + MutexLocker init_locker(profiler_bridge_init_lock); + if (_profiler_bridge_initialized) { + return delegateMonitorWaitEvents == _monitor_wait_events_delegated + ? ProfilerBridgeInitResult::SUCCESS + : ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT; + } + TEST_LOG("VM::initProfilerBridge"); if (!initShared(vm)) { - return false; + return ProfilerBridgeInitResult::FAILURE; } CodeCache *lib = openJvmLibrary(); if (lib == nullptr) { - return false; + return ProfilerBridgeInitResult::FAILURE; } // Under Agent_OnLoad (attach == false), this is the first native entry point and @@ -495,6 +640,8 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { _can_intercept_binding = potential_capabilities.can_generate_native_method_bind_events && HeapUsage::needsNativeBindingInterception(); + bool can_add_monitor_events = + potential_capabilities.can_generate_monitor_events; jvmtiCapabilities capabilities = {0}; capabilities.can_generate_all_class_hook_events = 1; @@ -511,11 +658,13 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { capabilities.can_get_source_file_name = 1; capabilities.can_get_line_numbers = 1; capabilities.can_generate_compiled_method_load_events = 1; - capabilities.can_generate_monitor_events = 1; + capabilities.can_generate_monitor_events = can_add_monitor_events ? 1 : 0; capabilities.can_tag_objects = 1; _jvmti->AddCapabilities(&capabilities); + configureMonitorEvents(delegateMonitorWaitEvents); + if (_hotspot) { probeJFRRequestStackTrace(); } @@ -533,6 +682,12 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { callbacks.GarbageCollectionStart = ReferenceChainTracker::GarbageCollectionStart; callbacks.GarbageCollectionFinish = ForwardedGarbageCollectionFinish; callbacks.NativeMethodBind = VMStructs::NativeMethodBind; + if (_native_monitor_events_available) { + callbacks.MonitorContendedEnter = MonitorContendedEnter; + callbacks.MonitorContendedEntered = MonitorContendedEntered; + callbacks.MonitorWait = MonitorWait; + callbacks.MonitorWaited = MonitorWaited; + } _jvmti->SetEventCallbacks(&callbacks, sizeof(callbacks)); _jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_VM_DEATH, NULL); @@ -585,7 +740,70 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { OS::installSignalHandler(WAKEUP_SIGNAL, NULL, wakeupHandler); - return true; + _profiler_bridge_initialized = true; + return ProfilerBridgeInitResult::SUCCESS; +} + +bool VM::setNativeMonitorEventsEnabled(bool enabled) { + if (!_native_monitor_events_available) return false; + + jvmtiError enter = JVMTI_ERROR_NONE; + jvmtiError entered = JVMTI_ERROR_NONE; + jvmtiError wait = JVMTI_ERROR_NONE; + jvmtiError waited = JVMTI_ERROR_NONE; + + if (enabled) { + // JVMTI enables each event independently and does not queue events that + // occur while disabled. Install every terminal notification before its + // entry notification so an admitted interval always has an exit path. + entered = _jvmti->SetEventNotificationMode( + JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, NULL); + if (entered != JVMTI_ERROR_NONE) goto enable_failed; + + if (!_monitor_wait_events_delegated) { + waited = _jvmti->SetEventNotificationMode( + JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAITED, NULL); + if (waited != JVMTI_ERROR_NONE) goto enable_failed; + } + + enter = _jvmti->SetEventNotificationMode( + JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER, NULL); + if (enter != JVMTI_ERROR_NONE) goto enable_failed; + + if (!_monitor_wait_events_delegated) { + wait = _jvmti->SetEventNotificationMode( + JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT, NULL); + if (wait != JVMTI_ERROR_NONE) goto enable_failed; + } + return true; + +enable_failed: + Log::warn("Unable to enable JVMTI monitor events: %d/%d/%d/%d", + enter, entered, wait, waited); + setNativeMonitorEventsEnabled(false); + return false; + } + + // Stop admitting new intervals before removing the terminal notifications. + // Disable all four events even when Object.wait is delegated so teardown + // also cleans up modes established before ownership was configured. + enter = _jvmti->SetEventNotificationMode( + JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER, NULL); + wait = _jvmti->SetEventNotificationMode( + JVMTI_DISABLE, JVMTI_EVENT_MONITOR_WAIT, NULL); + entered = _jvmti->SetEventNotificationMode( + JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, NULL); + waited = _jvmti->SetEventNotificationMode( + JVMTI_DISABLE, JVMTI_EVENT_MONITOR_WAITED, NULL); + + if (enter == JVMTI_ERROR_NONE && entered == JVMTI_ERROR_NONE && + wait == JVMTI_ERROR_NONE && waited == JVMTI_ERROR_NONE) { + return true; + } + + Log::warn("Unable to disable JVMTI monitor events: %d/%d/%d/%d", + enter, entered, wait, waited); + return false; } // Run late initialization when JVM is ready. May be called more than once (from @@ -722,7 +940,8 @@ Agent_OnLoad(JavaVM* vm, char* options, void* reserved) { return ARGUMENTS_ERROR; } - if (!VM::initProfilerBridge(vm, false)) { + if (VM::initProfilerBridge(vm, false) != + ProfilerBridgeInitResult::SUCCESS) { Log::error("JVM does not support Tool Interface"); return COMMAND_ERROR; } diff --git a/ddprof-lib/src/main/cpp/vmEntry.h b/ddprof-lib/src/main/cpp/vmEntry.h index 75725ef151..35268a62af 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.h +++ b/ddprof-lib/src/main/cpp/vmEntry.h @@ -132,6 +132,15 @@ class JavaVersionAccess { static int get_hotspot_version(char* prop_value); }; +// The profiler bridge is process-wide and initialized exactly once. Later Java +// API initialization may reuse it only with the same requested Object.wait +// ownership, independently of native monitor-event availability. +enum class ProfilerBridgeInitResult { + SUCCESS, + FAILURE, + MONITOR_EVENTS_DELEGATION_CONFLICT, +}; + class VM { friend class VMTestAccessor; @@ -147,6 +156,9 @@ class VM { static bool _zing; static bool _can_sample_objects; static bool _can_intercept_binding; + static bool _monitor_wait_events_delegated; + static bool _native_monitor_events_available; + static bool _profiler_bridge_initialized; static bool _is_adaptive_gc_boundary_flag_set; static CodeCache *_libjvm; @@ -168,6 +180,7 @@ class VM { static void *getLibraryHandle(const char *name); static bool initShared(JavaVM *vm); + static void configureMonitorEvents(bool delegateMonitorWaitEvents); static void probeJFRRequestStackTrace(); static CodeCache* openJvmLibrary(); @@ -183,7 +196,8 @@ class VM { static JVM_GetManagement _getManagement; static bool initLibrary(JavaVM *vm); - static bool initProfilerBridge(JavaVM *vm, bool attach); + static ProfilerBridgeInitResult initProfilerBridge( + JavaVM *vm, bool attach, bool delegateMonitorWaitEvents = false); static jvmtiEnv *jvmti() { return _jvmti; } @@ -218,6 +232,15 @@ class VM { static bool canSampleObjects() { return _can_sample_objects; } + static bool monitorWaitEventsDelegated() { + return _monitor_wait_events_delegated; + } + + static bool nativeMonitorEventsAvailable() { + return _native_monitor_events_available; + } + static bool setNativeMonitorEventsEnabled(bool enabled); + static bool isZing() { return _zing; } static bool isUseAdaptiveGCBoundarySet() { diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index 7706b82ea2..9d21afeb96 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -104,7 +104,35 @@ public static JavaProfiler getInstance(String scratchDir) throws IOException { * @param scratchDir directory where the bundled library will be exploded before linking; ignored when 'libLocation' is {@literal null} */ public static synchronized JavaProfiler getInstance(String libLocation, String scratchDir) throws IOException { + return getInstance(libLocation, scratchDir, false); + } + + /** + * Get a {@linkplain JavaProfiler} instance with explicit monitor-event ownership. + * + *

The first successful native bridge initialization fixes this process-wide setting because + * the native profiler is a singleton. This may occur during {@code -agentpath} startup before + * this method is called. When delegation is enabled, Java instrumentation owns + * {@code Object.wait} TaskBlock intervals and native JVMTI wait callbacks are suppressed; + * native JVMTI callbacks continue to own synchronized monitor contention. Ownership is + * preserved independently of whether the JVM provides native monitor-event capability. + * + * @param libLocation the path to the native library to use, or {@literal null} for the bundled library + * @param scratchDir directory where the bundled library will be exploded before linking + * @param delegateMonitorWaitEvents whether Java instrumentation owns {@code Object.wait} intervals + * @return the process-wide profiler instance + * @throws IOException if the native library cannot be loaded + * @throws IllegalStateException if monitor ownership conflicts with an earlier native bridge + * initialization + */ + public static synchronized JavaProfiler getInstance(String libLocation, String scratchDir, + boolean delegateMonitorWaitEvents) throws IOException { if (instance != null) { + if (monitorWaitEventsDelegated0() != delegateMonitorWaitEvents) { + throw new IllegalStateException( + "Monitor-event ownership conflicts with the profiler's " + + "process-wide initialization"); + } return instance; } @@ -113,12 +141,11 @@ public static synchronized JavaProfiler getInstance(String libLocation, String s if (!result.succeeded) { throw new IOException("Failed to load Datadog Java profiler library", result.error); } - if (isVirtualThread(Thread.currentThread())) { throw new IOException("Cannot initialize profiler on a virtual thread"); } - init0(); + init0(delegateMonitorWaitEvents); instance = profiler; @@ -134,6 +161,18 @@ public static synchronized JavaProfiler getInstance(String libLocation, String s return profiler; } + /** + * Reports whether Java instrumentation owns {@code Object.wait} TaskBlock intervals instead + * of native JVMTI {@code MonitorWait} and {@code MonitorWaited} callbacks. Synchronized-monitor + * contention remains owned by native JVMTI callbacks. This reports the process-wide ownership + * selected during bridge initialization, independently of native monitor-event capability. + * + * @return {@code true} when {@code Object.wait} handling is delegated to Java instrumentation + */ + public boolean isMonitorWaitEventsDelegated() { + return monitorWaitEventsDelegated0(); + } + /** * Stop profiling (without dumping results) * @@ -400,7 +439,7 @@ public void recordQueueTime(long startTicks, * @return {@code true} when this call owns a park interval that must be closed */ boolean parkEnter() { - return parkEnter0(); + return parkEnter0(Thread.currentThread()); } /** @@ -408,7 +447,7 @@ boolean parkEnter() { * {@code blocker} and {@code unblockingSpanId} are reserved for park instrumentation. */ void parkExit(long blocker, long unblockingSpanId) { - parkExit0(blocker, unblockingSpanId); + parkExit0(Thread.currentThread(), blocker, unblockingSpanId); } /** @@ -420,14 +459,14 @@ void parkExit(long blocker, long unblockingSpanId) { * @return an opaque token to pass to {@link #blockExit(long)}, or 0 if no state was armed */ long blockEnter(int state) { - return blockEnter0(state); + return blockEnter0(Thread.currentThread(), state); } /** * Clears a blocked interval previously armed by {@link #blockEnter(int)}. */ void blockExit(long token) { - blockExit0(token); + blockExit0(Thread.currentThread(), token); } /** @@ -499,7 +538,7 @@ public Map getDebugCounters() { return counters; } - private static native boolean init0(); + private static native boolean init0(boolean delegateMonitorWaitEvents); private native void stop0() throws IllegalStateException; private native String execute0(String command) throws IllegalArgumentException, IllegalStateException, IOException; @@ -507,6 +546,7 @@ public Map getDebugCounters() { private static native void filterThreadRemove0(); private static native int getTid0(); + private static native boolean monitorWaitEventsDelegated0(); /** * Test seam: returns the calling thread's profiler tid @@ -532,13 +572,13 @@ static int getTid() { private static native void recordQueueEnd0(long startTicks, long endTicks, String task, String scheduler, Thread origin, String queueType, int queueLength); - private static native boolean parkEnter0(); + private static native boolean parkEnter0(Thread thread); - private static native void parkExit0(long blocker, long unblockingSpanId); + private static native void parkExit0(Thread thread, long blocker, long unblockingSpanId); - private static native long blockEnter0(int state); + private static native long blockEnter0(Thread thread, int state); - private static native void blockExit0(long token); + private static native void blockExit0(Thread thread, long token); private static native long beginTaskBlock0(Thread thread); diff --git a/ddprof-lib/src/test/cpp/park_state_ut.cpp b/ddprof-lib/src/test/cpp/park_state_ut.cpp index cb08cbbbf8..c3ba651e24 100644 --- a/ddprof-lib/src/test/cpp/park_state_ut.cpp +++ b/ddprof-lib/src/test/cpp/park_state_ut.cpp @@ -139,6 +139,79 @@ TEST(ProfiledThreadParkStateTest, ParkExitReturnsZeroTokenWhenBlockRunWasNotArme EXPECT_EQ(0ULL, park_block_token); } +TEST(ProfiledThreadParkStateTest, ParkExitReturnsEntrySnapshot) { + TestProfiledThread thread = testThread(12351); + Context entered{}; + entered.spanId = 17; + entered.rootSpanId = 18; + ASSERT_TRUE(thread->parkEnter(123, entered)); + thread->setParkBlockToken(456); + + u64 start_ticks = 0; + u64 token = 0; + Context exited{}; + ASSERT_TRUE(thread->parkExit(start_ticks, exited, token)); + EXPECT_EQ(123ULL, start_ticks); + EXPECT_EQ(456ULL, token); + EXPECT_EQ(17ULL, exited.spanId); + EXPECT_EQ(18ULL, exited.rootSpanId); +} + +TEST(ProfiledThreadMonitorStateTest, MatchingExitReturnsEntrySnapshot) { + TestProfiledThread thread = testThread(12352); + Context entered{}; + entered.spanId = 21; + ASSERT_TRUE(thread->monitorEnter( + 100, entered, 200, OSThreadState::MONITOR_WAIT)); + thread->setMonitorBlockToken(300); + + u64 start_ticks = 0; + u64 blocker = 0; + u64 token = 0; + Context exited{}; + ASSERT_TRUE(thread->monitorExit(OSThreadState::MONITOR_WAIT, start_ticks, + exited, blocker, token)); + EXPECT_EQ(100ULL, start_ticks); + EXPECT_EQ(200ULL, blocker); + EXPECT_EQ(300ULL, token); + EXPECT_EQ(21ULL, exited.spanId); +} + +TEST(ProfiledThreadMonitorStateTest, NestedContentionDoesNotReplaceObjectWait) { + TestProfiledThread thread = testThread(12353); + Context context{}; + ASSERT_TRUE(thread->monitorEnter( + 100, context, 200, OSThreadState::OBJECT_WAIT)); + thread->setMonitorBlockToken(300); + EXPECT_FALSE(thread->monitorEnter( + 400, context, 500, OSThreadState::MONITOR_WAIT)); + + u64 start_ticks = 0; + u64 blocker = 0; + u64 token = 0; + Context exited{}; + EXPECT_FALSE(thread->monitorExit(OSThreadState::MONITOR_WAIT, start_ticks, + exited, blocker, token)); + ASSERT_TRUE(thread->monitorExit(OSThreadState::OBJECT_WAIT, start_ticks, + exited, blocker, token)); + EXPECT_EQ(100ULL, start_ticks); + EXPECT_EQ(200ULL, blocker); + EXPECT_EQ(300ULL, token); +} + +TEST(ProfiledThreadMonitorStateTest, ClearAllowsRecoveryFromStaleState) { + TestProfiledThread thread = testThread(12354); + Context context{}; + ASSERT_TRUE(thread->monitorEnter( + 100, context, 200, OSThreadState::OBJECT_WAIT)); + thread->setMonitorBlockToken(300); + thread->clearMonitorBlock(); + + ASSERT_TRUE(thread->monitorEnter( + 400, context, 500, OSThreadState::MONITOR_WAIT)); + EXPECT_EQ(0ULL, thread->monitorBlockToken()); +} + TEST(WallClockOwnedBlockFilterTest, SlotStateTransitions) { WallClockBlockTracker::BlockState slot; diff --git a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp index ba6bbf2643..9b555544b3 100644 --- a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp +++ b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp @@ -252,6 +252,107 @@ TEST_F(TaskBlockRecorderTest, StaleCachedFilterSlotIsSelfHealedBeforeExit) { "healed, instead of being stranded by a spurious slot mismatch"; } +TEST_F(TaskBlockRecorderTest, RotationRejectsParkExitWithoutBlockingOrStranding) { + constexpr int tid = 12346; + ThreadFilter filter; + WallClockBlockTracker tracker; + filter.setBlockTracker(&tracker); + filter.init("", true); + ThreadFilter::SlotID slot_id = filter.registerThread(tid); + ASSERT_GE(slot_id, 0); + + std::unique_ptr current( + ProfiledThread::forTid(tid), ProfiledThread::deleteForTest); + current->setFilterSlotId(slot_id); + Context context{}; + ASSERT_TRUE(current->parkEnter(TSC::ticks(), context)); + u64 token = tracker.enterBlockedRun( + &filter, slot_id, OSThreadState::CONDVAR_WAIT, BlockRunOwner::JAVA); + ASSERT_NE(0ULL, token); + current->setParkBlockToken(token); + + Profiler* profiler = Profiler::instance(); + profiler->beginTaskBlockRotationForTest(); + std::future result = std::async(std::launch::async, [&]() { + u64 start_ticks = 0; + u64 exit_token = 0; + Context exit_context{}; + if (!current->parkExit(start_ticks, exit_context, exit_token)) return true; + return finishTaskBlockAtExit( + current.get(), &filter, &tracker, nullptr, 1, exit_token, start_ticks, + exit_context, 0, 0); + }); + + EXPECT_EQ(std::future_status::ready, + result.wait_for(std::chrono::seconds(1))); + WallClockBlockTracker::BlockState* slot = tracker.slotForId(slot_id); + ASSERT_NE(nullptr, slot); + EXPECT_EQ(BlockRunOwner::NONE, slot->activeBlockOwner()); + EXPECT_EQ(OSThreadState::UNKNOWN, slot->activeBlockState()); + EXPECT_TRUE(current->parkEnter(TSC::ticks(), context)); + u64 ignored_ticks = 0; + u64 ignored_token = 0; + Context ignored_context{}; + EXPECT_TRUE(current->parkExit( + ignored_ticks, ignored_context, ignored_token)); + + profiler->endTaskBlockRotationForTest(); + EXPECT_FALSE(result.get()); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_DROPPED_ROTATION)); +} + +TEST_F(TaskBlockRecorderTest, + RotationRejectsMonitorExitWithoutBlockingOrStranding) { + constexpr int tid = 12347; + ThreadFilter filter; + WallClockBlockTracker tracker; + filter.setBlockTracker(&tracker); + filter.init("", true); + ThreadFilter::SlotID slot_id = filter.registerThread(tid); + ASSERT_GE(slot_id, 0); + + std::unique_ptr current( + ProfiledThread::forTid(tid), ProfiledThread::deleteForTest); + current->setFilterSlotId(slot_id); + Context context{}; + ASSERT_TRUE(current->monitorEnter( + TSC::ticks(), context, 7, OSThreadState::OBJECT_WAIT)); + u64 token = tracker.enterBlockedRun( + &filter, slot_id, OSThreadState::OBJECT_WAIT, BlockRunOwner::JVMTI); + ASSERT_NE(0ULL, token); + current->setMonitorBlockToken(token); + + Profiler* profiler = Profiler::instance(); + profiler->beginTaskBlockRotationForTest(); + std::future result = std::async(std::launch::async, [&]() { + u64 start_ticks = 0; + u64 blocker = 0; + u64 exit_token = 0; + Context exit_context{}; + if (!current->monitorExit(OSThreadState::OBJECT_WAIT, start_ticks, + exit_context, blocker, exit_token)) { + return true; + } + return finishTaskBlockAtExit( + current.get(), &filter, &tracker, nullptr, 0, exit_token, start_ticks, + exit_context, blocker, 0); + }); + + EXPECT_EQ(std::future_status::ready, + result.wait_for(std::chrono::seconds(1))); + WallClockBlockTracker::BlockState* slot = tracker.slotForId(slot_id); + ASSERT_NE(nullptr, slot); + EXPECT_EQ(BlockRunOwner::NONE, slot->activeBlockOwner()); + EXPECT_EQ(OSThreadState::UNKNOWN, slot->activeBlockState()); + EXPECT_TRUE(current->monitorEnter( + TSC::ticks(), context, 8, OSThreadState::MONITOR_WAIT)); + current->clearMonitorBlock(); + + profiler->endTaskBlockRotationForTest(); + EXPECT_FALSE(result.get()); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_DROPPED_ROTATION)); +} + TEST_F(TaskBlockRecorderTest, StackCaptureFailureIsCountedAndActivityReleased) { g_record_result.store(Profiler::TaskBlockRecordResult::STACK_CAPTURE_FAILED, std::memory_order_release); diff --git a/ddprof-lib/src/test/cpp/vmEntry_ut.cpp b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp new file mode 100644 index 0000000000..fa740cd17e --- /dev/null +++ b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp @@ -0,0 +1,500 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +#include + +#include "profiler.h" +#include "vmEntry.h" + +class VMTestAccessor { + public: + static jvmtiEnv* jvmti() { return VM::_jvmti; } + static void setJvmti(jvmtiEnv* jvmti) { VM::_jvmti = jvmti; } + + static bool nativeMonitorEventsAvailable() { + return VM::_native_monitor_events_available; + } + static void setNativeMonitorEventsAvailable(bool available) { + VM::_native_monitor_events_available = available; + } + + static bool monitorWaitEventsDelegated() { + return VM::_monitor_wait_events_delegated; + } + static void setMonitorWaitEventsDelegated(bool delegated) { + VM::_monitor_wait_events_delegated = delegated; + } + + static bool profilerBridgeInitialized() { + return VM::_profiler_bridge_initialized; + } + static void setProfilerBridgeInitialized(bool initialized) { + VM::_profiler_bridge_initialized = initialized; + } + + static void configureMonitorEvents(bool delegate_monitor_wait_events) { + VM::configureMonitorEvents(delegate_monitor_wait_events); + } +}; + +class ProfilerTestAccessor { + public: + static void setTaskBlockEnabled(Profiler* profiler, bool enabled) { + profiler->setTaskBlockEnabled(enabled); + } + + static void setTaskBlockState(Profiler* profiler, bool enabled, + bool monitor_events_enabled) { + profiler->_task_block_enabled.store(enabled, std::memory_order_release); + profiler->_task_block_monitor_events_enabled.store( + monitor_events_enabled, std::memory_order_release); + } + + static bool monitorEventsEnabled(Profiler* profiler) { + return profiler->_task_block_monitor_events_enabled.load( + std::memory_order_acquire); + } +}; + +class MonitorEventConfigurationTest : public ::testing::Test { + protected: + inline static MonitorEventConfigurationTest* active_test = nullptr; + + jvmtiInterface_1_ functions{}; + _jvmtiEnv mock_env{}; + jvmtiEnv* original_jvmti = nullptr; + bool original_initialized = false; + bool original_available = false; + bool original_delegated = false; + bool capability_available = false; + int get_capabilities_calls = 0; + + static jvmtiError JNICALL getCapabilities( + jvmtiEnv*, jvmtiCapabilities* capabilities) { + MonitorEventConfigurationTest* test = active_test; + *capabilities = jvmtiCapabilities{}; + capabilities->can_generate_monitor_events = test->capability_available; + test->get_capabilities_calls++; + return JVMTI_ERROR_NONE; + } + + void SetUp() override { + original_jvmti = VMTestAccessor::jvmti(); + original_initialized = VMTestAccessor::profilerBridgeInitialized(); + original_available = VMTestAccessor::nativeMonitorEventsAvailable(); + original_delegated = VMTestAccessor::monitorWaitEventsDelegated(); + + functions.GetCapabilities = &getCapabilities; + mock_env.functions = &functions; + VMTestAccessor::setJvmti(&mock_env); + VMTestAccessor::setProfilerBridgeInitialized(false); + active_test = this; + } + + void TearDown() override { + active_test = nullptr; + VMTestAccessor::setMonitorWaitEventsDelegated(original_delegated); + VMTestAccessor::setNativeMonitorEventsAvailable(original_available); + VMTestAccessor::setProfilerBridgeInitialized(original_initialized); + VMTestAccessor::setJvmti(original_jvmti); + } +}; + +TEST_F(MonitorEventConfigurationTest, + StoresRequestedOwnershipIndependentlyOfCapability) { + for (bool available : {false, true}) { + for (bool delegated : {false, true}) { + SCOPED_TRACE(::testing::Message() + << "available=" << available + << ", delegated=" << delegated); + capability_available = available; + get_capabilities_calls = 0; + + VMTestAccessor::configureMonitorEvents(delegated); + + EXPECT_EQ(1, get_capabilities_calls); + EXPECT_EQ(available, VMTestAccessor::nativeMonitorEventsAvailable()); + EXPECT_EQ(delegated, VMTestAccessor::monitorWaitEventsDelegated()); + EXPECT_FALSE(VMTestAccessor::profilerBridgeInitialized()); + } + } +} + +class ProfilerBridgeDelegationTest : public ::testing::Test { + protected: + bool original_initialized = false; + bool original_available = false; + bool original_delegated = false; + + void SetUp() override { + original_initialized = VMTestAccessor::profilerBridgeInitialized(); + original_available = VMTestAccessor::nativeMonitorEventsAvailable(); + original_delegated = VMTestAccessor::monitorWaitEventsDelegated(); + VMTestAccessor::setProfilerBridgeInitialized(true); + } + + void TearDown() override { + VMTestAccessor::setMonitorWaitEventsDelegated(original_delegated); + VMTestAccessor::setNativeMonitorEventsAvailable(original_available); + VMTestAccessor::setProfilerBridgeInitialized(original_initialized); + } + + static void expectNegotiation(bool available, bool delegated, + bool requested, + ProfilerBridgeInitResult expected) { + VMTestAccessor::setNativeMonitorEventsAvailable(available); + VMTestAccessor::setMonitorWaitEventsDelegated(delegated); + + EXPECT_EQ(expected, VM::initProfilerBridge(nullptr, true, requested)); + EXPECT_TRUE(VMTestAccessor::profilerBridgeInitialized()); + EXPECT_EQ(available, VMTestAccessor::nativeMonitorEventsAvailable()); + EXPECT_EQ(delegated, VMTestAccessor::monitorWaitEventsDelegated()); + } +}; + +TEST_F(ProfilerBridgeDelegationTest, + ReusesMatchingOwnershipWhenCapabilityIsUnavailable) { + expectNegotiation(false, false, false, ProfilerBridgeInitResult::SUCCESS); + expectNegotiation(false, true, true, ProfilerBridgeInitResult::SUCCESS); +} + +TEST_F(ProfilerBridgeDelegationTest, + RejectsConflictingOwnershipWhenCapabilityIsUnavailable) { + expectNegotiation( + false, false, true, + ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT); + expectNegotiation( + false, true, false, + ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT); +} + +TEST_F(ProfilerBridgeDelegationTest, + ReusesMatchingOwnershipWhenCapabilityIsAvailable) { + expectNegotiation(true, false, false, ProfilerBridgeInitResult::SUCCESS); + expectNegotiation(true, true, true, ProfilerBridgeInitResult::SUCCESS); +} + +TEST_F(ProfilerBridgeDelegationTest, + RejectsConflictingOwnershipWhenCapabilityIsAvailable) { + expectNegotiation( + true, false, true, + ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT); + expectNegotiation( + true, true, false, + ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT); +} + +class NativeMonitorEventsTest : public ::testing::Test { + protected: + struct EventCall { + jvmtiEventMode mode; + jvmtiEvent event; + bool task_block_enabled; + }; + + static constexpr std::array MONITOR_EVENTS = { + JVMTI_EVENT_MONITOR_CONTENDED_ENTER, + JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, + JVMTI_EVENT_MONITOR_WAIT, + JVMTI_EVENT_MONITOR_WAITED, + }; + + inline static NativeMonitorEventsTest* active_test = nullptr; + + jvmtiInterface_1_ functions{}; + _jvmtiEnv mock_env{}; + std::vector calls; + std::array event_enabled{}; + bool inject_failure = false; + bool fail_all_disables = false; + jvmtiEventMode failure_mode = JVMTI_ENABLE; + jvmtiEvent failure_event = JVMTI_EVENT_MONITOR_CONTENDED_ENTER; + + Profiler* profiler = Profiler::instance(); + jvmtiEnv* original_jvmti = nullptr; + bool original_available = false; + bool original_delegated = false; + bool original_task_block_enabled = false; + bool original_monitor_events_enabled = false; + + static jvmtiError JNICALL setEventNotificationMode( + jvmtiEnv*, jvmtiEventMode mode, jvmtiEvent event, jthread, ...) { + NativeMonitorEventsTest* test = active_test; + test->calls.push_back( + {mode, event, test->profiler->taskBlockEnabled()}); + if (test->inject_failure && mode == test->failure_mode && + event == test->failure_event) { + return JVMTI_ERROR_INTERNAL; + } + if (test->fail_all_disables && mode == JVMTI_DISABLE) { + return JVMTI_ERROR_INTERNAL; + } + + test->event_enabled[test->eventIndex(event)] = mode == JVMTI_ENABLE; + return JVMTI_ERROR_NONE; + } + + void SetUp() override { + original_jvmti = VMTestAccessor::jvmti(); + original_available = VMTestAccessor::nativeMonitorEventsAvailable(); + original_delegated = VMTestAccessor::monitorWaitEventsDelegated(); + original_task_block_enabled = profiler->taskBlockEnabled(); + original_monitor_events_enabled = + ProfilerTestAccessor::monitorEventsEnabled(profiler); + + functions.SetEventNotificationMode = &setEventNotificationMode; + mock_env.functions = &functions; + VMTestAccessor::setJvmti(&mock_env); + VMTestAccessor::setNativeMonitorEventsAvailable(true); + VMTestAccessor::setMonitorWaitEventsDelegated(false); + ProfilerTestAccessor::setTaskBlockState(profiler, false, false); + active_test = this; + } + + void TearDown() override { + active_test = nullptr; + ProfilerTestAccessor::setTaskBlockState( + profiler, original_task_block_enabled, original_monitor_events_enabled); + VMTestAccessor::setMonitorWaitEventsDelegated(original_delegated); + VMTestAccessor::setNativeMonitorEventsAvailable(original_available); + VMTestAccessor::setJvmti(original_jvmti); + } + + static size_t eventIndex(jvmtiEvent event) { + for (size_t i = 0; i < MONITOR_EVENTS.size(); i++) { + if (MONITOR_EVENTS[i] == event) return i; + } + ADD_FAILURE() << "Unexpected JVMTI event " << event; + return 0; + } + + bool eventIsEnabled(jvmtiEvent event) const { + return event_enabled[eventIndex(event)]; + } + + void setAllEventsEnabled(bool enabled) { + event_enabled.fill(enabled); + } + + void resetObservations() { + calls.clear(); + event_enabled.fill(false); + inject_failure = false; + fail_all_disables = false; + } + + void fail(jvmtiEventMode mode, jvmtiEvent event) { + inject_failure = true; + failure_mode = mode; + failure_event = event; + } + + void expectCalls( + const std::vector>& expected) { + ASSERT_EQ(expected.size(), calls.size()); + for (size_t i = 0; i < expected.size(); i++) { + EXPECT_EQ(expected[i].first, calls[i].mode) << "call " << i; + EXPECT_EQ(expected[i].second, calls[i].event) << "call " << i; + } + } + + static std::vector> disableCalls() { + return { + {JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER}, + {JVMTI_DISABLE, JVMTI_EVENT_MONITOR_WAIT}, + {JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED}, + {JVMTI_DISABLE, JVMTI_EVENT_MONITOR_WAITED}, + }; + } +}; + +TEST_F(NativeMonitorEventsTest, EnablesTerminalEventsBeforeEntryEvents) { + EXPECT_TRUE(VM::setNativeMonitorEventsEnabled(true)); + + expectCalls({ + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED}, + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAITED}, + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER}, + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT}, + }); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_TRUE(eventIsEnabled(event)); + } +} + +TEST_F(NativeMonitorEventsTest, DelegatedEnableOnlyInstallsContendedPair) { + VMTestAccessor::setMonitorWaitEventsDelegated(true); + + EXPECT_TRUE(VM::setNativeMonitorEventsEnabled(true)); + + expectCalls({ + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED}, + {JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER}, + }); + EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_CONTENDED_ENTER)); + EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_CONTENDED_ENTERED)); + EXPECT_FALSE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAIT)); + EXPECT_FALSE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAITED)); +} + +TEST_F(NativeMonitorEventsTest, DisableRemovesEntriesBeforeTerminalEvents) { + for (bool delegated : {false, true}) { + SCOPED_TRACE(delegated); + resetObservations(); + setAllEventsEnabled(true); + VMTestAccessor::setMonitorWaitEventsDelegated(delegated); + + EXPECT_TRUE(VM::setNativeMonitorEventsEnabled(false)); + + expectCalls(disableCalls()); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_FALSE(eventIsEnabled(event)); + } + } +} + +TEST_F(NativeMonitorEventsTest, EnableFailureStopsAndRollsBackAllEvents) { + const std::array enable_order = { + JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, + JVMTI_EVENT_MONITOR_WAITED, + JVMTI_EVENT_MONITOR_CONTENDED_ENTER, + JVMTI_EVENT_MONITOR_WAIT, + }; + + for (size_t failure_index = 0; failure_index < enable_order.size(); + failure_index++) { + SCOPED_TRACE(failure_index); + resetObservations(); + fail(JVMTI_ENABLE, enable_order[failure_index]); + + EXPECT_FALSE(VM::setNativeMonitorEventsEnabled(true)); + + std::vector> expected; + for (size_t i = 0; i <= failure_index; i++) { + expected.push_back({JVMTI_ENABLE, enable_order[i]}); + } + std::vector> rollback = + disableCalls(); + expected.insert(expected.end(), rollback.begin(), rollback.end()); + expectCalls(expected); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_FALSE(eventIsEnabled(event)); + } + } +} + +TEST_F(NativeMonitorEventsTest, DelegatedEnableFailureRollsBackAllEvents) { + VMTestAccessor::setMonitorWaitEventsDelegated(true); + const std::array enable_order = { + JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, + JVMTI_EVENT_MONITOR_CONTENDED_ENTER, + }; + + for (size_t failure_index = 0; failure_index < enable_order.size(); + failure_index++) { + SCOPED_TRACE(failure_index); + resetObservations(); + fail(JVMTI_ENABLE, enable_order[failure_index]); + + EXPECT_FALSE(VM::setNativeMonitorEventsEnabled(true)); + + std::vector> expected; + for (size_t i = 0; i <= failure_index; i++) { + expected.push_back({JVMTI_ENABLE, enable_order[i]}); + } + std::vector> rollback = + disableCalls(); + expected.insert(expected.end(), rollback.begin(), rollback.end()); + expectCalls(expected); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_FALSE(eventIsEnabled(event)); + } + } +} + +TEST_F(NativeMonitorEventsTest, DisableFailureStillAttemptsEveryEvent) { + for (jvmtiEvent failed_event : MONITOR_EVENTS) { + SCOPED_TRACE(failed_event); + resetObservations(); + setAllEventsEnabled(true); + fail(JVMTI_DISABLE, failed_event); + + EXPECT_FALSE(VM::setNativeMonitorEventsEnabled(false)); + + expectCalls(disableCalls()); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_EQ(event == failed_event, eventIsEnabled(event)); + } + } +} + +TEST_F(NativeMonitorEventsTest, UnavailableCapabilityDoesNotCallJvmti) { + VMTestAccessor::setNativeMonitorEventsAvailable(false); + + EXPECT_FALSE(VM::setNativeMonitorEventsEnabled(true)); + EXPECT_TRUE(calls.empty()); +} + +TEST_F(NativeMonitorEventsTest, AdmissionRemainsClosedDuringSuccessfulSetup) { + ProfilerTestAccessor::setTaskBlockEnabled(profiler, true); + + ASSERT_FALSE(calls.empty()); + for (const EventCall& call : calls) { + EXPECT_FALSE(call.task_block_enabled); + } + EXPECT_TRUE(profiler->taskBlockEnabled()); + EXPECT_TRUE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); +} + +TEST_F(NativeMonitorEventsTest, + AdmissionRemainsClosedDuringFailedSetupAndRollback) { + fail(JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT); + + ProfilerTestAccessor::setTaskBlockEnabled(profiler, true); + + ASSERT_FALSE(calls.empty()); + for (const EventCall& call : calls) { + EXPECT_FALSE(call.task_block_enabled); + } + EXPECT_TRUE(profiler->taskBlockEnabled()); + EXPECT_FALSE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_FALSE(eventIsEnabled(event)); + } +} + +TEST_F(NativeMonitorEventsTest, + NativeAdmissionRemainsClosedWhenSetupAndRollbackFail) { + fail(JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT); + fail_all_disables = true; + + ProfilerTestAccessor::setTaskBlockEnabled(profiler, true); + + EXPECT_TRUE(profiler->taskBlockEnabled()); + EXPECT_FALSE(profiler->nativeMonitorTaskBlockEnabled()); + EXPECT_FALSE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); + EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_CONTENDED_ENTER)); + EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_CONTENDED_ENTERED)); + EXPECT_FALSE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAIT)); + EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAITED)); +} + +TEST_F(NativeMonitorEventsTest, AdmissionClosesBeforeNativeTeardown) { + setAllEventsEnabled(true); + ProfilerTestAccessor::setTaskBlockState(profiler, true, true); + + ProfilerTestAccessor::setTaskBlockEnabled(profiler, false); + + expectCalls(disableCalls()); + for (const EventCall& call : calls) { + EXPECT_FALSE(call.task_block_enabled); + } + EXPECT_FALSE(profiler->taskBlockEnabled()); + EXPECT_FALSE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java index b22c6f42e4..2f0fc69681 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -15,7 +15,14 @@ import java.lang.management.ThreadMXBean; import java.lang.reflect.Method; import java.nio.file.Paths; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Random; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.LongAdder; /** @@ -58,6 +65,14 @@ * {@link ThreadLocalLeakScenario}: a growing {@code byte[]} collection held only through the * leaking thread's {@code ThreadLocal} map; the {@code -late} variant starts the leak thread * only after the recording began (see that scenario's own comment). + *

  • profiler-agent-compatible - reuses native monitor ownership after agent initialization
    • + *
  • profiler-delegation-conflict - requests delegated monitor ownership after agent initialization
    • + *
  • profiler-java-default-delegation-reuse - verifies explicit native ownership after default initialization
    • + *
  • profiler-java-default-delegation-conflict - verifies delegated ownership conflicts after default initialization
    • + *
  • profiler-java-delegation-reuse:<delegated> - verifies compatible Java singleton ownership reuse
    • + *
  • profiler-java-delegation-conflict:<initial>:<requested> - verifies conflicting Java singleton ownership requests
    • + *
  • profiler-preexisting-monitor-wait - exercises Object.wait on a thread created before profiler initialization
    • + *
  • profiler-preexisting-monitor-contention - exercises monitor contention on a thread created before profiler initialization
    • * */ public class ExternalLauncher { @@ -161,6 +176,63 @@ private static String[] unpackScenarioArgs(String mode, String[] args) { return new String[] {packed.substring(0, sep), packed.substring(sep + "|||".length())}; } + /** Runs one native monitor callback lifecycle on a platform thread created before JNI load. */ + private static void runPreExistingMonitorCallback(boolean contention) throws Exception { + ExecutorService executor = Executors.newSingleThreadExecutor(task -> { + Thread thread = new Thread(task, "preexisting-monitor-callback"); + thread.setDaemon(true); + return thread; + }); + executor.submit(Thread::currentThread).get(5, TimeUnit.SECONDS); + + Path recording = Files.createTempFile("preexisting-monitor-callback", ".jfr"); + JavaProfiler profiler = null; + boolean started = false; + try { + profiler = JavaProfiler.getInstance(); + profiler.execute("start,wall=1ms,filter=,wallprecheck=true,jfr,file=" + + recording.toAbsolutePath()); + started = true; + long before = profiler.getDebugCounters().getOrDefault("task_block_emitted", 0L); + Object monitor = new Object(); + + if (contention) { + CountDownLatch attempting = new CountDownLatch(1); + Future blocked; + synchronized (monitor) { + blocked = executor.submit(() -> { + attempting.countDown(); + synchronized (monitor) { + // Acquiring the monitor completes the contended interval. + } + }); + if (!attempting.await(5, TimeUnit.SECONDS)) { + throw new IllegalStateException("Worker did not attempt monitor entry"); + } + Thread.sleep(100L); + } + blocked.get(5, TimeUnit.SECONDS); + } else { + executor.submit(() -> { + synchronized (monitor) { + monitor.wait(100L); + } + return null; + }).get(5, TimeUnit.SECONDS); + } + + long emitted = profiler.getDebugCounters().getOrDefault("task_block_emitted", 0L) - before; + System.out.println("[preexisting-monitor-events] " + emitted); + } finally { + if (started) { + profiler.stop(); + } + executor.shutdownNow(); + executor.awaitTermination(5, TimeUnit.SECONDS); + Files.deleteIfExists(recording); + } + } + public static void main(String[] args) throws Exception { Thread worker = null; try { @@ -181,6 +253,70 @@ public static void main(String[] args) throws Exception { } }); vt.join(); + JavaProfiler initial = JavaProfiler.getInstance(); + JavaProfiler reused = JavaProfiler.getInstance(); + System.out.println("[virtual-thread-recovery] " + + (initial == reused) + " " + reused.isMonitorWaitEventsDelegated()); + } else if (args[0].equals("profiler-delegation-conflict")) { + String libraryPath = System.getProperty("ddprof.test.agent.path"); + try { + JavaProfiler.getInstance(libraryPath, null, true); + System.out.println("[delegation-conflict-missed]"); + } catch (IllegalStateException expected) { + JavaProfiler recovered = + JavaProfiler.getInstance(libraryPath, null, false); + System.out.println("[delegation-conflict] " + + recovered.isMonitorWaitEventsDelegated()); + } + } else if (args[0].equals("profiler-java-default-delegation-reuse")) { + JavaProfiler initial = JavaProfiler.getInstance(); + JavaProfiler reused = JavaProfiler.getInstance(null, null, false); + System.out.println("[java-default-delegation-reuse] " + + (initial == reused) + " " + reused.isMonitorWaitEventsDelegated()); + } else if (args[0].equals("profiler-java-default-delegation-conflict")) { + JavaProfiler initial = JavaProfiler.getInstance(); + try { + JavaProfiler.getInstance(null, null, true); + System.out.println("[java-default-delegation-conflict-missed]"); + } catch (IllegalStateException expected) { + JavaProfiler recovered = + JavaProfiler.getInstance(null, null, false); + System.out.println("[java-default-delegation-conflict] " + + (initial == recovered) + " " + + recovered.isMonitorWaitEventsDelegated()); + } + } else if (args[0].startsWith("profiler-java-delegation-reuse:")) { + boolean delegated = Boolean.parseBoolean( + args[0].substring("profiler-java-delegation-reuse:".length())); + JavaProfiler initial = JavaProfiler.getInstance(null, null, delegated); + JavaProfiler reused = JavaProfiler.getInstance(null, null, delegated); + System.out.println("[java-delegation-reuse] " + + (initial == reused) + " " + reused.isMonitorWaitEventsDelegated()); + } else if (args[0].startsWith("profiler-java-delegation-conflict:")) { + String[] delegationModes = args[0].split(":"); + boolean initialDelegation = Boolean.parseBoolean(delegationModes[1]); + boolean requestedDelegation = Boolean.parseBoolean(delegationModes[2]); + JavaProfiler initial = + JavaProfiler.getInstance(null, null, initialDelegation); + try { + JavaProfiler.getInstance(null, null, requestedDelegation); + System.out.println("[java-delegation-conflict-missed]"); + } catch (IllegalStateException expected) { + JavaProfiler recovered = + JavaProfiler.getInstance(null, null, initialDelegation); + System.out.println("[java-delegation-conflict] " + + (initial == recovered) + " " + + recovered.isMonitorWaitEventsDelegated()); + } + } else if (args[0].equals("profiler-agent-compatible")) { + String libraryPath = System.getProperty("ddprof.test.agent.path"); + JavaProfiler profiler = JavaProfiler.getInstance(libraryPath, null, false); + System.out.println("[agent-compatible] " + + profiler.isMonitorWaitEventsDelegated()); + } else if (args[0].equals("profiler-preexisting-monitor-wait")) { + runPreExistingMonitorCallback(false); + } else if (args[0].equals("profiler-preexisting-monitor-contention")) { + runPreExistingMonitorCallback(true); } else if (args[0].equals("profiler")) { JavaProfiler instance = JavaProfiler.getInstance(); if (args.length == 2) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java index 06da945600..c02ed38ac4 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -17,6 +17,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +/** Locks the supported public boundary and package-scoped producer hooks. */ public class JavaProfilerApiSurfaceTest { @Test public void taskBlockApiIsPublicButInternalHooksRemainPackageScoped() throws Exception { @@ -63,6 +64,15 @@ public void referenceChainTestSeamsAreNotPublicApi() { "every listed reference-chain test seam must still exist on JavaProfiler"); } + @Test + public void monitorWaitOwnershipIsExplicitPublicApi() throws Exception { + assertTrue(Modifier.isPublic(JavaProfiler.class + .getDeclaredMethod("getInstance", String.class, String.class, boolean.class) + .getModifiers())); + assertTrue(Modifier.isPublic(JavaProfiler.class + .getDeclaredMethod("isMonitorWaitEventsDelegated").getModifiers())); + } + private static void assertNotPublic(Method method) { assertFalse(Modifier.isPublic(method.getModifiers()), method.getName() + " is an internal instrumentation hook"); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java index 2023c4757c..02a378d3e2 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java @@ -7,8 +7,10 @@ import org.junit.jupiter.api.Test; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -18,12 +20,46 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.LockSupport; +import java.util.function.Function; import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assumptions.assumeFalse; import static org.junit.jupiter.api.Assumptions.assumeTrue; public class JavaProfilerTest extends AbstractProcessProfilerTest { + /** Extracts the packaged native library so a child JVM can load it through {@code -agentpath}. */ + private static Path extractProfilerLibrary() throws Exception { + OperatingSystem os = OperatingSystem.current(); + String extension = os == OperatingSystem.macos ? "dylib" : "so"; + String qualifier = os == OperatingSystem.linux && os.isMusl() ? "-musl" : ""; + String resource = "/META-INF/native-libs/" + os.name().toLowerCase() + "-" + + Arch.current().name().toLowerCase() + qualifier + "/libjavaProfiler." + extension; + Path library = Files.createTempFile("libjavaProfiler-agent-", "." + extension); + try (InputStream input = JavaProfiler.class.getResourceAsStream(resource)) { + assertNotNull(input, "Profiler library resource not found: " + resource); + Files.copy(input, library, StandardCopyOption.REPLACE_EXISTING); + } + return library; + } + + /** Launches a child JVM whose profiler bridge is initialized before Java application startup. */ + private LaunchResult launchWithProfilerAgent( + String target, Function onStdoutLine) throws Exception { + Path library = extractProfilerLibrary(); + Path recording = Files.createTempFile("agent-initialization-", ".jfr"); + try { + List jvmArgs = new ArrayList<>(); + jvmArgs.add("-agentpath:" + library.toAbsolutePath() + + "=start,wall=10ms,filter=,wallprecheck=true,jfr,file=" + + recording.toAbsolutePath()); + jvmArgs.add("-Dddprof.test.agent.path=" + library.toAbsolutePath()); + return launch(target, jvmArgs, "", onStdoutLine, null); + } finally { + Files.deleteIfExists(recording); + Files.deleteIfExists(library); + } + } + @Test void sanityInitailizationTest() throws Exception { String config = System.getProperty("ddprof_test.config"); @@ -118,20 +154,173 @@ void testJ9ForceJvmtiSanity() throws Exception { void getInstanceFromVirtualThreadThrowsIOException() throws Exception { assumeTrue(Platform.isJavaVersionAtLeast(21)); - AtomicReference resultLine = new AtomicReference<>(); + AtomicReference attemptLine = new AtomicReference<>(); + AtomicReference recoveryLine = new AtomicReference<>(); boolean val = launch("profiler-virtual-thread", Collections.emptyList(), "", l -> { if (l.startsWith("[virtual-thread-")) { - resultLine.set(l); - return LineConsumerResult.STOP; + if (l.startsWith("[virtual-thread-recovery]")) { + recoveryLine.set(l); + return LineConsumerResult.STOP; + } + attemptLine.set(l); + return LineConsumerResult.CONTINUE; } return LineConsumerResult.CONTINUE; }, null).inTime; assertTrue(val); - String result = resultLine.get(); + String result = attemptLine.get(); assertNotNull(result, "getInstance() did not report a result from the virtual thread"); assertTrue(result.startsWith("[virtual-thread-ioexception]"), "Expected IOException from getInstance() on a virtual thread, got: " + result); + assertEquals("[virtual-thread-recovery] true false", recoveryLine.get()); + } + + @Test + void compatibleLateJavaInitializationReusesAgentBridge() throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launchWithProfilerAgent("profiler-agent-compatible", line -> { + if (line.startsWith("[agent-compatible]")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertEquals("[agent-compatible] false", resultLine.get()); + } + + @Test + void conflictingLateMonitorDelegationIsRejected() throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launchWithProfilerAgent("profiler-delegation-conflict", line -> { + if (line.startsWith("[delegation-conflict")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertEquals("[delegation-conflict] false", resultLine.get()); + } + + @Test + void defaultJavaSingletonMonitorDelegationIsReused() throws Exception { + assertJavaDelegationScenario( + "profiler-java-default-delegation-reuse", + "[java-default-delegation-reuse]", + "[java-default-delegation-reuse] true false"); + } + + @Test + void conflictingDefaultJavaSingletonMonitorDelegationDoesNotPoisonInstance() + throws Exception { + assertJavaDelegationScenario( + "profiler-java-default-delegation-conflict", + "[java-default-delegation-conflict", + "[java-default-delegation-conflict] true false"); + } + + @Test + void conflictingJavaSingletonMonitorDelegationIsRejected() throws Exception { + assertJavaSingletonDelegationConflict(false, true); + assertJavaSingletonDelegationConflict(true, false); + } + + @Test + void compatibleJavaSingletonMonitorDelegationIsReused() throws Exception { + assertJavaSingletonDelegationReuse(false); + assertJavaSingletonDelegationReuse(true); + } + + /** Launches a fresh JVM and verifies that repeated ownership returns the same singleton. */ + private void assertJavaSingletonDelegationReuse(boolean delegated) throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch( + "profiler-java-delegation-reuse:" + delegated, + Collections.emptyList(), "", line -> { + if (line.startsWith("[java-delegation-reuse]")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertEquals("[java-delegation-reuse] true " + delegated, resultLine.get()); + } + + /** Launches a fresh JVM and verifies that a second ownership mode is rejected. */ + private void assertJavaSingletonDelegationConflict(boolean initialDelegation, + boolean requestedDelegation) throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch( + "profiler-java-delegation-conflict:" + initialDelegation + ":" + requestedDelegation, + Collections.emptyList(), "", line -> { + if (line.startsWith("[java-delegation-conflict")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertEquals( + "[java-delegation-conflict] true " + initialDelegation, + resultLine.get()); + } + + /** Launches a fresh JVM and verifies the exact output of a delegation scenario. */ + private void assertJavaDelegationScenario( + String target, String marker, String expected) throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch( + target, Collections.emptyList(), "", line -> { + if (line.startsWith(marker)) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertEquals(expected, resultLine.get()); + } + + @Test + void preExistingThreadObjectWaitUsesNativeMonitorCallbacks() throws Exception { + assertPreExistingMonitorCallback("profiler-preexisting-monitor-wait"); + } + + @Test + void preExistingThreadContentionUsesNativeMonitorCallbacks() throws Exception { + assertPreExistingMonitorCallback("profiler-preexisting-monitor-contention"); + } + + /** Verifies that a pre-JNI-load worker emits a TaskBlock through its first monitor callback. */ + private void assertPreExistingMonitorCallback(String target) throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch(target, Collections.emptyList(), "", line -> { + if (line.startsWith("[preexisting-monitor-events]")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertNotNull(resultLine.get(), "Pre-existing monitor callback did not report a result"); + long emitted = Long.parseLong(resultLine.get().substring( + "[preexisting-monitor-events] ".length())); + assertTrue(emitted > 0, "Pre-existing thread emitted no native monitor TaskBlock event"); } @Test diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java new file mode 100644 index 0000000000..ff6df5c970 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java @@ -0,0 +1,30 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.Platform; +import java.util.Map; +import org.junit.jupiter.api.Assumptions; + +/** Verifies synchronous monitor production when delegated wall-clock stacks are enabled. */ +public class JvmtiBasedMonitorTaskBlockTest extends MonitorTaskBlockTest { + @Override + protected void before() { + Map counters = profiler.getDebugCounters(); + Assumptions.assumeTrue(counters.getOrDefault("jvmti_stacks_init_ok", 0L) > 0, + "HotSpot RequestStackTrace JVMTI extension is not available"); + } + + @Override + protected void withTestAssumptions() { + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11)); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,filter=,wallprecheck=true,jvmtistacks=true"; + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedParkTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedParkTaskBlockTest.java new file mode 100644 index 0000000000..63e56c3805 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedParkTaskBlockTest.java @@ -0,0 +1,30 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.Platform; +import java.util.Map; +import org.junit.jupiter.api.Assumptions; + +/** Verifies synchronous park production when delegated wall-clock stacks are enabled. */ +public class JvmtiBasedParkTaskBlockTest extends ParkTaskBlockTest { + @Override + protected void before() { + Map counters = profiler.getDebugCounters(); + Assumptions.assumeTrue(counters.getOrDefault("jvmti_stacks_init_ok", 0L) > 0, + "HotSpot RequestStackTrace JVMTI extension is not available"); + } + + @Override + protected void withTestAssumptions() { + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11)); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,filter=,wallprecheck=true,jvmtistacks=true"; + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java new file mode 100644 index 0000000000..25d81b8e4a --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java @@ -0,0 +1,240 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.AbstractProfilerTest; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import com.datadoghq.profiler.JfrEvents; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assumptions; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Verifies TaskBlock production from native JVMTI monitor callbacks. */ +public class MonitorTaskBlockTest extends AbstractProfilerTest { + @Test + public void objectWaitEmitsTaskBlockOutsideContextWindow() throws Exception { + Object monitor = new Object(); + CountDownLatch entered = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + synchronized (monitor) { + entered.countDown(); + monitor.wait(100); + } + } catch (Throwable t) { + failure.set(t); + } + }, "taskblock-object-wait"); + + worker.start(); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + assertCompleted(worker, failure); + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertTaskBlockStackReference(events); + TaskBlockAssertions.assertContains(events, 0, 0, identityHash(monitor), 0); + TaskBlockAssertions.assertContainsObservedState(events, "WAITING"); + } + + @Test + public void monitorContentionEmitsTaskBlockOutsideContextWindow() throws Exception { + Object monitor = new Object(); + CountDownLatch attempting = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Thread worker; + synchronized (monitor) { + worker = new Thread(() -> { + try { + attempting.countDown(); + synchronized (monitor) { + } + } catch (Throwable t) { + failure.set(t); + } + }, "taskblock-monitor-contention"); + worker.start(); + assertTrue(attempting.await(5, TimeUnit.SECONDS)); + Thread.sleep(100); + } + + assertCompleted(worker, failure); + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertTaskBlockStackReference(events); + TaskBlockAssertions.assertContains(events, 0, 0, identityHash(monitor), 0); + TaskBlockAssertions.assertContainsObservedState(events, "CONTENDED"); + } + + @Test + public void contextWindowObjectWaitDoesNotEmitTaskBlock() throws Exception { + Object monitor = new Object(); + AtomicReference failure = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + registerCurrentThreadForWallClockProfiling(); + profiler.setTraceContext(0x4400L, 0x4401L, 0L, 0x4401L, -1, null, -1, null); + synchronized (monitor) { + monitor.wait(100); + } + } catch (Throwable t) { + failure.set(t); + } finally { + profiler.clearTraceContext(); + profiler.removeThread(); + } + }, "taskblock-traced-object-wait"); + + worker.start(); + assertCompleted(worker, failure); + stopProfiler(); + + assertFalse(TaskBlockAssertions.containsBlocker( + verifyEvents("datadog.TaskBlock", false), identityHash(monitor))); + } + + @Test + public void staleWaitStateIsRecoveredAfterProfilerRestart() throws Exception { + Object waitMonitor = new Object(); + Object contentionMonitor = new Object(); + CountDownLatch waiting = new CountDownLatch(1); + CountDownLatch waitCompleted = new CountDownLatch(1); + CountDownLatch restartReady = new CountDownLatch(1); + CountDownLatch attemptingContention = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + synchronized (waitMonitor) { + waiting.countDown(); + waitMonitor.wait(); + } + waitCompleted.countDown(); + assertTrue(restartReady.await(5, TimeUnit.SECONDS)); + attemptingContention.countDown(); + synchronized (contentionMonitor) { + } + } catch (Throwable t) { + failure.set(t); + } + }, "taskblock-monitor-restart"); + + worker.start(); + assertTrue(waiting.await(5, TimeUnit.SECONDS)); + Thread.sleep(50); + stopProfiler(); + synchronized (waitMonitor) { + waitMonitor.notifyAll(); + } + assertTrue(waitCompleted.await(5, TimeUnit.SECONDS)); + + Path recording = Files.createTempFile("MonitorTaskBlockTest-restart-", ".jfr"); + boolean restarted = false; + try { + profiler.execute("start,wall=1ms,filter=,wallprecheck=true,jfr,file=" + + recording.toAbsolutePath()); + restarted = true; + synchronized (contentionMonitor) { + restartReady.countDown(); + assertTrue(attemptingContention.await(5, TimeUnit.SECONDS)); + Thread.sleep(100); + } + assertCompleted(worker, failure); + profiler.stop(); + restarted = false; + + JfrEvents events = verifyEvents(recording, "datadog.TaskBlock", false); + assertTaskBlockStackReference(events); + assertTrue(TaskBlockAssertions.containsBlocker( + events, identityHash(contentionMonitor))); + } finally { + restartReady.countDown(); + synchronized (waitMonitor) { + waitMonitor.notifyAll(); + } + if (restarted) profiler.stop(); + worker.join(5_000); + Files.deleteIfExists(recording); + } + } + + @Test + public void virtualMonitorCallbacksDoNotEmitCarrierTaskBlocks() throws Exception { + Method startVirtualThread; + try { + startVirtualThread = Thread.class.getMethod("startVirtualThread", Runnable.class); + } catch (NoSuchMethodException unavailableBeforeJdk21) { + Assumptions.assumeTrue(false, "virtual threads require JDK 21"); + return; + } + + Object waitMonitor = new Object(); + AtomicReference failure = new AtomicReference<>(); + Thread waiter = (Thread) startVirtualThread.invoke(null, (Runnable) () -> { + try { + synchronized (waitMonitor) { + waitMonitor.wait(100); + } + } catch (Throwable t) { + failure.set(t); + } + }); + assertCompleted(waiter, failure); + + Object contentionMonitor = new Object(); + CountDownLatch attempting = new CountDownLatch(1); + Thread contender; + synchronized (contentionMonitor) { + contender = (Thread) startVirtualThread.invoke(null, (Runnable) () -> { + try { + attempting.countDown(); + synchronized (contentionMonitor) { + } + } catch (Throwable t) { + failure.set(t); + } + }); + assertTrue(attempting.await(5, TimeUnit.SECONDS)); + Thread.sleep(100); + } + assertCompleted(contender, failure); + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock", false); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(waitMonitor))); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(contentionMonitor))); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,filter=,wallprecheck=true"; + } + + protected void assertTaskBlockStackReference(JfrEvents events) { + TaskBlockAssertions.assertContainsStackTrace(events); + TaskBlockAssertions.assertContainsJavaType(events, "MonitorTaskBlockTest"); + TaskBlockAssertions.assertNoCorrelationId(events); + } + + private static void assertCompleted(Thread thread, AtomicReference failure) + throws InterruptedException { + thread.join(5_000); + assertFalse(thread.isAlive(), "worker did not complete"); + if (failure.get() != null) throw new AssertionError(failure.get()); + } + + private static long identityHash(Object object) { + return Integer.toUnsignedLong(System.identityHashCode(object)); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java new file mode 100644 index 0000000000..a1c288be5b --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java @@ -0,0 +1,169 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.JfrEvents; +import com.datadoghq.profiler.ProfilerOwnedBlockHooks; +import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.LockSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assumptions; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Verifies TaskBlock production from Java-owned platform-thread park hooks. */ +public class ParkTaskBlockTest extends AbstractProfilerTest { + private static final long BLOCKER = 0x3102L; + private static final long UNBLOCKING_SPAN_ID = 0x3103L; + + @Test + public void platformParkEmitsTaskBlockOutsideContextWindow() { + ProfilerOwnedBlockHooks.parkEnter(profiler); + try { + parkForMillis(200); + } finally { + ProfilerOwnedBlockHooks.parkExit(profiler, BLOCKER, UNBLOCKING_SPAN_ID); + } + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock"); + TaskBlockAssertions.assertNoAnchorFields(events); + assertTaskBlockStackReference(events); + TaskBlockAssertions.assertContains(events, 0, 0, BLOCKER, UNBLOCKING_SPAN_ID); + TaskBlockAssertions.assertContainsObservedState(events, "PARKED"); + } + + @Test + public void contextWindowParkDoesNotEmitTaskBlock() { + registerCurrentThreadForWallClockProfiling(); + profiler.setTraceContext(0x3100L, 0x3101L, 0L, 0x3101L, -1, null, -1, null); + try { + ProfilerOwnedBlockHooks.parkEnter(profiler); + try { + parkForMillis(200); + } finally { + ProfilerOwnedBlockHooks.parkExit(profiler, BLOCKER, UNBLOCKING_SPAN_ID); + } + } finally { + profiler.clearTraceContext(); + profiler.removeThread(); + } + stopProfiler(); + + assertFalse(verifyEvents("datadog.TaskBlock", false).hasItems(), + "A park inside the context window must remain ordinary wall-clock data"); + } + + @Test + public void virtualParkDoesNotMutateCarrierProducerState() throws Exception { + Method startVirtualThread; + try { + startVirtualThread = Thread.class.getMethod("startVirtualThread", Runnable.class); + } catch (NoSuchMethodException unavailableBeforeJdk21) { + Assumptions.assumeTrue(false, "virtual threads require JDK 21"); + return; + } + + long virtualBlocker = 0x3201L; + Thread virtual = (Thread) startVirtualThread.invoke(null, (Runnable) () -> { + ProfilerOwnedBlockHooks.parkEnter(profiler); + try { + parkForMillis(20); + } finally { + ProfilerOwnedBlockHooks.parkExit(profiler, virtualBlocker, 0); + } + }); + virtual.join(5_000); + assertFalse(virtual.isAlive()); + + ProfilerOwnedBlockHooks.parkEnter(profiler); + try { + parkForMillis(200); + } finally { + ProfilerOwnedBlockHooks.parkExit(profiler, BLOCKER, UNBLOCKING_SPAN_ID); + } + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertFalse(TaskBlockAssertions.containsBlocker(events, virtualBlocker)); + TaskBlockAssertions.assertContains(events, 0, 0, BLOCKER, UNBLOCKING_SPAN_ID); + } + + @Test + public void platformParkSuppressesSignalsAndClearsOwnership() throws Exception { + long baseline = profiler.getDebugCounters() + .getOrDefault("wc_signals_suppressed_owned_block", 0L); + long afterFirstPark = runSuppressedPark(baseline); + runSuppressedPark(afterFirstPark); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,filter=,wallprecheck=true"; + } + + protected void assertTaskBlockStackReference(JfrEvents events) { + TaskBlockAssertions.assertContainsStackTrace(events); + TaskBlockAssertions.assertContainsJavaType(events, "ParkTaskBlockTest"); + TaskBlockAssertions.assertNoCorrelationId(events); + } + + private static void parkForMillis(long millis) { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(millis); + long remaining; + while ((remaining = deadline - System.nanoTime()) > 0) { + LockSupport.parkNanos(remaining); + } + } + + private long runSuppressedPark(long baseline) throws Exception { + CountDownLatch armed = new CountDownLatch(1); + AtomicBoolean release = new AtomicBoolean(); + AtomicReference error = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + ProfilerOwnedBlockHooks.parkEnter(profiler); + armed.countDown(); + while (!release.get()) { + Thread.yield(); + } + } catch (Throwable t) { + error.set(t); + } finally { + ProfilerOwnedBlockHooks.parkExit(profiler, BLOCKER, UNBLOCKING_SPAN_ID); + } + }, "taskblock-park-suppression"); + + worker.start(); + assertTrue(armed.await(5, TimeUnit.SECONDS)); + try { + waitForCounterAbove("wc_signals_suppressed_owned_block", baseline, 5_000L); + } finally { + release.set(true); + } + worker.join(5_000L); + assertFalse(worker.isAlive()); + if (error.get() != null) throw new AssertionError(error.get()); + return profiler.getDebugCounters() + .getOrDefault("wc_signals_suppressed_owned_block", 0L); + } + + private void waitForCounterAbove(String name, long baseline, long timeoutMillis) + throws Exception { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMillis); + while (System.nanoTime() < deadline) { + if (profiler.getDebugCounters().getOrDefault(name, 0L) > baseline) return; + Thread.sleep(10L); + } + throw new AssertionError("Counter did not increase: " + name); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java index 435a18dc8d..5b54c2981c 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java @@ -26,6 +26,15 @@ final class TaskBlockAssertions { private TaskBlockAssertions() {} + static boolean containsBlocker(JfrEvents events, long blocker) { + for (JfrEvent item : events) { + if (item.getLong(BLOCKER, Long.MIN_VALUE) == blocker) { + return true; + } + } + return false; + } + static void assertContains(JfrEvents events, long rootSpanId, long spanId, long blocker, long unblockingSpanId) { for (JfrEvent item : events) { From fd415df30bd90993ff0a76febfcf69eb6544fd71 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 20 Aug 2026 14:25:10 +0200 Subject: [PATCH 2/9] fix --- ddprof-lib/src/main/cpp/javaApi.cpp | 23 ++-- ddprof-lib/src/main/cpp/profiler.cpp | 11 +- ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | 5 +- ddprof-lib/src/main/cpp/taskBlockRecorder.h | 5 +- ddprof-lib/src/main/cpp/vmEntry.cpp | 38 ++++-- .../com/datadoghq/profiler/JavaProfiler.java | 38 ++++-- ddprof-lib/src/test/cpp/vmEntry_ut.cpp | 29 +++- .../datadoghq/profiler/ExternalLauncher.java | 15 ++ .../datadoghq/profiler/JavaProfilerTest.java | 30 ++++ .../JvmtiBasedMonitorTaskBlockTest.java | 17 +++ .../wallclock/MonitorTaskBlockTest.java | 128 +++++++++++++++++- .../profiler/wallclock/ParkTaskBlockTest.java | 7 +- 12 files changed, 302 insertions(+), 44 deletions(-) diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 7911c0caac..65ef12313f 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -445,8 +445,10 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0( extern "C" DLLEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( - JNIEnv *env, jclass unused, jthread thread) { - if (!JVMSupport::isPlatformThread(env, thread)) { + JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual) { + // Virtuality is resolved once on the Java side; re-deriving it here would cost a + // GetVersion() plus an IsVirtualThread() JNI round-trip on every park. + if (isVirtual != JNI_FALSE) { return JNI_FALSE; } ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); @@ -474,9 +476,9 @@ Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkExit0( - JNIEnv *env, jclass unused, jthread thread, jlong blocker, - jlong unblockingSpanId) { - if (!JVMSupport::isPlatformThread(env, thread)) { + JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual, + jlong blocker, jlong unblockingSpanId) { + if (isVirtual != JNI_FALSE) { return; } ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); @@ -522,10 +524,10 @@ static bool isCurrentJniThread(JNIEnv* env, jthread thread) { extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( - JNIEnv *env, jclass unused, jthread thread, jint state) { + JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual, + jint state) { OSThreadState decoded; - if (!decodeJavaBlockState(state, decoded) || - !JVMSupport::isPlatformThread(env, thread)) { + if (!decodeJavaBlockState(state, decoded) || isVirtual != JNI_FALSE) { return 0; } ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); @@ -552,9 +554,10 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0( - JNIEnv *env, jclass unused, jthread thread, jlong token) { + JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual, + jlong token) { u64 block_token = static_cast(token); - if (block_token == 0 || !JVMSupport::isPlatformThread(env, thread)) { + if (block_token == 0 || isVirtual != JNI_FALSE) { return; } diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 2cf2e28a81..87c56e9b84 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1637,10 +1637,13 @@ void Profiler::setTaskBlockEnabled(bool enabled) { } _task_block_enabled.store(false, std::memory_order_release); - if (_task_block_monitor_events_enabled.exchange( - false, std::memory_order_acq_rel)) { - VM::setNativeMonitorEventsEnabled(false); - } + // Clear the admission flag first so no consumer can observe enabled events, then + // always attempt teardown. A previous enable whose setup AND rollback both failed + // left the flag false while JVMTI events stayed on; retrying unconditionally is the + // only way that leak is ever reclaimed. setNativeMonitorEventsEnabled(false) is + // documented as a no-op when the capability was never enabled. + _task_block_monitor_events_enabled.exchange(false, std::memory_order_acq_rel); + VM::setNativeMonitorEventsEnabled(false); } Error Profiler::start(Arguments &args, bool reset) { diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp index c6adddaa06..c6d61d65f7 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp @@ -44,7 +44,8 @@ bool finishTaskBlockAtExit(ProfiledThread* current, WallClockBlockTracker* tracker, jthread thread, int start_depth, u64 block_token, u64 start_ticks, const Context& context, u64 blocker, - u64 unblocking_span_id) { + u64 unblocking_span_id, u64 end_ticks) { + if (end_ticks == 0) end_ticks = TSC::ticks(); Profiler* profiler = Profiler::instance(); bool recording_enabled = profiler->taskBlockEnabled(); TaskBlockActivity activity; @@ -76,6 +77,6 @@ bool finishTaskBlockAtExit(ProfiledThread* current, } return recordTaskBlockIfEligible( - current->tid(), thread, start_depth, start_ticks, TSC::ticks(), context, + current->tid(), thread, start_depth, start_ticks, end_ticks, context, blocker, unblocking_span_id, snapshot.active_state, true); } diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.h b/ddprof-lib/src/main/cpp/taskBlockRecorder.h index a7126b4a46..d8246b3973 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -25,12 +25,15 @@ bool recordTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter, // Cleanup is deliberately performed even when admission is rejected so an // application thread never waits for rotation and suppression cannot be left // armed. +// 'end_ticks' lets a caller that already had to sample the clock (e.g. to decide +// whether the interval is worth resolving a blocker identity for) share the exact +// same end timestamp with the eligibility check; 0 means "sample it here". bool finishTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter, WallClockBlockTracker* tracker, jthread thread, int start_depth, u64 block_token, u64 start_ticks, const Context& context, u64 blocker, - u64 unblocking_span_id); + u64 unblocking_span_id, u64 end_ticks = 0); class TaskBlockActivity { private: diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 4b481cde50..8cbdd82869 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -88,8 +88,12 @@ static u64 monitorBlockerHash(jvmtiEnv *jvmti, jobject object) { return static_cast(static_cast(hash)); } -static void monitorBlockEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, - jobject object, OSThreadState state) { +// Deliberately takes no jvmtiEnv: no JVMTI call may run on this hot path. The +// blocker identity hash is resolved lazily in monitorBlockExit, and only for +// intervals that pass the minimum-duration filter (GetObjectHashCode mutates the +// object's mark word on HotSpot). +static void monitorBlockEnter(JNIEnv *jni, jthread thread, + OSThreadState state) { Profiler *profiler = Profiler::instance(); if (!profiler->taskBlockEnabled() || !profiler->nativeMonitorTaskBlockEnabled() || @@ -104,8 +108,7 @@ static void monitorBlockEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, return; } - if (!current->monitorEnter(TSC::ticks(), context, - monitorBlockerHash(jvmti, object), state)) { + if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { u64 token = current->monitorBlockToken(); ThreadFilter *tf = profiler->threadFilter(); bool current_owner = false; @@ -125,8 +128,7 @@ static void monitorBlockEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, return; } current->clearMonitorBlock(); - if (!current->monitorEnter(TSC::ticks(), context, - monitorBlockerHash(jvmti, object), state)) { + if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { return; } } @@ -150,13 +152,15 @@ static void monitorBlockEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, current->setMonitorBlockToken(token); } -static void monitorBlockExit(JNIEnv *jni, jthread thread, OSThreadState state) { +static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, OSThreadState state) { if (!JVMSupport::isPlatformThread(jni, thread)) return; ProfiledThread *current = ProfiledThread::current(); if (current == nullptr) return; u64 start_ticks = 0; Context context{}; + // The entry side no longer records a blocker; it is resolved lazily below. u64 blocker = 0; u64 token = 0; if (!current->monitorExit(state, start_ticks, context, blocker, token) || @@ -164,32 +168,42 @@ static void monitorBlockExit(JNIEnv *jni, jthread thread, OSThreadState state) { return; } + // Resolve the blocker identity hash only for intervals that will actually pass + // the eligibility filter. GetObjectHashCode mutates the object's mark word on + // HotSpot, so it must not run for short, high-frequency contention that gets + // discarded anyway. These conditions mirror taskBlockPassesBasicEligibility, and + // the same end_ticks is handed down so there is no boundary drift. + u64 end_ticks = TSC::ticks(); + if (context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { + blocker = monitorBlockerHash(jvmti, object); + } + Profiler *profiler = Profiler::instance(); finishTaskBlockAtExit(current, profiler->threadFilter(), thread, 0, token, - start_ticks, context, blocker, 0); + start_ticks, context, blocker, 0, end_ticks); } static void JNICALL MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jobject object) { - monitorBlockEnter(jvmti, jni, thread, object, OSThreadState::MONITOR_WAIT); + monitorBlockEnter(jni, thread, OSThreadState::MONITOR_WAIT); } static void JNICALL MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jobject object) { - monitorBlockExit(jni, thread, OSThreadState::MONITOR_WAIT); + monitorBlockExit(jvmti, jni, thread, object, OSThreadState::MONITOR_WAIT); } static void JNICALL MonitorWait(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jobject object, jlong timeout) { if (!VM::monitorWaitEventsDelegated()) { - monitorBlockEnter(jvmti, jni, thread, object, OSThreadState::OBJECT_WAIT); + monitorBlockEnter(jni, thread, OSThreadState::OBJECT_WAIT); } } static void JNICALL MonitorWaited(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jobject object, jboolean timed_out) { if (!VM::monitorWaitEventsDelegated()) { - monitorBlockExit(jni, thread, OSThreadState::OBJECT_WAIT); + monitorBlockExit(jvmti, jni, thread, object, OSThreadState::OBJECT_WAIT); } } diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index 9d21afeb96..c7297af7d4 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -81,6 +81,10 @@ private JavaProfiler() { * Get a {@linkplain JavaProfiler} instance backed by the bundled native library and using * the default temp directory as the scratch where the bundled library will be exploded * before linking. + * + *

    This overload expresses no preference about monitor-event ownership: when the + * process-wide instance already exists it is returned unchanged, whatever its + * {@code delegateMonitorWaitEvents} setting is. */ public static JavaProfiler getInstance() throws IOException { return getInstance(null, null); @@ -91,6 +95,10 @@ public static JavaProfiler getInstance() throws IOException { * the given directory as the scratch where the bundled library will be exploded * before linking. * @param scratchDir directory where the bundled library will be exploded before linking + * + *

    This overload expresses no preference about monitor-event ownership: when the + * process-wide instance already exists it is returned unchanged, whatever its + * {@code delegateMonitorWaitEvents} setting is. */ public static JavaProfiler getInstance(String scratchDir) throws IOException { return getInstance(null, scratchDir); @@ -102,8 +110,18 @@ public static JavaProfiler getInstance(String scratchDir) throws IOException { * before linking. * @param libLocation the path to the native library to be used instead of the bundled one * @param scratchDir directory where the bundled library will be exploded before linking; ignored when 'libLocation' is {@literal null} + * + *

    This overload expresses no preference about monitor-event ownership: when the + * process-wide instance already exists it is returned unchanged, whatever its + * {@code delegateMonitorWaitEvents} setting is. Only the explicit three-argument + * overload enforces the ownership-conflict check. */ public static synchronized JavaProfiler getInstance(String libLocation, String scratchDir) throws IOException { + // No preference expressed: an already-initialized singleton is acceptable as-is. + if (instance != null) { + return instance; + } + // 'false' is the default for the *first* initialization only. return getInstance(libLocation, scratchDir, false); } @@ -439,7 +457,8 @@ public void recordQueueTime(long startTicks, * @return {@code true} when this call owns a park interval that must be closed */ boolean parkEnter() { - return parkEnter0(Thread.currentThread()); + Thread thread = Thread.currentThread(); + return parkEnter0(thread, isVirtualThread(thread)); } /** @@ -447,7 +466,8 @@ boolean parkEnter() { * {@code blocker} and {@code unblockingSpanId} are reserved for park instrumentation. */ void parkExit(long blocker, long unblockingSpanId) { - parkExit0(Thread.currentThread(), blocker, unblockingSpanId); + Thread thread = Thread.currentThread(); + parkExit0(thread, isVirtualThread(thread), blocker, unblockingSpanId); } /** @@ -459,14 +479,16 @@ void parkExit(long blocker, long unblockingSpanId) { * @return an opaque token to pass to {@link #blockExit(long)}, or 0 if no state was armed */ long blockEnter(int state) { - return blockEnter0(Thread.currentThread(), state); + Thread thread = Thread.currentThread(); + return blockEnter0(thread, isVirtualThread(thread), state); } /** * Clears a blocked interval previously armed by {@link #blockEnter(int)}. */ void blockExit(long token) { - blockExit0(Thread.currentThread(), token); + Thread thread = Thread.currentThread(); + blockExit0(thread, isVirtualThread(thread), token); } /** @@ -572,13 +594,13 @@ static int getTid() { private static native void recordQueueEnd0(long startTicks, long endTicks, String task, String scheduler, Thread origin, String queueType, int queueLength); - private static native boolean parkEnter0(Thread thread); + private static native boolean parkEnter0(Thread thread, boolean isVirtual); - private static native void parkExit0(Thread thread, long blocker, long unblockingSpanId); + private static native void parkExit0(Thread thread, boolean isVirtual, long blocker, long unblockingSpanId); - private static native long blockEnter0(Thread thread, int state); + private static native long blockEnter0(Thread thread, boolean isVirtual, int state); - private static native void blockExit0(Thread thread, long token); + private static native void blockExit0(Thread thread, boolean isVirtual, long token); private static native long beginTaskBlock0(Thread thread); diff --git a/ddprof-lib/src/test/cpp/vmEntry_ut.cpp b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp index fa740cd17e..e40e20f450 100644 --- a/ddprof-lib/src/test/cpp/vmEntry_ut.cpp +++ b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp @@ -469,13 +469,18 @@ TEST_F(NativeMonitorEventsTest, } } -TEST_F(NativeMonitorEventsTest, - NativeAdmissionRemainsClosedWhenSetupAndRollbackFail) { +// Guards the "leaked JVMTI monitor events" defect: when the enable partially fails +// *and* its own rollback fails, the events stay enabled with no consumer. The disable +// path must therefore retry the teardown unconditionally instead of skipping it because +// the monitor-events flag was already stored false. +TEST_F(NativeMonitorEventsTest, FailedRollbackIsRetriedOnDisable) { fail(JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT); fail_all_disables = true; ProfilerTestAccessor::setTaskBlockEnabled(profiler, true); + // Pre-disable state: the leak is still present, the fix does not repair a failed + // rollback in place. EXPECT_TRUE(profiler->taskBlockEnabled()); EXPECT_FALSE(profiler->nativeMonitorTaskBlockEnabled()); EXPECT_FALSE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); @@ -483,6 +488,26 @@ TEST_F(NativeMonitorEventsTest, EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_CONTENDED_ENTERED)); EXPECT_FALSE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAIT)); EXPECT_TRUE(eventIsEnabled(JVMTI_EVENT_MONITOR_WAITED)); + + // The fixture is sticky across calls, so stop forcing disables to fail explicitly. + inject_failure = false; + fail_all_disables = false; + calls.clear(); + + ProfilerTestAccessor::setTaskBlockEnabled(profiler, false); + + // The disable path retried the teardown and reclaimed the leaked events. + EXPECT_FALSE(profiler->taskBlockEnabled()); + EXPECT_FALSE(ProfilerTestAccessor::monitorEventsEnabled(profiler)); + for (jvmtiEvent event : MONITOR_EVENTS) { + EXPECT_FALSE(eventIsEnabled(event)) << "event " << event << " left enabled"; + } + + // Admission still closes before native teardown. + ASSERT_FALSE(calls.empty()); + for (const EventCall& call : calls) { + EXPECT_FALSE(call.task_block_enabled); + } } TEST_F(NativeMonitorEventsTest, AdmissionClosesBeforeNativeTeardown) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java index 2f0fc69681..f3fa9c3781 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -71,6 +71,7 @@ *

  • profiler-java-default-delegation-conflict - verifies delegated ownership conflicts after default initialization
    • *
  • profiler-java-delegation-reuse:<delegated> - verifies compatible Java singleton ownership reuse
    • *
  • profiler-java-delegation-conflict:<initial>:<requested> - verifies conflicting Java singleton ownership requests
    • + *
  • profiler-java-delegation-legacy-reuse:<initial> - verifies the legacy no-preference overloads reuse the existing singleton
    • *
  • profiler-preexisting-monitor-wait - exercises Object.wait on a thread created before profiler initialization
    • *
  • profiler-preexisting-monitor-contention - exercises monitor contention on a thread created before profiler initialization
    • * @@ -292,6 +293,20 @@ public static void main(String[] args) throws Exception { JavaProfiler reused = JavaProfiler.getInstance(null, null, delegated); System.out.println("[java-delegation-reuse] " + (initial == reused) + " " + reused.isMonitorWaitEventsDelegated()); + } else if (args[0].startsWith("profiler-java-delegation-legacy-reuse:")) { + // A legacy overload expresses no preference about monitor-event ownership: + // it must return the existing singleton whatever its delegation setting is, + // never throw IllegalStateException. An escaping ISE is the failure signal. + boolean initialDelegation = Boolean.parseBoolean(args[0].substring( + "profiler-java-delegation-legacy-reuse:".length())); + JavaProfiler initial = + JavaProfiler.getInstance(null, null, initialDelegation); + JavaProfiler reused = JavaProfiler.getInstance(); + JavaProfiler reused2 = JavaProfiler.getInstance(null, null); + System.out.println("[java-delegation-legacy-reuse] " + + (initial == reused) + " " + + (initial == reused2) + " " + + initial.isMonitorWaitEventsDelegated()); } else if (args[0].startsWith("profiler-java-delegation-conflict:")) { String[] delegationModes = args[0].split(":"); boolean initialDelegation = Boolean.parseBoolean(delegationModes[1]); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java index 02a378d3e2..54be8b9fd5 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java @@ -237,6 +237,36 @@ void compatibleJavaSingletonMonitorDelegationIsReused() throws Exception { assertJavaSingletonDelegationReuse(true); } + @Test + void legacyGetInstanceOverloadsReuseAnyExistingSingleton() throws Exception { + // A legacy overload expresses no preference about monitor-event ownership, so it must + // return the existing singleton instead of throwing - including when that singleton was + // initialized with delegateMonitorWaitEvents=true. + assertJavaSingletonLegacyReuse(true); + assertJavaSingletonLegacyReuse(false); + } + + /** Launches a fresh JVM and verifies the legacy overloads never conflict with the singleton. */ + private void assertJavaSingletonLegacyReuse(boolean initialDelegation) throws Exception { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch( + "profiler-java-delegation-legacy-reuse:" + initialDelegation, + Collections.emptyList(), "", line -> { + if (line.startsWith("[java-delegation-legacy-reuse]")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + // An IllegalStateException escaping the launcher shows up here as a non-zero exit, + // distinguishing a thrown exception from a missing-output flake. + assertEquals(0, result.exitCode); + assertEquals("[java-delegation-legacy-reuse] true true " + initialDelegation, + resultLine.get()); + } + /** Launches a fresh JVM and verifies that repeated ownership returns the same singleton. */ private void assertJavaSingletonDelegationReuse(boolean delegated) throws Exception { AtomicReference resultLine = new AtomicReference<>(); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java index ff6df5c970..f8434f8cab 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java @@ -7,6 +7,7 @@ import com.datadoghq.profiler.Platform; import java.util.Map; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; /** Verifies synchronous monitor production when delegated wall-clock stacks are enabled. */ @@ -27,4 +28,20 @@ protected void withTestAssumptions() { protected String getProfilerCommand() { return "wall=1ms,filter=,wallprecheck=true,jvmtistacks=true"; } + + /** + * Proves the restarted recording really ran with {@code jvmtistacks=true}: JVMTI stacks + * must have been requested again after the restart. Catches a regression back to a + * hardcoded restart command that drops this class's configuration. + * + *

    Starting the restarted recording resets the native counters, so any non-zero count + * here was accumulated by the restarted recording alone. + */ + @Override + protected void assertRestartedConfiguration(Map counters) { + long requested = counters.getOrDefault("jvmti_stacks_requested", 0L); + Assertions.assertTrue(requested > 0, + "restarted recording did not use the JVMTI stack path: jvmti_stacks_requested " + + requested); + } } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java index 25d81b8e4a..967624a18c 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java @@ -9,6 +9,7 @@ import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -78,9 +79,70 @@ public void monitorContentionEmitsTaskBlockOutsideContextWindow() throws Excepti TaskBlockAssertions.assertContainsObservedState(events, "CONTENDED"); } + @Test + public void shortMonitorContentionIsFilteredAndDoesNotSuppressLongerOnes() throws Exception { + Object shortMonitor = new Object(); + Object longMonitor = new Object(); + + // Burst of genuinely contended, microsecond-long enters: two threads hammer the same + // monitor with an empty critical section, so no interval can reach the 1ms threshold. + CountDownLatch start = new CountDownLatch(1); + AtomicReference burstFailure = new AtomicReference<>(); + int[] counter = new int[1]; + Thread[] burst = new Thread[2]; + for (int i = 0; i < burst.length; i++) { + burst[i] = new Thread(() -> { + try { + assertTrue(start.await(5, TimeUnit.SECONDS)); + for (int n = 0; n < 20_000; n++) { + synchronized (shortMonitor) { + counter[0]++; + } + } + } catch (Throwable t) { + burstFailure.set(t); + } + }, "taskblock-short-contention-" + i); + burst[i].start(); + } + start.countDown(); + for (Thread thread : burst) { + assertCompleted(thread, burstFailure); + } + + // One long contended enter, the positive control: it proves the producer was alive. + CountDownLatch attempting = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Thread worker; + synchronized (longMonitor) { + worker = new Thread(() -> { + try { + attempting.countDown(); + synchronized (longMonitor) { + } + } catch (Throwable t) { + failure.set(t); + } + }, "taskblock-long-contention"); + worker.start(); + assertTrue(attempting.await(5, TimeUnit.SECONDS)); + Thread.sleep(100); + } + assertCompleted(worker, failure); + stopProfiler(); + + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertTrue(TaskBlockAssertions.containsBlocker(events, identityHash(longMonitor)), + "long contention was not emitted"); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(shortMonitor)), + "sub-threshold contention must be filtered"); + assertTaskBlockStackReference(events); + } + @Test public void contextWindowObjectWaitDoesNotEmitTaskBlock() throws Exception { Object monitor = new Object(); + Object controlMonitor = new Object(); AtomicReference failure = new AtomicReference<>(); Thread worker = new Thread(() -> { try { @@ -99,10 +161,29 @@ public void contextWindowObjectWaitDoesNotEmitTaskBlock() throws Exception { worker.start(); assertCompleted(worker, failure); + + // Positive control: an untraced platform-thread wait in the same recording must be + // produced, so this test also fails when the producer stops emitting anything at all. + AtomicReference controlFailure = new AtomicReference<>(); + Thread control = new Thread(() -> { + try { + synchronized (controlMonitor) { + controlMonitor.wait(100); + } + } catch (Throwable t) { + controlFailure.set(t); + } + }, "taskblock-control-object-wait"); + control.start(); + assertCompleted(control, controlFailure); + stopProfiler(); - assertFalse(TaskBlockAssertions.containsBlocker( - verifyEvents("datadog.TaskBlock", false), identityHash(monitor))); + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertTrue(TaskBlockAssertions.containsBlocker(events, identityHash(controlMonitor)), + "control wait was not produced"); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(monitor)), + "traced wait was not suppressed"); } @Test @@ -141,8 +222,10 @@ public void staleWaitStateIsRecoveredAfterProfilerRestart() throws Exception { Path recording = Files.createTempFile("MonitorTaskBlockTest-restart-", ".jfr"); boolean restarted = false; + // Built from getProfilerCommand() so subclasses exercise their own configuration on the + // restarted recording too, not just on the initial one. try { - profiler.execute("start,wall=1ms,filter=,wallprecheck=true,jfr,file=" + profiler.execute("start," + getProfilerCommand() + ",jfr,file=" + recording.toAbsolutePath()); restarted = true; synchronized (contentionMonitor) { @@ -158,6 +241,7 @@ public void staleWaitStateIsRecoveredAfterProfilerRestart() throws Exception { assertTaskBlockStackReference(events); assertTrue(TaskBlockAssertions.containsBlocker( events, identityHash(contentionMonitor))); + assertRestartedConfiguration(profiler.getDebugCounters()); } finally { restartReady.countDown(); synchronized (waitMonitor) { @@ -209,9 +293,34 @@ public void virtualMonitorCallbacksDoNotEmitCarrierTaskBlocks() throws Exception Thread.sleep(100); } assertCompleted(contender, failure); + + // Positive control: the same contention shape on a platform thread must be produced, + // so this test fails when carrier suppression breaks *and* when production breaks. + Object platformMonitor = new Object(); + CountDownLatch platformAttempting = new CountDownLatch(1); + AtomicReference platformFailure = new AtomicReference<>(); + Thread platformContender; + synchronized (platformMonitor) { + platformContender = new Thread(() -> { + try { + platformAttempting.countDown(); + synchronized (platformMonitor) { + } + } catch (Throwable t) { + platformFailure.set(t); + } + }, "taskblock-control-monitor-contention"); + platformContender.start(); + assertTrue(platformAttempting.await(5, TimeUnit.SECONDS)); + Thread.sleep(100); + } + assertCompleted(platformContender, platformFailure); + stopProfiler(); - JfrEvents events = verifyEvents("datadog.TaskBlock", false); + JfrEvents events = verifyEvents("datadog.TaskBlock"); + assertTrue(TaskBlockAssertions.containsBlocker(events, identityHash(platformMonitor)), + "platform control was not produced"); assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(waitMonitor))); assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(contentionMonitor))); } @@ -221,6 +330,17 @@ protected String getProfilerCommand() { return "wall=1ms,filter=,wallprecheck=true"; } + /** + * Hook for subclasses to assert that the restarted recording ran under their own + * configuration. No-op here so the base class stays configuration-agnostic. + * + *

    The restart's {@code start,...} command resets the native debug counters, so + * {@code counters} only accumulates over the restarted recording: subclasses can assert + * absolute values rather than deltas against a pre-restart baseline. + */ + protected void assertRestartedConfiguration(Map counters) { + } + protected void assertTaskBlockStackReference(JfrEvents events) { TaskBlockAssertions.assertContainsStackTrace(events); TaskBlockAssertions.assertContainsJavaType(events, "MonitorTaskBlockTest"); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java index a1c288be5b..e169447ad2 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java @@ -85,6 +85,8 @@ public void virtualParkDoesNotMutateCarrierProducerState() throws Exception { virtual.join(5_000); assertFalse(virtual.isAlive()); + // Positive control on a platform thread, well above the 1ms threshold: proves the park + // producer is alive, so the virtual-thread short-circuit assertion is not vacuous. ProfilerOwnedBlockHooks.parkEnter(profiler); try { parkForMillis(200); @@ -94,7 +96,10 @@ public void virtualParkDoesNotMutateCarrierProducerState() throws Exception { stopProfiler(); JfrEvents events = verifyEvents("datadog.TaskBlock"); - assertFalse(TaskBlockAssertions.containsBlocker(events, virtualBlocker)); + assertTrue(TaskBlockAssertions.containsBlocker(events, BLOCKER), + "platform control park was not produced"); + assertFalse(TaskBlockAssertions.containsBlocker(events, virtualBlocker), + "virtual-thread park must not reach the carrier producer"); TaskBlockAssertions.assertContains(events, 0, 0, BLOCKER, UNBLOCKING_SPAN_ID); } From b7784bddc8621ad249381b5abc0c66d25c78be93 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 26 Aug 2026 10:37:51 +0200 Subject: [PATCH 3/9] Arm OBJECT_WAIT in the generic blockEnter TaskBlock hook decodeJavaBlockState only recognized SLEEPING, so delegated Object.wait intervals could never be armed via blockEnter/blockExit even though delegateMonitorWaitEvents documents Java instrumentation as owning them. Co-Authored-By: Claude Sonnet 5 --- ddprof-lib/src/main/cpp/javaApi.cpp | 4 ++++ .../java/com/datadoghq/profiler/JavaProfiler.java | 2 +- .../datadoghq/profiler/wallclock/PrecheckTest.java | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 65ef12313f..da9e813caf 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -504,6 +504,10 @@ static bool decodeJavaBlockState(jint state, OSThreadState &decoded) { decoded = OSThreadState::SLEEPING; return true; } + if (state == static_cast(OSThreadState::OBJECT_WAIT)) { + decoded = OSThreadState::OBJECT_WAIT; + return true; + } decoded = OSThreadState::UNKNOWN; return false; } diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index c7297af7d4..65bab51fe6 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -475,7 +475,7 @@ void parkExit(long blocker, long unblockingSpanId) { * blocked interval. The public paired API is {@link #beginTaskBlock()}. * * @param state native {@code OSThreadState} value for the blocked interval; - * currently only {@code SLEEPING} is armed + * currently {@code SLEEPING} and {@code OBJECT_WAIT} are armed * @return an opaque token to pass to {@link #blockExit(long)}, or 0 if no state was armed */ long blockEnter(int state) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java index 0a4d2191f3..c8d80f26ea 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java @@ -30,6 +30,7 @@ public class PrecheckTest extends AbstractProfilerTest { private static final int OSTHREAD_STATE_SLEEPING = 7; private static final int POOL_WORKERS = 4; private static final long POOL_SLEEP_MILLIS = 300; + private static final int OSTHREAD_STATE_OBJECT_WAIT = 5; private static final String TAIL_WEIGHT_THREAD = "precheck-tail-weight"; private static final int TAIL_WEIGHT_ITERATIONS = 50; private static final int TAIL_WEIGHT_SLEEP_MILLIS = 6; @@ -65,6 +66,17 @@ public void testSleepingThreadIsNotSampled() throws InterruptedException { } } + @Test + public void testBlockEnterArmsObjectWaitState() { + Assumptions.assumeTrue(!Platform.isJ9()); + Assumptions.assumeTrue(Platform.isJavaVersionAtLeast(11)); + leaveClearedInitializedContext(); + + long token = ProfilerOwnedBlockHooks.blockEnter(profiler, OSTHREAD_STATE_OBJECT_WAIT); + assertTrue(token != 0, "Expected native blockEnter to arm OBJECT_WAIT state"); + ProfilerOwnedBlockHooks.blockExit(profiler, token); + } + @Test public void testBlockEnterRejectedWithActiveTraceContext() { Assumptions.assumeTrue(!Platform.isJ9()); From a683dd24ac1e3520f9610e659d209304ae0a7da5 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 1 Sep 2026 17:56:28 +0200 Subject: [PATCH 4/9] Address review feedback for taskblock JVM producers Fixes 24 findings across TaskBlock lifecycle, ThreadFilter slot management, and monitor/park hooks; includes stale-slot recovery for FLAG_TASK_BLOCKED so a restarted profiler doesn't refuse new blocked intervals on threads whose previous interval was abandoned mid-run. --- .../native/config/ConfigurationPresets.kt | 16 -- ddprof-lib/src/main/cpp/counters.h | 1 + ddprof-lib/src/main/cpp/javaApi.cpp | 89 +++++++-- ddprof-lib/src/main/cpp/jvmSupport.cpp | 7 +- ddprof-lib/src/main/cpp/profiler.cpp | 16 +- ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | 147 +++++++++++++++ ddprof-lib/src/main/cpp/taskBlockRecorder.h | 18 ++ ddprof-lib/src/main/cpp/threadLocalData.cpp | 7 +- ddprof-lib/src/main/cpp/threadLocalData.h | 173 +++++++++-------- ddprof-lib/src/main/cpp/vmEntry.cpp | 176 +++--------------- ddprof-lib/src/main/cpp/vmEntry.h | 31 ++- .../com/datadoghq/profiler/JavaProfiler.java | 20 +- ddprof-lib/src/test/cpp/vmEntry_ut.cpp | 10 +- .../WallClockPrecheckBenchmarkHooks.java | 2 +- .../profiler/JavaProfilerApiSurfaceTest.java | 3 +- .../profiler/ProfilerOwnedBlockHooks.java | 7 +- 16 files changed, 438 insertions(+), 285 deletions(-) diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt index 9201177860..29b57b47c8 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt @@ -1,19 +1,3 @@ -/* - * 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. - */ - package com.datadoghq.native.config import com.datadoghq.native.model.Architecture diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index b0f98e3ae4..5c10667622 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -106,6 +106,7 @@ X(TASK_BLOCK_DROPPED_ROTATION, "task_block_dropped_rotation") \ X(TASK_BLOCK_SKIPPED_THREAD_MISMATCH, "task_block_skipped_thread_mismatch") \ X(TASK_BLOCK_ROTATION_TIMEOUT, "task_block_rotation_timeout") \ + X(TASK_BLOCK_VIRTUAL_THREAD_DETECTION_FAILED, "task_block_virtual_thread_detection_failed") \ X(UNWINDING_TIME_ASYNC, "unwinding_ticks_async") \ X(UNWINDING_TIME_JVMTI, "unwinding_ticks_jvmti") \ X(CALLTRACE_STORAGE_DROPPED, "calltrace_storage_dropped_traces") \ diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index da9e813caf..52e0432217 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -83,6 +83,9 @@ Java_com_datadoghq_profiler_JavaProfiler_init0( ProfilerBridgeInitResult result = VM::initProfilerBridge(nullptr, true, delegateMonitorWaitEvents); if (result == ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT) { + // Keep this message identical to the one JavaProfiler.getInstance(String, String, + // boolean) throws for the analogous Java-singleton conflict; there is no shared + // constant across the JNI boundary, so both call sites must be updated together. throwNew(env, "java/lang/IllegalStateException", "Monitor-event ownership conflicts with the profiler's " "process-wide initialization"); @@ -455,15 +458,18 @@ Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( if (current == nullptr) { return JNI_FALSE; } + Profiler *profiler = Profiler::instance(); + ThreadFilter *tf = profiler->threadFilter(); + if (!tf->registryActive() || + !(profiler->taskBlockEnabled() || tf->enabled())) { + return JNI_FALSE; + } Context context = ContextApi::snapshot(); if (!current->parkEnter(TSC::ticks(), context)) { return JNI_FALSE; } - Profiler *profiler = Profiler::instance(); - ThreadFilter *tf = profiler->threadFilter(); - if (context.spanId == 0 && tf->registryActive() && - (profiler->taskBlockEnabled() || tf->enabled())) { + if (context.spanId == 0) { ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); if (slot_id >= 0) { WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); @@ -528,8 +534,7 @@ static bool isCurrentJniThread(JNIEnv* env, jthread thread) { extern "C" DLLEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( - JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual, - jint state) { + JNIEnv *env, jclass unused, jboolean isVirtual, jint state) { OSThreadState decoded; if (!decodeJavaBlockState(state, decoded) || isVirtual != JNI_FALSE) { return 0; @@ -538,28 +543,66 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( if (current == nullptr) { return 0; } - u64 span_id = 0, root_span_id = 0; - ContextApi::get(span_id, root_span_id); - if (span_id != 0) { + Context context = ContextApi::snapshot(); + if (context.spanId != 0) { return 0; } Profiler *profiler = Profiler::instance(); ThreadFilter *tf = profiler->threadFilter(); - if (!profiler->taskBlockEnabled() && !tf->registryActive()) { + bool task_block_armable = profiler->taskBlockEnabled(); + if (!task_block_armable && !tf->registryActive()) { return 0; } ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); - if (slot_id < 0) { - return 0; - } + if (slot_id < 0) return 0; WallClockBlockTracker *tracker = profiler->blockTracker(); - return static_cast(tracker->enterBlockedRun(tf, slot_id, decoded)); + u64 token = tracker->enterBlockedRun(tf, slot_id, decoded); + if (token == 0) return 0; + // Also arm the TaskBlock recording slot so blockExit0 can produce an event + // with blocker identity, matching the beginTaskBlock0/endTaskBlock0 pair. + // Skipped when TaskBlock recording is disabled so this stays purely a + // wall-clock suppression marker for the precheck tests that exercise it + // with task-block recording off. + if (task_block_armable && + !current->taskBlockEnter(token, TSC::ticks(), context)) { + // FLAG_TASK_BLOCKED is sticky across recordings (unlike the WallClockBlockTracker + // blocked-run, which is generation-scoped), so a worker whose previous + // interval was abandoned mid-run (e.g. profiler stop/restart without a + // matching blockExit0) can still be holding it. Mirror monitorBlockEnter's + // stale-slot check: only refuse the new interval if the old token still + // maps to a genuinely active, same-generation blocked run. + u64 stale_token = current->taskBlockToken(); + bool current_owner = false; + if (stale_token != 0) { + ThreadFilter::SlotID stale_slot_id = WallClockBlockTracker::tokenSlotId(stale_token); + ThreadFilter::Slot *stale_slot = current->filterSlotId() == stale_slot_id + ? tf->activeSlotForId(stale_slot_id, current->tid()) + : nullptr; + if (stale_slot != nullptr) { + WallClockBlockTracker::BlockState *block_state = tracker->slotForId(stale_slot_id); + current_owner = block_state != nullptr && + block_state->activeBlockState() != OSThreadState::UNKNOWN && + block_state->activeBlockOwner() == BlockRunOwner::JAVA && + block_state->blockGeneration() == WallClockBlockTracker::tokenGeneration(stale_token); + } + } + if (current_owner) { + tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(token)); + return 0; + } + current->clearTaskBlock(); + if (!current->taskBlockEnter(token, TSC::ticks(), context)) { + tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(token)); + return 0; + } + } + return static_cast(token); } extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0( - JNIEnv *env, jclass unused, jthread thread, jboolean isVirtual, - jlong token) { + JNIEnv *env, jclass unused, jboolean isVirtual, jlong token, jlong blocker, + jlong unblockingSpanId) { u64 block_token = static_cast(token); if (block_token == 0 || isVirtual != JNI_FALSE) { return; @@ -575,6 +618,20 @@ Java_com_datadoghq_profiler_JavaProfiler_blockExit0( tf->activeSlotForId(slot_id, current->tid()) == nullptr) { return; } + + u64 start_ticks = 0; + Context context{}; + if (current->taskBlockExit(block_token, start_ticks, context)) { + // thread=nullptr: GetStackTrace treats NULL as "the calling thread", which + // is always correct here since blockExit0 only ever runs on the blocked + // thread itself. + finishTaskBlockAtExit(current, tf, Profiler::instance()->blockTracker(), + /*thread=*/nullptr, 1, block_token, + start_ticks, context, static_cast(blocker), + static_cast(unblockingSpanId)); + return; + } + if (tf->registryActive()) { WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(block_token)); diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 26addfe36e..2a13bcf6eb 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -7,6 +7,7 @@ #include "asyncSampleMutex.h" #include "common.h" +#include "counters.h" #include "frames.h" #include "os.h" #include "profiler.h" @@ -51,12 +52,14 @@ bool JVMSupport::isPlatformThread(JNIEnv* jni, jthread thread) { reinterpret_cast( functions[IS_VIRTUAL_THREAD_INDEX]); if (is_virtual_thread == nullptr) { + Counters::increment(TASK_BLOCK_VIRTUAL_THREAD_DETECTION_FAILED); static std::atomic warning_emitted{false}; bool expected = false; if (warning_emitted.compare_exchange_strong(expected, true, std::memory_order_relaxed)) { - LOG_WARN("JNI version 19 or later does not expose IsVirtualThread; " - "JVM producer callbacks will be ignored"); + LOG_WARN("Failed to resolve IsVirtualThread from the JNI function table " + "even though JNI version is 19 or later; JVM producer callbacks " + "will be ignored"); } return false; } diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 87c56e9b84..d296bf8c6c 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1627,9 +1627,7 @@ void Profiler::setTaskBlockEnabled(bool enabled) { if (enabled) { // Keep callback admission closed until native setup has either completed // or rolled back, so partial event enablement cannot create paired state. - bool monitor_events_enabled = - VM::nativeMonitorEventsAvailable() && - VM::setNativeMonitorEventsEnabled(true); + bool monitor_events_enabled = setNativeMonitorTaskBlockEventsEnabled(true); _task_block_monitor_events_enabled.store(monitor_events_enabled, std::memory_order_release); _task_block_enabled.store(true, std::memory_order_release); @@ -1640,10 +1638,10 @@ void Profiler::setTaskBlockEnabled(bool enabled) { // Clear the admission flag first so no consumer can observe enabled events, then // always attempt teardown. A previous enable whose setup AND rollback both failed // left the flag false while JVMTI events stayed on; retrying unconditionally is the - // only way that leak is ever reclaimed. setNativeMonitorEventsEnabled(false) is + // only way that leak is ever reclaimed. setNativeMonitorTaskBlockEventsEnabled(false) is // documented as a no-op when the capability was never enabled. _task_block_monitor_events_enabled.exchange(false, std::memory_order_acq_rel); - VM::setNativeMonitorEventsEnabled(false); + setNativeMonitorTaskBlockEventsEnabled(false); } Error Profiler::start(Arguments &args, bool reset) { @@ -2310,10 +2308,10 @@ Error Profiler::dump(const char *path, const int length) { Error err = Error::OK; // rotateDictsAndRun rotates the dictionaries, takes lockAll() around the // dump (fences ASGCT/JNI writers to CallTraceStorage), then clearStandby()s - // the rotated buffers. StringDictionary's RefCountGuard protocol handles - // its own writer/reader coordination; #527's classMapSharedGuard readers - // (deferred vtable receiver resolution) are coordinated through - // _class_map_lock. + // the rotated buffers. StringDictionary's rotate()/standby()/clearStandby() + // protocol handles the dump's own writer/reader coordination for each + // dictionary, including _class_map (see flightRecorder.cpp's writeCpool: + // classMap()->standby() stays stable for the dump's lifetime). if (beginTaskBlockRotation()) { rotateDictsAndRun([&]{ err = _jfr.dump(path, length); diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp index c6d61d65f7..5ecce508ba 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp @@ -5,6 +5,11 @@ #include "taskBlockRecorder.h" +#include "context_api.h" +#include "jvmSupport.h" +#include "threadLocalData.inline.h" +#include "tsc.h" + #include static const u64 kMinTaskBlockNanos = 1000000; @@ -80,3 +85,145 @@ bool finishTaskBlockAtExit(ProfiledThread* current, current->tid(), thread, start_depth, start_ticks, end_ticks, context, blocker, unblocking_span_id, snapshot.active_state, true); } + +static u64 monitorBlockerHash(jvmtiEnv *jvmti, jobject object) { + if (object == NULL) return 0; + jint hash = 0; + if (jvmti->GetObjectHashCode(object, &hash) != JVMTI_ERROR_NONE) return 0; + return static_cast(static_cast(hash)); +} + +// Deliberately takes no jvmtiEnv: no JVMTI call may run on this hot path. The +// blocker identity hash is resolved lazily in monitorBlockExit, and only for +// intervals that pass the minimum-duration filter (GetObjectHashCode mutates the +// object's mark word on HotSpot). +static void monitorBlockEnter(JNIEnv *jni, jthread thread, + OSThreadState state) { + Profiler *profiler = Profiler::instance(); + if (!profiler->taskBlockEnabled() || + !profiler->nativeMonitorTaskBlockEnabled() || + !JVMSupport::isPlatformThread(jni, thread)) { + return; + } + ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); + if (current == nullptr) return; + Context context = ContextApi::snapshot(); + if (context.spanId != 0) { + Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); + return; + } + + if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { + u64 token = current->monitorBlockToken(); + ThreadFilter *tf = profiler->threadFilter(); + bool current_owner = false; + if (token != 0) { + ThreadFilter::SlotID slot_id = WallClockBlockTracker::tokenSlotId(token); + ThreadFilter::Slot *slot = current->filterSlotId() == slot_id + ? tf->activeSlotForId(slot_id, current->tid()) + : nullptr; + if (slot != nullptr) { + WallClockBlockTracker::BlockState *block_state = profiler->blockTracker()->slotForId(slot_id); + current_owner = block_state != nullptr && + block_state->activeBlockState() != OSThreadState::UNKNOWN && + block_state->activeBlockOwner() == BlockRunOwner::JVMTI && + block_state->blockGeneration() == WallClockBlockTracker::tokenGeneration(token); + } + } + if (current_owner) { + return; + } + current->clearMonitorBlock(); + if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { + return; + } + } + + ThreadFilter *tf = profiler->threadFilter(); + ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); + if (!tf->unfilteredWallTrackingActive() || slot_id < 0) { + current->clearMonitorBlock(); + return; + } + u64 token = + profiler->blockTracker()->enterBlockedRun(tf, slot_id, state, BlockRunOwner::JVMTI); + if (token == 0) { + ThreadFilter::Slot *slot = tf->slotForId(slot_id); + if (slot != nullptr && slot->inContextWindow()) { + Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); + } + current->clearMonitorBlock(); + return; + } + current->setMonitorBlockToken(token); +} + +static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, OSThreadState state) { + Profiler *profiler = Profiler::instance(); + if (!profiler->taskBlockEnabled() || !profiler->nativeMonitorTaskBlockEnabled()) { + return; + } + if (!JVMSupport::isPlatformThread(jni, thread)) return; + ProfiledThread *current = ProfiledThread::current(); + if (current == nullptr) return; + + u64 start_ticks = 0; + Context context{}; + // The entry side no longer records a blocker; it is resolved lazily below. + u64 blocker = 0; + u64 token = 0; + if (!current->monitorExit(state, start_ticks, context, blocker, token) || + token == 0) { + return; + } + + // Resolve the blocker identity hash only for intervals that will actually pass + // the eligibility filter. GetObjectHashCode mutates the object's mark word on + // HotSpot, so it must not run for short, high-frequency contention that gets + // discarded anyway. That mutation also permanently disables biased locking for + // this object, a cost borne by the profiled application, not just the profiler. + // These conditions mirror taskBlockPassesBasicEligibility, and the same + // end_ticks is handed down so there is no boundary drift. + u64 end_ticks = TSC::ticks(); + if (context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { + blocker = monitorBlockerHash(jvmti, object); + } + + finishTaskBlockAtExit(current, profiler->threadFilter(), + profiler->blockTracker(), thread, 0, token, + start_ticks, context, blocker, 0, end_ticks); +} + +void JNICALL MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *jni, + jthread thread, jobject object) { + monitorBlockEnter(jni, thread, OSThreadState::MONITOR_WAIT); +} + +void JNICALL MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *jni, + jthread thread, jobject object) { + monitorBlockExit(jvmti, jni, thread, object, OSThreadState::MONITOR_WAIT); +} + +void JNICALL MonitorWait(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jlong timeout) { + if (!VM::monitorWaitEventsDelegated()) { + monitorBlockEnter(jni, thread, OSThreadState::OBJECT_WAIT); + } +} + +void JNICALL MonitorWaited(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jboolean timed_out) { + if (!VM::monitorWaitEventsDelegated()) { + monitorBlockExit(jvmti, jni, thread, object, OSThreadState::OBJECT_WAIT); + } +} + +bool setNativeMonitorTaskBlockEventsEnabled(bool enabled) { + if (enabled) { + return VM::nativeMonitorEventsAvailable() && + VM::setNativeMonitorEventsEnabled(true); + } + VM::setNativeMonitorEventsEnabled(false); + return false; +} diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.h b/ddprof-lib/src/main/cpp/taskBlockRecorder.h index d8246b3973..4cef7e74cf 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -35,6 +35,24 @@ bool finishTaskBlockAtExit(ProfiledThread* current, const Context& context, u64 blocker, u64 unblocking_span_id, u64 end_ticks = 0); +// JVMTI event callbacks for the native monitor-contention/Object.wait task-block +// producer. Registered directly into jvmtiEventCallbacks by vmEntry.cpp. +void JNICALL MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object); +void JNICALL MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *jni, + jthread thread, jobject object); +void JNICALL MonitorWait(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jlong timeout); +void JNICALL MonitorWaited(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, + jobject object, jboolean timed_out); + +// Enables/disables the native monitor-contention/Object.wait JVMTI events that +// back the MonitorContendedEnter/Entered/Wait/Waited callbacks above. Returns +// whether the events ended up enabled after the attempt; callers use this to +// gate their own admission flag rather than assuming success. Disabling is a +// no-op when the capability was never enabled. +bool setNativeMonitorTaskBlockEventsEnabled(bool enabled); + class TaskBlockActivity { private: Profiler* _profiler; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index ab794b8505..d6f3c1ae56 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -169,10 +169,9 @@ void ProfiledThread::unclaimAndReset() { _wall_epoch = 0; _call_trace_id = 0; _recording_epoch = 0; - _park_block_token = 0; - _task_block_start_ticks = 0; - _task_block_token = 0; - _task_block_context = {}; + _park_slot = {}; + _task_block_slot = {}; + _monitor_slot = {}; _filter_slot_id = -1; _init_window = 0; _signal_depth = 0; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index efe6861ebd..fb3f129cc9 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -59,6 +59,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_MONITOR_BLOCKED = 0x10u; + static constexpr u32 FLAG_TASK_BLOCKED = 0x20u; // We are allowing several levels of nesting because we can be // eg. in a crash handler when wallclock signal kicks in, @@ -88,17 +89,20 @@ class ProfiledThread : public ThreadLocalData { u64 _call_trace_id; u32 _recording_epoch; volatile u32 _misc_flags; - u64 _park_start_ticks; - u64 _park_block_token; - Context _park_context; - u64 _task_block_start_ticks; - u64 _task_block_token; - Context _task_block_context; - u64 _monitor_start_ticks; - Context _monitor_context; - u64 _monitor_blocker; - u64 _monitor_block_token; - OSThreadState _monitor_block_state; + // Shared payload for the blocked-interval state machines (park/generic + // task-block/monitor). Each machine owns a dedicated FLAG_* bit in + // _misc_flags that gates access to its slot; see blockSlotEnter()/ + // blockSlotExit() below. + struct BlockSlot { + u64 start_ticks{0}; + Context context{}; + u64 blocker{0}; + u64 block_token{0}; + OSThreadState block_state{OSThreadState::UNKNOWN}; + }; + BlockSlot _park_slot; + BlockSlot _task_block_slot; + BlockSlot _monitor_slot; int _filter_slot_id; // Slot ID for thread filtering uint8_t _init_window; // Countdown for JVM thread init race window (PROF-13072) volatile int _signal_depth; // Nested signal-handler depth (see SignalHandlerScope) @@ -121,10 +125,7 @@ class ProfiledThread : public ThreadLocalData { 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(0), - _park_start_ticks(0), _park_block_token(0), _park_context{}, - _task_block_start_ticks(0), _task_block_token(0), _task_block_context{}, - _monitor_start_ticks(0), _monitor_context{}, _monitor_blocker(0), - _monitor_block_token(0), _monitor_block_state(OSThreadState::UNKNOWN), + _park_slot{}, _task_block_slot{}, _monitor_slot{}, _filter_slot_id(-1), _init_window(0), _signal_depth(0), @@ -228,8 +229,8 @@ class ProfiledThread : public ThreadLocalData { __atomic_load_n(&_crash_depth, __ATOMIC_RELAXED), _cpu_epoch, _wall_epoch, _call_trace_id, _recording_epoch, __atomic_load_n(&_misc_flags, __ATOMIC_RELAXED), - _park_block_token, _task_block_start_ticks, _task_block_token, - _task_block_context, _filter_slot_id, _init_window, + _park_slot.block_token, _task_block_slot.start_ticks, _task_block_slot.block_token, + _task_block_slot.context, _filter_slot_id, _init_window, __atomic_load_n(&_signal_depth, __ATOMIC_RELAXED), _in_critical_section, _otel_ctx_initialized, _otel_local_root_span_id }; @@ -430,17 +431,8 @@ class ProfiledThread : public ThreadLocalData { } inline bool parkEnter(u64 start_ticks, const Context& context) { - u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); - while ((flags & FLAG_PARKED) == 0) { - _park_start_ticks = start_ticks; - _park_context = context; - if (__atomic_compare_exchange_n(&_misc_flags, &flags, - flags | FLAG_PARKED, true, - __ATOMIC_RELEASE, __ATOMIC_ACQUIRE)) { - return true; - } - } - return false; + return blockSlotEnter(_park_slot, start_ticks, context, + /*blocker=*/0, OSThreadState::UNKNOWN); } #ifdef UNIT_TEST @@ -448,38 +440,44 @@ class ProfiledThread : public ThreadLocalData { #endif inline void setParkBlockToken(u64 token) { - _park_block_token = token; + _park_slot.block_token = token; } inline bool taskBlockEnter(u64 token, u64 start_ticks, const Context& context) { - if (token == 0 || _task_block_token != 0) return false; - _task_block_start_ticks = start_ticks; - _task_block_context = context; - _task_block_token = token; + if (token == 0) return false; + if (!blockSlotEnter(_task_block_slot, start_ticks, + context, /*blocker=*/0, + OSThreadState::UNKNOWN)) { + return false; + } + _task_block_slot.block_token = token; return true; } inline bool taskBlockExit(u64 token, u64& start_ticks, Context& context) { - if (token == 0 || _task_block_token != token) return false; - start_ticks = _task_block_start_ticks; - context = _task_block_context; - _task_block_token = 0; - return true; + if (token == 0 || _task_block_slot.block_token != token) return false; + u64 blocker_unused, token_unused; + return blockSlotExit(_task_block_slot, + OSThreadState::UNKNOWN, + start_ticks, context, + blocker_unused, token_unused); + } + + inline u64 taskBlockToken() const { return _task_block_slot.block_token; } + + inline void clearTaskBlock() { + __atomic_fetch_and(&_misc_flags, ~FLAG_TASK_BLOCKED, __ATOMIC_ACQ_REL); + _task_block_slot.block_token = 0; } // Returns false if the thread was not parked (idempotent). inline bool parkExit(u64& start_ticks, Context& context, u64& park_block_token) { - u32 prev = __atomic_fetch_and(&_misc_flags, ~FLAG_PARKED, __ATOMIC_ACQ_REL); - if ((prev & FLAG_PARKED) == 0) { - return false; - } - start_ticks = _park_start_ticks; - context = _park_context; - park_block_token = _park_block_token; - _park_block_token = 0; - return true; + u64 blocker_unused; + return blockSlotExit(_park_slot, OSThreadState::UNKNOWN, + start_ticks, context, blocker_unused, + park_block_token); } #ifdef UNIT_TEST @@ -494,52 +492,79 @@ class ProfiledThread : public ThreadLocalData { // reacquisition. A nested contention callback must not overwrite that state. inline bool monitorEnter(u64 start_ticks, const Context& context, u64 blocker, OSThreadState state) { - u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); - if ((flags & FLAG_MONITOR_BLOCKED) != 0) return false; - _monitor_start_ticks = start_ticks; - _monitor_context = context; - _monitor_blocker = blocker; - _monitor_block_token = 0; - _monitor_block_state = state; - __atomic_fetch_or(&_misc_flags, FLAG_MONITOR_BLOCKED, __ATOMIC_RELEASE); - return true; + return blockSlotEnter(_monitor_slot, start_ticks, + context, blocker, state); } inline void setMonitorBlockToken(u64 token) { - _monitor_block_token = token; + _monitor_slot.block_token = token; } - inline u64 monitorBlockToken() const { return _monitor_block_token; } + inline u64 monitorBlockToken() const { return _monitor_slot.block_token; } inline void clearMonitorBlock() { __atomic_fetch_and(&_misc_flags, ~FLAG_MONITOR_BLOCKED, __ATOMIC_ACQ_REL); - _monitor_block_token = 0; - _monitor_block_state = OSThreadState::UNKNOWN; + _monitor_slot.block_token = 0; + _monitor_slot.block_state = OSThreadState::UNKNOWN; } inline bool monitorExit(OSThreadState expected_state, u64& start_ticks, Context& context, u64& blocker, u64& monitor_block_token) { + return blockSlotExit(_monitor_slot, expected_state, + start_ticks, context, blocker, + monitor_block_token); + } + + Context snapshotContext(size_t numAttrs); + +private: + // Shared enter/exit for the blocked-interval state machines above. FLAG is + // the caller's dedicated bit in _misc_flags; expected_state == UNKNOWN in + // blockSlotExit() means "don't gate the clear on the stored state" (only + // monitorExit needs that gate, to avoid a nested contention callback + // clobbering an in-progress Object.wait interval). + template + inline bool blockSlotEnter(BlockSlot& slot, u64 start_ticks, + const Context& context, u64 blocker, + OSThreadState state) { + u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); + while ((flags & FLAG) == 0) { + slot.start_ticks = start_ticks; + slot.context = context; + slot.blocker = blocker; + slot.block_token = 0; + slot.block_state = state; + if (__atomic_compare_exchange_n(&_misc_flags, &flags, flags | FLAG, + true, __ATOMIC_RELEASE, + __ATOMIC_ACQUIRE)) { + return true; + } + } + return false; + } + + template + inline bool blockSlotExit(BlockSlot& slot, OSThreadState expected_state, + u64& start_ticks, Context& context, u64& blocker, + u64& block_token) { u32 flags = __atomic_load_n(&_misc_flags, __ATOMIC_ACQUIRE); - if ((flags & FLAG_MONITOR_BLOCKED) == 0 || - _monitor_block_state != expected_state) { + if ((flags & FLAG) == 0) return false; + if (expected_state != OSThreadState::UNKNOWN && + slot.block_state != expected_state) { return false; } - u32 prev = __atomic_fetch_and(&_misc_flags, ~FLAG_MONITOR_BLOCKED, - __ATOMIC_ACQ_REL); - if ((prev & FLAG_MONITOR_BLOCKED) == 0) return false; - start_ticks = _monitor_start_ticks; - context = _monitor_context; - blocker = _monitor_blocker; - monitor_block_token = _monitor_block_token; - _monitor_block_token = 0; - _monitor_block_state = OSThreadState::UNKNOWN; + u32 prev = __atomic_fetch_and(&_misc_flags, ~FLAG, __ATOMIC_ACQ_REL); + if ((prev & FLAG) == 0) return false; + start_ticks = slot.start_ticks; + context = slot.context; + blocker = slot.blocker; + block_token = slot.block_token; + slot.block_token = 0; + slot.block_state = OSThreadState::UNKNOWN; return true; } - Context snapshotContext(size_t numAttrs); - -private: // Atomic flag for signal handler reentrancy protection within the same thread // Must be atomic because a signal handler can interrupt normal execution mid-instruction, // and both contexts may attempt to enter the critical section. Without atomic exchange(), diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 8cbdd82869..b67c863e36 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -54,14 +54,14 @@ bool VM::_hotspot = false; bool VM::_zing = false; bool VM::_can_sample_objects = false; bool VM::_can_intercept_binding = false; -bool VM::_monitor_wait_events_delegated = false; -bool VM::_native_monitor_events_available = false; -bool VM::_profiler_bridge_initialized = false; +NegotiatedSetting VM::_monitor_wait_events_delegated{}; +std::atomic VM::_native_monitor_events_available{false}; +std::atomic VM::_profiler_bridge_initialized{false}; bool VM::_is_adaptive_gc_boundary_flag_set = false; // Serializes the one-time bridge installation and ownership negotiation. -// Callback readers need no synchronization because ownership is assigned -// before callbacks can be enabled and is never changed afterward. +// The flags above are still std::atomic because setNativeMonitorEventsEnabled() +// and the Java-facing getters read them without holding this lock. static Mutex profiler_bridge_init_lock; jvmtiExtensionFunction VM::_request_stack_trace = nullptr; @@ -81,132 +81,6 @@ static void wakeupHandler(int signo) { // Dummy handler for interrupting syscalls } -static u64 monitorBlockerHash(jvmtiEnv *jvmti, jobject object) { - if (object == NULL) return 0; - jint hash = 0; - if (jvmti->GetObjectHashCode(object, &hash) != JVMTI_ERROR_NONE) return 0; - return static_cast(static_cast(hash)); -} - -// Deliberately takes no jvmtiEnv: no JVMTI call may run on this hot path. The -// blocker identity hash is resolved lazily in monitorBlockExit, and only for -// intervals that pass the minimum-duration filter (GetObjectHashCode mutates the -// object's mark word on HotSpot). -static void monitorBlockEnter(JNIEnv *jni, jthread thread, - OSThreadState state) { - Profiler *profiler = Profiler::instance(); - if (!profiler->taskBlockEnabled() || - !profiler->nativeMonitorTaskBlockEnabled() || - !JVMSupport::isPlatformThread(jni, thread)) { - return; - } - ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe(); - if (current == nullptr) return; - Context context = ContextApi::snapshot(); - if (context.spanId != 0) { - Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); - return; - } - - if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { - u64 token = current->monitorBlockToken(); - ThreadFilter *tf = profiler->threadFilter(); - bool current_owner = false; - if (token != 0) { - ThreadFilter::SlotID slot_id = ThreadFilter::tokenSlotId(token); - ThreadFilter::Slot *slot = current->filterSlotId() == slot_id - ? tf->activeSlotForId(slot_id, current->tid()) - : nullptr; - if (slot != nullptr) { - BlockRunSnapshot snapshot = slot->snapshotBlockRun(); - current_owner = snapshot.active && - snapshot.owner == BlockRunOwner::JVMTI && - snapshot.generation == ThreadFilter::tokenGeneration(token); - } - } - if (current_owner) { - return; - } - current->clearMonitorBlock(); - if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { - return; - } - } - - ThreadFilter *tf = profiler->threadFilter(); - ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); - if (!tf->unfilteredWallTrackingActive() || slot_id < 0) { - current->clearMonitorBlock(); - return; - } - u64 token = - tf->enterBlockedRun(slot_id, state, BlockRunOwner::JVMTI); - if (token == 0) { - ThreadFilter::Slot *slot = tf->slotForId(slot_id); - if (slot != nullptr && slot->inContextWindow()) { - Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); - } - current->clearMonitorBlock(); - return; - } - current->setMonitorBlockToken(token); -} - -static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, - jobject object, OSThreadState state) { - if (!JVMSupport::isPlatformThread(jni, thread)) return; - ProfiledThread *current = ProfiledThread::current(); - if (current == nullptr) return; - - u64 start_ticks = 0; - Context context{}; - // The entry side no longer records a blocker; it is resolved lazily below. - u64 blocker = 0; - u64 token = 0; - if (!current->monitorExit(state, start_ticks, context, blocker, token) || - token == 0) { - return; - } - - // Resolve the blocker identity hash only for intervals that will actually pass - // the eligibility filter. GetObjectHashCode mutates the object's mark word on - // HotSpot, so it must not run for short, high-frequency contention that gets - // discarded anyway. These conditions mirror taskBlockPassesBasicEligibility, and - // the same end_ticks is handed down so there is no boundary drift. - u64 end_ticks = TSC::ticks(); - if (context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { - blocker = monitorBlockerHash(jvmti, object); - } - - Profiler *profiler = Profiler::instance(); - finishTaskBlockAtExit(current, profiler->threadFilter(), thread, 0, token, - start_ticks, context, blocker, 0, end_ticks); -} - -static void JNICALL MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *jni, - jthread thread, jobject object) { - monitorBlockEnter(jni, thread, OSThreadState::MONITOR_WAIT); -} - -static void JNICALL MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *jni, - jthread thread, jobject object) { - monitorBlockExit(jvmti, jni, thread, object, OSThreadState::MONITOR_WAIT); -} - -static void JNICALL MonitorWait(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, - jobject object, jlong timeout) { - if (!VM::monitorWaitEventsDelegated()) { - monitorBlockEnter(jni, thread, OSThreadState::OBJECT_WAIT); - } -} - -static void JNICALL MonitorWaited(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, - jobject object, jboolean timed_out) { - if (!VM::monitorWaitEventsDelegated()) { - monitorBlockExit(jvmti, jni, thread, object, OSThreadState::OBJECT_WAIT); - } -} - static bool isVmRuntimeEntry(const char* blob_name) { return strcmp(blob_name, "_ZNK12MemAllocator8allocateEv") == 0 || strncmp(blob_name, "_Z22post_allocation_notify", 26) == 0 @@ -526,7 +400,7 @@ bool VM::initShared(JavaVM* vm) { bool VM::initLibrary(JavaVM *vm) { MutexLocker init_locker(profiler_bridge_init_lock); - if (_profiler_bridge_initialized) { + if (_profiler_bridge_initialized.load(std::memory_order_acquire)) { return true; } @@ -601,17 +475,21 @@ void JNICALL ForwardedGarbageCollectionFinish(jvmtiEnv *jvmti_env) { void VM::configureMonitorEvents(bool delegateMonitorWaitEvents) { jvmtiCapabilities actual_capabilities = {0}; - _jvmti->GetCapabilities(&actual_capabilities); - _native_monitor_events_available = - actual_capabilities.can_generate_monitor_events; - _monitor_wait_events_delegated = delegateMonitorWaitEvents; + jvmtiError rc = _jvmti->GetCapabilities(&actual_capabilities); + if (rc != JVMTI_ERROR_NONE) { + Log::warn("GetCapabilities failed: %d; native monitor events treated as unavailable", rc); + } + _native_monitor_events_available.store( + rc == JVMTI_ERROR_NONE && actual_capabilities.can_generate_monitor_events, + std::memory_order_release); + _monitor_wait_events_delegated.set(delegateMonitorWaitEvents); } ProfilerBridgeInitResult VM::initProfilerBridge(JavaVM *vm, bool attach, bool delegateMonitorWaitEvents) { MutexLocker init_locker(profiler_bridge_init_lock); - if (_profiler_bridge_initialized) { - return delegateMonitorWaitEvents == _monitor_wait_events_delegated + if (_profiler_bridge_initialized.load(std::memory_order_acquire)) { + return _monitor_wait_events_delegated.matches(delegateMonitorWaitEvents) ? ProfilerBridgeInitResult::SUCCESS : ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT; } @@ -696,7 +574,7 @@ ProfilerBridgeInitResult VM::initProfilerBridge(JavaVM *vm, bool attach, callbacks.GarbageCollectionStart = ReferenceChainTracker::GarbageCollectionStart; callbacks.GarbageCollectionFinish = ForwardedGarbageCollectionFinish; callbacks.NativeMethodBind = VMStructs::NativeMethodBind; - if (_native_monitor_events_available) { + if (_native_monitor_events_available.load(std::memory_order_acquire)) { callbacks.MonitorContendedEnter = MonitorContendedEnter; callbacks.MonitorContendedEntered = MonitorContendedEntered; callbacks.MonitorWait = MonitorWait; @@ -754,17 +632,18 @@ ProfilerBridgeInitResult VM::initProfilerBridge(JavaVM *vm, bool attach, OS::installSignalHandler(WAKEUP_SIGNAL, NULL, wakeupHandler); - _profiler_bridge_initialized = true; + _profiler_bridge_initialized.store(true, std::memory_order_release); return ProfilerBridgeInitResult::SUCCESS; } bool VM::setNativeMonitorEventsEnabled(bool enabled) { - if (!_native_monitor_events_available) return false; + if (!_native_monitor_events_available.load(std::memory_order_acquire)) return false; jvmtiError enter = JVMTI_ERROR_NONE; jvmtiError entered = JVMTI_ERROR_NONE; jvmtiError wait = JVMTI_ERROR_NONE; jvmtiError waited = JVMTI_ERROR_NONE; + bool delegated = _monitor_wait_events_delegated.value(); if (enabled) { // JVMTI enables each event independently and does not queue events that @@ -774,7 +653,7 @@ bool VM::setNativeMonitorEventsEnabled(bool enabled) { JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, NULL); if (entered != JVMTI_ERROR_NONE) goto enable_failed; - if (!_monitor_wait_events_delegated) { + if (!delegated) { waited = _jvmti->SetEventNotificationMode( JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAITED, NULL); if (waited != JVMTI_ERROR_NONE) goto enable_failed; @@ -784,7 +663,7 @@ bool VM::setNativeMonitorEventsEnabled(bool enabled) { JVMTI_ENABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER, NULL); if (enter != JVMTI_ERROR_NONE) goto enable_failed; - if (!_monitor_wait_events_delegated) { + if (!delegated) { wait = _jvmti->SetEventNotificationMode( JVMTI_ENABLE, JVMTI_EVENT_MONITOR_WAIT, NULL); if (wait != JVMTI_ERROR_NONE) goto enable_failed; @@ -792,8 +671,15 @@ bool VM::setNativeMonitorEventsEnabled(bool enabled) { return true; enable_failed: - Log::warn("Unable to enable JVMTI monitor events: %d/%d/%d/%d", - enter, entered, wait, waited); + // wait/waited are skipped (left JVMTI_ERROR_NONE) rather than attempted + // when delegated is true; log that explicitly so a 0 there isn't read as success. + if (delegated) { + Log::warn("Unable to enable JVMTI monitor events: enter=%d/entered=%d " + "(wait/waited skipped: delegated to Java)", enter, entered); + } else { + Log::warn("Unable to enable JVMTI monitor events: enter=%d/entered=%d/wait=%d/waited=%d", + enter, entered, wait, waited); + } setNativeMonitorEventsEnabled(false); return false; } diff --git a/ddprof-lib/src/main/cpp/vmEntry.h b/ddprof-lib/src/main/cpp/vmEntry.h index 35268a62af..0c6f4c7585 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.h +++ b/ddprof-lib/src/main/cpp/vmEntry.h @@ -12,6 +12,8 @@ #include "arguments.h" #include "codeCache.h" +#include + #ifdef __clang__ #define DLLEXPORT __attribute__((visibility("default"))) #else @@ -141,6 +143,25 @@ enum class ProfilerBridgeInitResult { MONITOR_EVENTS_DELEGATION_CONFLICT, }; +// A value fixed once per profiler-bridge lifetime by the one-time setup path +// and compared against by any later caller trying to join the same singleton +// with a possibly-different request. Read lock-free and cross-thread (e.g. by +// JVMTI event callbacks on arbitrary JVM threads), so plain atomic load/store +// rather than the mutex the one-time setup path already holds. +template +class NegotiatedSetting { +public: + void set(T value) { _value.store(value, std::memory_order_release); } + T value() const { return _value.load(std::memory_order_acquire); } + // True if 'value' agrees with what the one-time setup path already set. + bool matches(T value) const { + return value == _value.load(std::memory_order_acquire); + } + +private: + std::atomic _value{T{}}; +}; + class VM { friend class VMTestAccessor; @@ -156,9 +177,9 @@ class VM { static bool _zing; static bool _can_sample_objects; static bool _can_intercept_binding; - static bool _monitor_wait_events_delegated; - static bool _native_monitor_events_available; - static bool _profiler_bridge_initialized; + static NegotiatedSetting _monitor_wait_events_delegated; + static std::atomic _native_monitor_events_available; + static std::atomic _profiler_bridge_initialized; static bool _is_adaptive_gc_boundary_flag_set; static CodeCache *_libjvm; @@ -233,11 +254,11 @@ class VM { static bool canSampleObjects() { return _can_sample_objects; } static bool monitorWaitEventsDelegated() { - return _monitor_wait_events_delegated; + return _monitor_wait_events_delegated.value(); } static bool nativeMonitorEventsAvailable() { - return _native_monitor_events_available; + return _native_monitor_events_available.load(std::memory_order_acquire); } static bool setNativeMonitorEventsEnabled(bool enabled); diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index 65bab51fe6..86b74f56fe 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -147,6 +147,9 @@ public static synchronized JavaProfiler getInstance(String libLocation, String s boolean delegateMonitorWaitEvents) throws IOException { if (instance != null) { if (monitorWaitEventsDelegated0() != delegateMonitorWaitEvents) { + // Keep this message identical to the one javaApi.cpp's init0 throws for the + // analogous native-bridge conflict; there is no shared constant across the + // JNI boundary, so both call sites must be updated together. throw new IllegalStateException( "Monitor-event ownership conflicts with the profiler's " + "process-wide initialization"); @@ -480,15 +483,19 @@ void parkExit(long blocker, long unblockingSpanId) { */ long blockEnter(int state) { Thread thread = Thread.currentThread(); - return blockEnter0(thread, isVirtualThread(thread), state); + return blockEnter0(isVirtualThread(thread), state); } /** - * Clears a blocked interval previously armed by {@link #blockEnter(int)}. + * Clears a blocked interval previously armed by {@link #blockEnter(int)} and records its + * {@code TaskBlock} event when it satisfies the profiler's eligibility rules. + * + * @param blocker stable identifier describing the blocking resource, or {@code 0} + * @param unblockingSpanId span responsible for unblocking the interval, or {@code 0} */ - void blockExit(long token) { + void blockExit(long token, long blocker, long unblockingSpanId) { Thread thread = Thread.currentThread(); - blockExit0(thread, isVirtualThread(thread), token); + blockExit0(isVirtualThread(thread), token, blocker, unblockingSpanId); } /** @@ -598,9 +605,10 @@ static int getTid() { private static native void parkExit0(Thread thread, boolean isVirtual, long blocker, long unblockingSpanId); - private static native long blockEnter0(Thread thread, boolean isVirtual, int state); + private static native long blockEnter0(boolean isVirtual, int state); - private static native void blockExit0(Thread thread, boolean isVirtual, long token); + private static native void blockExit0(boolean isVirtual, long token, long blocker, + long unblockingSpanId); private static native long beginTaskBlock0(Thread thread); diff --git a/ddprof-lib/src/test/cpp/vmEntry_ut.cpp b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp index e40e20f450..e5b953ccfd 100644 --- a/ddprof-lib/src/test/cpp/vmEntry_ut.cpp +++ b/ddprof-lib/src/test/cpp/vmEntry_ut.cpp @@ -14,7 +14,7 @@ class VMTestAccessor { public: - static jvmtiEnv* jvmti() { return VM::_jvmti; } + static jvmtiEnv* getJvmti() { return VM::_jvmti; } static void setJvmti(jvmtiEnv* jvmti) { VM::_jvmti = jvmti; } static bool nativeMonitorEventsAvailable() { @@ -25,10 +25,10 @@ class VMTestAccessor { } static bool monitorWaitEventsDelegated() { - return VM::_monitor_wait_events_delegated; + return VM::_monitor_wait_events_delegated.value(); } static void setMonitorWaitEventsDelegated(bool delegated) { - VM::_monitor_wait_events_delegated = delegated; + VM::_monitor_wait_events_delegated.set(delegated); } static bool profilerBridgeInitialized() { @@ -85,7 +85,7 @@ class MonitorEventConfigurationTest : public ::testing::Test { } void SetUp() override { - original_jvmti = VMTestAccessor::jvmti(); + original_jvmti = VMTestAccessor::getJvmti(); original_initialized = VMTestAccessor::profilerBridgeInitialized(); original_available = VMTestAccessor::nativeMonitorEventsAvailable(); original_delegated = VMTestAccessor::monitorWaitEventsDelegated(); @@ -241,7 +241,7 @@ class NativeMonitorEventsTest : public ::testing::Test { } void SetUp() override { - original_jvmti = VMTestAccessor::jvmti(); + original_jvmti = VMTestAccessor::getJvmti(); original_available = VMTestAccessor::nativeMonitorEventsAvailable(); original_delegated = VMTestAccessor::monitorWaitEventsDelegated(); original_task_block_enabled = profiler->taskBlockEnabled(); diff --git a/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java b/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java index ac96ec5b85..61844c67ba 100644 --- a/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java +++ b/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java @@ -16,6 +16,6 @@ public static long enterSleeping(JavaProfiler profiler) { /** Closes an interval returned by {@link #enterSleeping(JavaProfiler)}. */ public static void exit(JavaProfiler profiler, long token) { - profiler.blockExit(token); + profiler.blockExit(token, 0, 0); } } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java index c02ed38ac4..fe7059c3b3 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -27,7 +27,8 @@ public void taskBlockApiIsPublicButInternalHooksRemainPackageScoped() throws Exc assertNotPublic(JavaProfiler.class.getDeclaredMethod( "parkExit", long.class, long.class)); assertNotPublic(JavaProfiler.class.getDeclaredMethod("blockEnter", int.class)); - assertNotPublic(JavaProfiler.class.getDeclaredMethod("blockExit", long.class)); + assertNotPublic(JavaProfiler.class.getDeclaredMethod( + "blockExit", long.class, long.class, long.class)); assertTrue(Modifier.isPublic(JavaProfiler.class .getDeclaredMethod("beginTaskBlock").getModifiers())); assertTrue(Modifier.isPublic(JavaProfiler.class diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ProfilerOwnedBlockHooks.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ProfilerOwnedBlockHooks.java index 68208fac73..96bd33cbf2 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ProfilerOwnedBlockHooks.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ProfilerOwnedBlockHooks.java @@ -22,7 +22,12 @@ public static long blockEnter(JavaProfiler profiler, int state) { } public static void blockExit(JavaProfiler profiler, long token) { - profiler.blockExit(token); + profiler.blockExit(token, 0, 0); + } + + public static void blockExit(JavaProfiler profiler, long token, long blocker, + long unblockingSpanId) { + profiler.blockExit(token, blocker, unblockingSpanId); } public static long beginTaskBlockForThread(JavaProfiler profiler, Thread thread) { From 8e894a8d3632e777e080a46b87154e41f05508fe Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Wed, 21 Oct 2026 22:20:31 +0200 Subject: [PATCH 5/9] fix: preserve wall-clock lifecycle state in JVM TaskBlock producers Refresh producer TLS against retained registry epochs, recover abandoned intervals after context entry, and clean up late monitor exits. Flush pending unowned wall-clock weight before admitting an owned run, keeping cached traces published until their JFR writer stripe is held. Share JNI hook declarations with native restart tests and add regression coverage for producer restarts, pooled TLS reset, and tail replay across a live JFR dump. --- ddprof-lib/src/main/cpp/javaApi.cpp | 124 ++++----- ddprof-lib/src/main/cpp/javaApi.h | 28 +++ ddprof-lib/src/main/cpp/profiler.cpp | 6 +- ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | 64 +++-- ddprof-lib/src/main/cpp/taskBlockRecorder.h | 6 + ddprof-lib/src/main/cpp/threadLocalData.cpp | 1 + ddprof-lib/src/main/cpp/threadLocalData.h | 25 +- ddprof-lib/src/main/cpp/wallClock.cpp | 26 ++ ddprof-lib/src/main/cpp/wallClock.h | 3 + .../src/main/cpp/wallClockBlockTracker.h | 7 + .../profiler/JavaProfilerTestSupport.java | 17 ++ .../src/test/cpp/taskBlockProducers_ut.cpp | 236 ++++++++++++++++++ .../src/test/cpp/threadLocalDataPool_ut.cpp | 2 + .../src/test/cpp/wallClockBlockRestart_ut.cpp | 38 ++- .../TaskBlockProducerCompatibilityTest.java | 132 ++++++++++ .../UnownedTailBeforeTaskBlockTest.java | 54 ++++ 16 files changed, 677 insertions(+), 92 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/javaApi.h create mode 100644 ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockProducerCompatibilityTest.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnownedTailBeforeTaskBlockTest.java diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 52e0432217..ef0908b4b7 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -26,6 +26,7 @@ #include "engine.h" #include "hotspot/vmStructs.inline.h" #include "incbin.h" +#include "javaApi.h" #include "jvmSupport.h" #include "jvmThread.h" #include "os.h" @@ -379,6 +380,47 @@ Java_com_datadoghq_profiler_JavaProfilerTestSupport_recordUnownedWallTailAcrossD #endif } +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfilerTestSupport_recordUnownedWallTailBeforeOwnedBlockForTest0( + JNIEnv *env, jclass unused, jthread thread, jstring snapshot, jint producer) { +#ifdef DEBUG + SignalBlocker blocker; + if (!BaseWallClock::seedUnownedWallTailForTest(thread)) return JNI_FALSE; + Profiler* profiler = Profiler::instance(); + JniString path(env, snapshot); + if (profiler->dump(path.c_str(), path.length())) return JNI_FALSE; + u64 token = 0; + switch (producer) { + case 0: + token = Java_com_datadoghq_profiler_JavaProfiler_beginTaskBlock0(env, nullptr, thread); + Java_com_datadoghq_profiler_JavaProfiler_endTaskBlock0(env, nullptr, thread, token, 0, 0); + break; + case 1: + token = Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( + env, nullptr, JNI_FALSE, static_cast(OSThreadState::SLEEPING)); + Java_com_datadoghq_profiler_JavaProfiler_blockExit0(env, nullptr, JNI_FALSE, token, 0, 0); + break; + case 2: + if (Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(env, nullptr, thread, JNI_FALSE)) { + token = profiler->blockTracker()->hasActiveBlockRun( + ProfiledThread::current()->filterSlotId()) ? 1 : 0; + Java_com_datadoghq_profiler_JavaProfiler_parkExit0(env, nullptr, thread, JNI_FALSE, 0, 0); + } + break; + case 3: + MonitorContendedEnter(VM::jvmti(), env, thread, nullptr); + token = ProfiledThread::current()->monitorBlockToken(); + MonitorContendedEntered(VM::jvmti(), env, thread, nullptr); + break; + default: + return JNI_FALSE; + } + return token != 0 ? JNI_TRUE : JNI_FALSE; +#else + return JNI_FALSE; +#endif +} + extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_recordSettingEvent0( JNIEnv *env, jclass unused, jstring name, jstring value, jstring unit) { @@ -460,22 +502,20 @@ Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( } Profiler *profiler = Profiler::instance(); ThreadFilter *tf = profiler->threadFilter(); - if (!tf->registryActive() || - !(profiler->taskBlockEnabled() || tf->enabled())) { - return JNI_FALSE; - } Context context = ContextApi::snapshot(); + ThreadFilter::SlotID slot_id = tf->registryActive() && context.spanId == 0 + ? tf->ensureCurrentThreadSlot(current) : -1; + // Use the owner's epoch rather than a second global epoch read: stop/start + // may race admission, but only this thread refreshes its retained identity. + current->refreshBlockState(slot_id >= 0 + ? tf->slotForId(slot_id)->recordingEpoch() : tf->recordingEpoch()); if (!current->parkEnter(TSC::ticks(), context)) { return JNI_FALSE; } - if (context.spanId == 0) { - ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); - if (slot_id >= 0) { - WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); - current->setParkBlockToken( - tracker->enterBlockedRun(tf, slot_id, OSThreadState::CONDVAR_WAIT)); - } + if (slot_id >= 0) { + current->setParkBlockToken( + enterOwnedBlockRun(current, slot_id, OSThreadState::CONDVAR_WAIT)); } return JNI_TRUE; } @@ -555,41 +595,14 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( } ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); if (slot_id < 0) return 0; + current->refreshBlockState(tf->slotForId(slot_id)->recordingEpoch()); WallClockBlockTracker *tracker = profiler->blockTracker(); - u64 token = tracker->enterBlockedRun(tf, slot_id, decoded); + u64 token = enterOwnedBlockRun(current, slot_id, decoded); if (token == 0) return 0; - // Also arm the TaskBlock recording slot so blockExit0 can produce an event - // with blocker identity, matching the beginTaskBlock0/endTaskBlock0 pair. - // Skipped when TaskBlock recording is disabled so this stays purely a - // wall-clock suppression marker for the precheck tests that exercise it - // with task-block recording off. - if (task_block_armable && - !current->taskBlockEnter(token, TSC::ticks(), context)) { - // FLAG_TASK_BLOCKED is sticky across recordings (unlike the WallClockBlockTracker - // blocked-run, which is generation-scoped), so a worker whose previous - // interval was abandoned mid-run (e.g. profiler stop/restart without a - // matching blockExit0) can still be holding it. Mirror monitorBlockEnter's - // stale-slot check: only refuse the new interval if the old token still - // maps to a genuinely active, same-generation blocked run. - u64 stale_token = current->taskBlockToken(); - bool current_owner = false; - if (stale_token != 0) { - ThreadFilter::SlotID stale_slot_id = WallClockBlockTracker::tokenSlotId(stale_token); - ThreadFilter::Slot *stale_slot = current->filterSlotId() == stale_slot_id - ? tf->activeSlotForId(stale_slot_id, current->tid()) - : nullptr; - if (stale_slot != nullptr) { - WallClockBlockTracker::BlockState *block_state = tracker->slotForId(stale_slot_id); - current_owner = block_state != nullptr && - block_state->activeBlockState() != OSThreadState::UNKNOWN && - block_state->activeBlockOwner() == BlockRunOwner::JAVA && - block_state->blockGeneration() == WallClockBlockTracker::tokenGeneration(stale_token); - } - } - if (current_owner) { - tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(token)); - return 0; - } + if (task_block_armable) { + // enterOwnedBlockRun() acquired the slot's sole ownership above. Any older + // TLS token is therefore abandoned (for example, addThread() cleared its + // JAVA-owned run) and cannot still own a live interval. current->clearTaskBlock(); if (!current->taskBlockEnter(token, TSC::ticks(), context)) { tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(token)); @@ -614,11 +627,6 @@ Java_com_datadoghq_profiler_JavaProfiler_blockExit0( } ThreadFilter *tf = Profiler::instance()->threadFilter(); ThreadFilter::SlotID slot_id = WallClockBlockTracker::tokenSlotId(block_token); - if (current->filterSlotId() != slot_id || - tf->activeSlotForId(slot_id, current->tid()) == nullptr) { - return; - } - u64 start_ticks = 0; Context context{}; if (current->taskBlockExit(block_token, start_ticks, context)) { @@ -632,7 +640,8 @@ Java_com_datadoghq_profiler_JavaProfiler_blockExit0( return; } - if (tf->registryActive()) { + if (tf->registryActive() && current->filterSlotId() == slot_id && + tf->activeSlotForId(slot_id, current->tid()) != nullptr) { WallClockBlockTracker *tracker = Profiler::instance()->blockTracker(); tracker->exitBlockedRun(slot_id, WallClockBlockTracker::tokenGeneration(block_token)); } @@ -659,19 +668,22 @@ Java_com_datadoghq_profiler_JavaProfiler_beginTaskBlock0( ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); if (slot_id < 0) return 0; + current->refreshBlockState(tf->slotForId(slot_id)->recordingEpoch()); Context context = ContextApi::snapshot(); if (context.spanId != 0) { Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT); return 0; } WallClockBlockTracker *tracker = profiler->blockTracker(); - u64 token = tracker->enterBlockedRun( - tf, slot_id, OSThreadState::SLEEPING, BlockRunOwner::TASK_BLOCK); + u64 token = enterOwnedBlockRun( + current, slot_id, OSThreadState::SLEEPING, BlockRunOwner::TASK_BLOCK); + if (token == 0) return 0; + // A successfully acquired run excludes any previous registry owner, even + // when its TLS payload survived a context-window transition. + current->clearTaskBlock(); if (!current->taskBlockEnter(token, TSC::ticks(), context)) { - if (token != 0) { - tracker->exitBlockedRun(slot_id, - WallClockBlockTracker::tokenGeneration(token)); - } + tracker->exitBlockedRun(slot_id, + WallClockBlockTracker::tokenGeneration(token)); return 0; } return static_cast(token); diff --git a/ddprof-lib/src/main/cpp/javaApi.h b/ddprof-lib/src/main/cpp/javaApi.h new file mode 100644 index 0000000000..d68492546d --- /dev/null +++ b/ddprof-lib/src/main/cpp/javaApi.h @@ -0,0 +1,28 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef _JAVA_API_H +#define _JAVA_API_H + +#include + +// Shared declarations for the blocking hooks exercised directly by native tests. +// Keep the implementation and callers on the same JNI ABI. +extern "C" { +JNIEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( + JNIEnv*, jclass, jboolean, jint); +JNIEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0( + JNIEnv*, jclass, jboolean, jlong, jlong, jlong); +JNIEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + JNIEnv*, jclass, jthread, jboolean); +JNIEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkExit0( + JNIEnv*, jclass, jthread, jboolean, jlong, jlong); +JNIEXPORT jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_beginTaskBlock0( + JNIEnv*, jclass, jthread); +JNIEXPORT jboolean JNICALL Java_com_datadoghq_profiler_JavaProfiler_endTaskBlock0( + JNIEnv*, jclass, jthread, jlong, jlong, jlong); +} + +#endif // _JAVA_API_H diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index d296bf8c6c..cb9c6842ee 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -2308,10 +2308,8 @@ Error Profiler::dump(const char *path, const int length) { Error err = Error::OK; // rotateDictsAndRun rotates the dictionaries, takes lockAll() around the // dump (fences ASGCT/JNI writers to CallTraceStorage), then clearStandby()s - // the rotated buffers. StringDictionary's rotate()/standby()/clearStandby() - // protocol handles the dump's own writer/reader coordination for each - // dictionary, including _class_map (see flightRecorder.cpp's writeCpool: - // classMap()->standby() stays stable for the dump's lifetime). + // the rotated buffers. StringDictionary's RefCountGuard protocol handles + // its own writer/reader coordination. if (beginTaskBlockRotation()) { rotateDictsAndRun([&]{ err = _jfr.dump(path, length); diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp index 5ecce508ba..506a953d8b 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp @@ -29,6 +29,40 @@ bool exceedsMinTaskBlockDuration(u64 start_ticks, u64 end_ticks) { return end_ticks > start_ticks && end_ticks - start_ticks >= min_ticks; } +u64 enterOwnedBlockRun(ProfiledThread* current, ThreadFilter::SlotID slot_id, + OSThreadState state, BlockRunOwner owner) { + Profiler* profiler = Profiler::instance(); + ThreadFilter* registry = profiler->threadFilter(); + WallClockBlockTracker* tracker = profiler->blockTracker(); + WallClockBlockTracker::BlockState* slot = tracker->slotForId(slot_id); + if (slot == nullptr) return 0; + + bool fallback_enabled = slot->pauseUnownedBlockedFallback(); + if (fallback_enabled && + slot->unowned_blocked_pending_weight.load(std::memory_order_acquire) != 0) { + // Signal handlers normally own this write path. A producer needs the + // same stop/dump exclusion before replaying a cached trace into JFR. + TaskBlockActivity activity; + if (!activity.active() || !registry->unfilteredWallTrackingActive() || + registry->activeSlotForId(slot_id, current->tid()) == nullptr) { + slot->resumeUnownedBlockedFallback(); + return 0; + } + CriticalSection cs(current); + u64 call_trace_id = 0, weight = 0; + OSThreadState tail_state = OSThreadState::UNKNOWN; + if (slot->prepareUnownedBlockedTail(call_trace_id, weight, tail_state)) { + // Keep the trace published until recordUnownedBlockedTail() holds its + // writer stripe, preserving the core branch's JFR liveness protocol. + profiler->recordUnownedBlockedTail(current->tid(), slot, weight); + } + } + + u64 token = tracker->enterBlockedRun(registry, slot_id, state, owner); + if (token == 0 && fallback_enabled) slot->resumeUnownedBlockedFallback(); + return token; +} + bool recordTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter, WallClockBlockTracker* tracker, jthread thread, int start_depth, u64 block_token, u64 blocker, @@ -59,8 +93,8 @@ bool finishTaskBlockAtExit(ProfiledThread* current, u64 generation = WallClockBlockTracker::tokenGeneration(block_token); // beginTaskBlock0 resolves its slot through ThreadFilter::ensureCurrentThreadSlot(), // which self-heals a cached-but-stale slot_id (e.g. after a recording restart - // reset the registry) back to this tid's real slot. The block duration between - // enter and exit can span such a reset, so re-validate here too instead of + // changed the epoch) back to this tid's real slot. The block duration between + // enter and exit can span such a restart, so re-validate here too instead of // trusting the cached value directly - otherwise a stale cache makes the slot // comparison below meaningless and silently drops the event uncounted. ThreadFilter::SlotID current_slot = thread_filter->ensureCurrentThreadSlot(current); @@ -113,9 +147,12 @@ static void monitorBlockEnter(JNIEnv *jni, jthread thread, return; } + ThreadFilter *tf = profiler->threadFilter(); + ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); + if (!tf->unfilteredWallTrackingActive() || slot_id < 0) return; + current->refreshBlockState(tf->slotForId(slot_id)->recordingEpoch()); if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { u64 token = current->monitorBlockToken(); - ThreadFilter *tf = profiler->threadFilter(); bool current_owner = false; if (token != 0) { ThreadFilter::SlotID slot_id = WallClockBlockTracker::tokenSlotId(token); @@ -139,14 +176,7 @@ static void monitorBlockEnter(JNIEnv *jni, jthread thread, } } - ThreadFilter *tf = profiler->threadFilter(); - ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); - if (!tf->unfilteredWallTrackingActive() || slot_id < 0) { - current->clearMonitorBlock(); - return; - } - u64 token = - profiler->blockTracker()->enterBlockedRun(tf, slot_id, state, BlockRunOwner::JVMTI); + u64 token = enterOwnedBlockRun(current, slot_id, state, BlockRunOwner::JVMTI); if (token == 0) { ThreadFilter::Slot *slot = tf->slotForId(slot_id); if (slot != nullptr && slot->inContextWindow()) { @@ -161,12 +191,9 @@ static void monitorBlockEnter(JNIEnv *jni, jthread thread, static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, jobject object, OSThreadState state) { Profiler *profiler = Profiler::instance(); - if (!profiler->taskBlockEnabled() || !profiler->nativeMonitorTaskBlockEnabled()) { - return; - } - if (!JVMSupport::isPlatformThread(jni, thread)) return; ProfiledThread *current = ProfiledThread::current(); if (current == nullptr) return; + if (!JVMSupport::isPlatformThread(jni, thread)) return; u64 start_ticks = 0; Context context{}; @@ -181,12 +208,13 @@ static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, // Resolve the blocker identity hash only for intervals that will actually pass // the eligibility filter. GetObjectHashCode mutates the object's mark word on // HotSpot, so it must not run for short, high-frequency contention that gets - // discarded anyway. That mutation also permanently disables biased locking for - // this object, a cost borne by the profiled application, not just the profiler. + // discarded anyway. On older HotSpot versions with biased locking, assigning + // an identity hash also prevents the object from being biased again. // These conditions mirror taskBlockPassesBasicEligibility, and the same // end_ticks is handed down so there is no boundary drift. u64 end_ticks = TSC::ticks(); - if (context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { + if (profiler->taskBlockEnabled() && profiler->nativeMonitorTaskBlockEnabled() && + context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { blocker = monitorBlockerHash(jvmti, object); } diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.h b/ddprof-lib/src/main/cpp/taskBlockRecorder.h index 4cef7e74cf..f400e30f70 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -15,6 +15,12 @@ void initializeTaskBlockDurationThreshold(); bool exceedsMinTaskBlockDuration(u64 start_ticks, u64 end_ticks); +// Starts a producer-owned run after flushing the preceding unowned wall tail. +// Called on the slot's owning thread, so pausing fallback accumulation also +// prevents a profiling signal from adding weight between the flush and admission. +u64 enterOwnedBlockRun(ProfiledThread* current, ThreadFilter::SlotID slot_id, + OSThreadState state, BlockRunOwner owner = BlockRunOwner::JAVA); + bool recordTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter, WallClockBlockTracker* tracker, jthread thread, int start_depth, u64 block_token, u64 blocker, diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index d6f3c1ae56..db24d9c9fa 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -172,6 +172,7 @@ void ProfiledThread::unclaimAndReset() { _park_slot = {}; _task_block_slot = {}; _monitor_slot = {}; + _block_recording_epoch = 0; _filter_slot_id = -1; _init_window = 0; _signal_depth = 0; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index fb3f129cc9..da970a07f4 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -103,6 +103,7 @@ class ProfiledThread : public ThreadLocalData { BlockSlot _park_slot; BlockSlot _task_block_slot; BlockSlot _monitor_slot; + u64 _block_recording_epoch; int _filter_slot_id; // Slot ID for thread filtering uint8_t _init_window; // Countdown for JVM thread init race window (PROF-13072) volatile int _signal_depth; // Nested signal-handler depth (see SignalHandlerScope) @@ -125,7 +126,7 @@ class ProfiledThread : public ThreadLocalData { 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(0), - _park_slot{}, _task_block_slot{}, _monitor_slot{}, + _park_slot{}, _task_block_slot{}, _monitor_slot{}, _block_recording_epoch(0), _filter_slot_id(-1), _init_window(0), _signal_depth(0), @@ -215,6 +216,7 @@ class ProfiledThread : public ThreadLocalData { u64 task_block_start_ticks; u64 task_block_token; Context task_block_context; + u64 block_recording_epoch; int filter_slot_id; uint8_t init_window; int signal_depth; @@ -230,7 +232,7 @@ class ProfiledThread : public ThreadLocalData { _cpu_epoch, _wall_epoch, _call_trace_id, _recording_epoch, __atomic_load_n(&_misc_flags, __ATOMIC_RELAXED), _park_slot.block_token, _task_block_slot.start_ticks, _task_block_slot.block_token, - _task_block_slot.context, _filter_slot_id, _init_window, + _task_block_slot.context, _block_recording_epoch, _filter_slot_id, _init_window, __atomic_load_n(&_signal_depth, __ATOMIC_RELAXED), _in_critical_section, _otel_ctx_initialized, _otel_local_root_span_id }; @@ -430,6 +432,20 @@ class ProfiledThread : public ThreadLocalData { _otel_local_root_span_id = 0; } + // Called by the owning thread before admitting a producer interval. Registry + // identities survive restart, but their old producer payload must not: an + // exit callback may have been disabled while the thread was blocked. + inline void refreshBlockState(u64 recording_epoch) { + if (_block_recording_epoch == recording_epoch) return; + __atomic_fetch_and(&_misc_flags, + ~(FLAG_PARKED | FLAG_TASK_BLOCKED | FLAG_MONITOR_BLOCKED), + __ATOMIC_ACQ_REL); + _park_slot = {}; + _task_block_slot = {}; + _monitor_slot = {}; + _block_recording_epoch = recording_epoch; + } + inline bool parkEnter(u64 start_ticks, const Context& context) { return blockSlotEnter(_park_slot, start_ticks, context, /*blocker=*/0, OSThreadState::UNKNOWN); @@ -488,8 +504,9 @@ class ProfiledThread : public ThreadLocalData { } #endif - // Object.wait owns its interval until MonitorWaited, including monitor - // reacquisition. A nested contention callback must not overwrite that state. + // Object.wait owns its interval until MonitorWaited. HotSpot 21 posts that + // callback before monitor reacquisition (ObjectMonitor::wait()). A nested + // contention callback must not overwrite an interval that is still open. inline bool monitorEnter(u64 start_ticks, const Context& context, u64 blocker, OSThreadState state) { return blockSlotEnter(_monitor_slot, start_ticks, diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp index 0cd5e02e97..57e4ac3c95 100644 --- a/ddprof-lib/src/main/cpp/wallClock.cpp +++ b/ddprof-lib/src/main/cpp/wallClock.cpp @@ -272,6 +272,32 @@ bool BaseWallClock::recordUnownedWallTailAcrossDumpForTest( return recordAsgctWallSample(nullptr, 1, current->tid(), 0, &event, tail, &dump_test) && !dump_test.error; } + +bool BaseWallClock::seedUnownedWallTailForTest(jthread thread) { + Profiler* profiler = Profiler::instance(); + ProfiledThread* current = ProfiledThread::initCurrentThreadSignalSafe(); + ThreadFilter* registry = profiler->threadFilter(); + if (current == nullptr || !profiler->isRunning() || + !registry->unfilteredWallTrackingActive()) return false; + ThreadFilter::SlotID slot_id = registry->ensureCurrentThreadSlot(current); + WallClockBlockTracker::BlockState* slot = profiler->blockTracker()->slotForId(slot_id); + if (slot == nullptr || slot->activeBlockOwner() != BlockRunOwner::NONE) return false; + slot->resetUnownedBlockedSampling(); + ExecutionEvent event; + event._thread_state = OSThreadState::SLEEPING; + event._execution_mode = ExecutionMode::JAVA; + WallPrecheckResult first = prepareWallPrecheck(current, true, true, OSThreadState::SLEEPING); + if (first.suppress || first.unowned_weight_slot != slot) return false; + u64 trace_id = profiler->recordJVMTISample( + 1, current->tid(), thread, BCI_WALL, &event, true); + if (trace_id == 0 || !profiler->recordWallSample( + nullptr, 1, current->tid(), trace_id, &event, slot)) return false; + finishWallPrecheck(first, true); + for (int i = 0; i < 5; i++) { + if (!prepareWallPrecheck(current, true, true, OSThreadState::SLEEPING).suppress) return false; + } + return true; +} #endif bool BaseWallClock::inSyscall(void *ucontext) { diff --git a/ddprof-lib/src/main/cpp/wallClock.h b/ddprof-lib/src/main/cpp/wallClock.h index 3763a15baf..21e83136b2 100644 --- a/ddprof-lib/src/main/cpp/wallClock.h +++ b/ddprof-lib/src/main/cpp/wallClock.h @@ -252,6 +252,9 @@ class BaseWallClock : public Engine { static bool recordUnownedWallTailAcrossDumpForTest( jthread thread, const char* path, int path_length, bool before_publication, bool reset_before_tail); + // Seeds a real cached stack and five suppressed observations. The caller + // blocks profiling signals until the tested ownership transition finishes. + static bool seedUnownedWallTailForTest(jthread thread); #endif }; diff --git a/ddprof-lib/src/main/cpp/wallClockBlockTracker.h b/ddprof-lib/src/main/cpp/wallClockBlockTracker.h index 3c9af54192..7e01f212a1 100644 --- a/ddprof-lib/src/main/cpp/wallClockBlockTracker.h +++ b/ddprof-lib/src/main/cpp/wallClockBlockTracker.h @@ -131,6 +131,13 @@ class WallClockBlockTracker { inline bool unownedBlockedFallbackEnabled() const { return unowned_blocked_fallback_enabled.load(std::memory_order_acquire) != 0; } + // Stop signal-side accumulation without discarding the pending tail. + inline bool pauseUnownedBlockedFallback() { + return unowned_blocked_fallback_enabled.exchange(0, std::memory_order_acq_rel) != 0; + } + inline void resumeUnownedBlockedFallback() { + unowned_blocked_fallback_enabled.store(1, std::memory_order_release); + } inline void enableUnownedBlockedFallback() { resetUnownedBlockedSampling(); unowned_blocked_fallback_enabled.store(1, std::memory_order_release); diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfilerTestSupport.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfilerTestSupport.java index 297a7aea93..cc89ba0c9d 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfilerTestSupport.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfilerTestSupport.java @@ -54,6 +54,20 @@ public static boolean recordUnownedWallTailAcrossDumpForTest( Thread.currentThread(), snapshot, beforePublication, resetBeforeTail); } + /** + * Seeds a blocked stack and five suppressed observations, dumps JFR, then invokes a real + * owned-block producer while profiling signals are blocked. Debug builds only. + * + * @param snapshot path receiving the chunk containing the initial blocked sample + * @param producer producer to invoke: 0 for paired API, 1 for generic block, 2 for park, + * or 3 for native monitor contention + * @return whether seeding, dumping and producer admission succeeded + */ + public static boolean recordUnownedWallTailBeforeOwnedBlockForTest(String snapshot, int producer) { + return recordUnownedWallTailBeforeOwnedBlockForTest0( + Thread.currentThread(), snapshot, producer); + } + private static native void setForceWallStartFailureForTest0(boolean force); private static native boolean isForceWallStartFailureArmedForTest0(); @@ -62,4 +76,7 @@ public static boolean recordUnownedWallTailAcrossDumpForTest( private static native boolean recordUnownedWallTailAcrossDumpForTest0( Thread thread, String snapshot, boolean beforePublication, boolean resetBeforeTail); + + private static native boolean recordUnownedWallTailBeforeOwnedBlockForTest0( + Thread thread, String snapshot, int producer); } diff --git a/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp b/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp new file mode 100644 index 0000000000..95e1915596 --- /dev/null +++ b/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp @@ -0,0 +1,236 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "guards.h" +#include "javaApi.h" +#include "profiler.h" +#include "taskBlockRecorder.h" +#include "threadLocalData.inline.h" + +class ProfilerTestAccessor { +public: + static void setProducerState(Profiler* profiler, bool enabled) { + profiler->_task_block_enabled.store(enabled, std::memory_order_release); + profiler->_task_block_monitor_events_enabled.store(enabled, std::memory_order_release); + } + static State exchangeState(Profiler* profiler, State state) { + return profiler->_state.exchange(state, std::memory_order_acq_rel); + } +}; + +class VMTestAccessor { +public: + static jvmtiEnv* exchangeJvmti(jvmtiEnv* jvmti) { + jvmtiEnv* previous = VM::_jvmti; + VM::_jvmti = jvmti; + return previous; + } +}; + +// Exercise the real producer lifecycle and registry with a minimal JNI thread +// identity. JFR stack capture is covered by the Java integration suites. +class TaskBlockProducersTest : public ::testing::Test { +protected: + void SetUp() override { + jni_functions.GetVersion = [](JNIEnv*) -> jint { return JNI_VERSION_1_8; }; + jni_functions.IsSameObject = [](JNIEnv*, jobject a, jobject b) -> jboolean { + return a == b ? JNI_TRUE : JNI_FALSE; + }; + jni_functions.DeleteLocalRef = [](JNIEnv*, jobject) {}; + jni.functions = &jni_functions; + jvmti_functions.GetCurrentThread = [](jvmtiEnv*, jthread* thread) -> jvmtiError { + *thread = testThread(); + return JVMTI_ERROR_NONE; + }; + jvmti.functions = &jvmti_functions; + previous_jvmti = VMTestAccessor::exchangeJvmti(&jvmti); + previous_state = ProfilerTestAccessor::exchangeState(profiler, RUNNING); + ProfilerTestAccessor::setProducerState(profiler, true); + Profiler::setTaskBlockRecordOverrideForTest( + [](int, jthread, int, TaskBlockEvent*) { + return Profiler::TaskBlockRecordResult::RECORDED; + }); + registry->init("", true); + current = ProfiledThread::initCurrentThreadSignalSafe(); + ASSERT_NE(nullptr, current); + current->setFilterSlotId(-1); + } + + void TearDown() override { + ProfilerTestAccessor::setProducerState(profiler, false); + ProfilerTestAccessor::exchangeState(profiler, previous_state); + Profiler::setTaskBlockRecordOverrideForTest(nullptr); + VMTestAccessor::exchangeJvmti(previous_jvmti); + registry->deactivateRecording(); + ProfiledThread::release(); + } + + static jthread testThread() { + static int identity; + return reinterpret_cast(&identity); + } + + u64 begin() { + return Java_com_datadoghq_profiler_JavaProfiler_beginTaskBlock0( + &jni, nullptr, testThread()); + } + + void end(u64 token) { + Java_com_datadoghq_profiler_JavaProfiler_endTaskBlock0( + &jni, nullptr, testThread(), token, 0, 0); + } + + WallClockBlockTracker::BlockState* block() { + return tracker->slotForId(current->filterSlotId()); + } + + SignalBlocker signal_blocker; + Profiler* profiler = Profiler::instance(); + ThreadFilter* registry = profiler->threadFilter(); + WallClockBlockTracker* tracker = profiler->blockTracker(); + ProfiledThread* current = nullptr; + JNINativeInterface_ jni_functions{}; + JNIEnv jni{}; + jvmtiInterface_1_ jvmti_functions{}; + jvmtiEnv jvmti{}; + jvmtiEnv* previous_jvmti = nullptr; + State previous_state = NEW; +}; + +TEST_F(TaskBlockProducersTest, PairedApiRecoversAbandonedIntervalAfterRestart) { + u64 old_token = begin(); + ASSERT_NE(0u, old_token); + int slot_id = current->filterSlotId(); + auto* identity = registry->slotForId(slot_id); + u64 lifecycle = identity->lifecycleGeneration(); + + registry->deactivateRecording(); + registry->init("", true); + // Another hook may refresh the registry first, independently of producer TLS. + ASSERT_EQ(slot_id, registry->ensureCurrentThreadSlot(current)); + u64 new_token = begin(); + ASSERT_NE(0u, new_token); + EXPECT_NE(old_token, new_token); + EXPECT_EQ(identity, registry->activeSlotForId(slot_id, current->tid())); + EXPECT_EQ(lifecycle, identity->lifecycleGeneration()); + + end(old_token); + EXPECT_EQ(new_token, current->taskBlockToken()); + EXPECT_EQ(BlockRunOwner::TASK_BLOCK, block()->activeBlockOwner()); + end(new_token); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); +} + +TEST_F(TaskBlockProducersTest, PairedApiRecoversAbandonedGenericHookAfterContextEntry) { + u64 old_token = Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( + &jni, nullptr, JNI_FALSE, static_cast(OSThreadState::SLEEPING)); + ASSERT_NE(0u, old_token); + ASSERT_TRUE(registry->add(current->tid(), current->filterSlotId())); + registry->remove(current->filterSlotId()); + ASSERT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); + + u64 token = begin(); + ASSERT_NE(0u, token); + EXPECT_NE(old_token, token); + EXPECT_EQ(0u, begin()) << "A nested begin must preserve the current owner"; + EXPECT_EQ(token, current->taskBlockToken()); + end(token); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); +} + +TEST_F(TaskBlockProducersTest, GenericExitAfterRestartClearsAbandonedTls) { + u64 token = Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( + &jni, nullptr, JNI_FALSE, static_cast(OSThreadState::SLEEPING)); + ASSERT_NE(0u, token); + registry->deactivateRecording(); + registry->init("", true); + Java_com_datadoghq_profiler_JavaProfiler_blockExit0( + &jni, nullptr, JNI_FALSE, token, 0, 0); + EXPECT_EQ(0u, current->taskBlockToken()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); + u64 next = begin(); + ASSERT_NE(0u, next); + end(next); +} + +TEST_F(TaskBlockProducersTest, MonitorRestartPreservesIdentityAndRejectsOldWaitExit) { + EXPECT_EQ(-1, current->filterSlotId()) << "Thread start must not reserve a slot"; + MonitorWait(&jvmti, &jni, testThread(), nullptr, 100); + u64 old_token = current->monitorBlockToken(); + ASSERT_NE(0u, old_token); + int slot_id = current->filterSlotId(); + auto* identity = registry->slotForId(slot_id); + u64 lifecycle = identity->lifecycleGeneration(); + + registry->deactivateRecording(); + registry->init("", true); + MonitorContendedEnter(&jvmti, &jni, testThread(), nullptr); + u64 new_token = current->monitorBlockToken(); + ASSERT_NE(0u, new_token); + EXPECT_NE(old_token, new_token); + EXPECT_EQ(slot_id, current->filterSlotId()); + EXPECT_EQ(lifecycle, identity->lifecycleGeneration()); + EXPECT_EQ(OSThreadState::MONITOR_WAIT, block()->activeBlockState()); + MonitorWaited(&jvmti, &jni, testThread(), nullptr, JNI_FALSE); + EXPECT_EQ(new_token, current->monitorBlockToken()); + MonitorContendedEntered(&jvmti, &jni, testThread(), nullptr); + EXPECT_EQ(0u, current->monitorBlockToken()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); +} + +TEST_F(TaskBlockProducersTest, InflightMonitorExitCleansUpAfterAdmissionCloses) { + MonitorWait(&jvmti, &jni, testThread(), nullptr, 100); + ASSERT_NE(0u, current->monitorBlockToken()); + ProfilerTestAccessor::setProducerState(profiler, false); + registry->deactivateRecording(); + MonitorWaited(&jvmti, &jni, testThread(), nullptr, JNI_FALSE); + EXPECT_EQ(0u, current->monitorBlockToken()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); +} + +TEST_F(TaskBlockProducersTest, ParkLifecycleDoesNotRequireRegistryAdmission) { + ProfilerTestAccessor::setProducerState(profiler, false); + registry->deactivateRecording(); + EXPECT_EQ(JNI_TRUE, Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + &jni, nullptr, testThread(), JNI_FALSE)); + EXPECT_EQ(-1, current->filterSlotId()); + EXPECT_EQ(0u, current->snapshotForTest().park_block_token); + EXPECT_EQ(JNI_FALSE, Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + &jni, nullptr, testThread(), JNI_FALSE)); + Java_com_datadoghq_profiler_JavaProfiler_parkExit0( + &jni, nullptr, testThread(), JNI_FALSE, 0, 0); + EXPECT_EQ(JNI_TRUE, Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + &jni, nullptr, testThread(), JNI_FALSE)); + Java_com_datadoghq_profiler_JavaProfiler_parkExit0( + &jni, nullptr, testThread(), JNI_FALSE, 0, 0); +} + +TEST_F(TaskBlockProducersTest, RotationDefersOwnershipWithoutDiscardingUnownedTail) { + int slot_id = registry->ensureCurrentThreadSlot(current); + ASSERT_GE(slot_id, 0); + block()->recordUnownedBlockedSample(0x1234, OSThreadState::SLEEPING); + ASSERT_TRUE(block()->shouldRecordUnownedBlockedSample()); + ASSERT_FALSE(block()->shouldRecordUnownedBlockedSample()); + profiler->beginTaskBlockRotationForTest(); + u64 token = enterOwnedBlockRun(current, slot_id, OSThreadState::CONDVAR_WAIT); + profiler->endTaskBlockRotationForTest(); + + EXPECT_EQ(0u, token); + EXPECT_EQ(1u, block()->unowned_blocked_pending_weight.load(std::memory_order_acquire)); + EXPECT_EQ(0x1234u, block()->unowned_blocked_call_trace_id.load(std::memory_order_acquire)); + EXPECT_TRUE(block()->unownedBlockedFallbackEnabled()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); +} + +TEST_F(TaskBlockProducersTest, RejectedContextAdmissionRestoresUnownedFallback) { + int slot_id = registry->ensureCurrentThreadSlot(current); + ASSERT_GE(slot_id, 0); + ASSERT_TRUE(registry->add(current->tid(), slot_id)); + EXPECT_EQ(0u, enterOwnedBlockRun(current, slot_id, OSThreadState::CONDVAR_WAIT)); + EXPECT_TRUE(block()->unownedBlockedFallbackEnabled()); + registry->remove(slot_id); +} diff --git a/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp index b635474cb4..ec984b90ac 100644 --- a/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadLocalDataPool_ut.cpp @@ -179,6 +179,7 @@ TEST(ProfiledThreadTest, UnclaimAndResetMatchesFreshInstance) { ProfiledThread* fresh = ProfiledThread::forTid(0); sigjmp_buf fake_jmp_buf; + dirty->refreshBlockState(17); dirty->set_unwinding_Java(true); dirty->setJmpCtx(&fake_jmp_buf); // Falls through to the cache-miss path (fresh _call_trace_id is 0), @@ -238,6 +239,7 @@ TEST(ProfiledThreadTest, UnclaimAndResetMatchesFreshInstance) { EXPECT_EQ(reset_snapshot.park_block_token, fresh_snapshot.park_block_token); EXPECT_EQ(reset_snapshot.task_block_start_ticks, fresh_snapshot.task_block_start_ticks); EXPECT_EQ(reset_snapshot.task_block_token, fresh_snapshot.task_block_token); + EXPECT_EQ(reset_snapshot.block_recording_epoch, fresh_snapshot.block_recording_epoch); EXPECT_EQ(reset_snapshot.task_block_context.spanId, fresh_snapshot.task_block_context.spanId); EXPECT_EQ(reset_snapshot.task_block_context.rootSpanId, fresh_snapshot.task_block_context.rootSpanId); for (u32 i = 0; i < DD_TAGS_CAPACITY; i++) { diff --git a/ddprof-lib/src/test/cpp/wallClockBlockRestart_ut.cpp b/ddprof-lib/src/test/cpp/wallClockBlockRestart_ut.cpp index 0eb1f468df..2ec89b7fae 100644 --- a/ddprof-lib/src/test/cpp/wallClockBlockRestart_ut.cpp +++ b/ddprof-lib/src/test/cpp/wallClockBlockRestart_ut.cpp @@ -9,16 +9,11 @@ #include #include #include "guards.h" +#include "javaApi.h" #include "profiler.h" #include "threadLocalData.inline.h" #include "wallClockBlockTracker.h" -extern "C" { -jlong JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockEnter0(JNIEnv*, jclass, jint); -void JNICALL Java_com_datadoghq_profiler_JavaProfiler_blockExit0(JNIEnv*, jclass, jlong); -void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(JNIEnv*, jclass); -void JNICALL Java_com_datadoghq_profiler_JavaProfiler_parkExit0(JNIEnv*, jclass, jlong, jlong); -} enum class RestartHook { BLOCK, PARK }; @@ -45,18 +40,20 @@ class WallClockBlockRestartTest : public ::testing::TestWithParam { u64 enterHook() { if (GetParam() == RestartHook::BLOCK) { return static_cast(Java_com_datadoghq_profiler_JavaProfiler_blockEnter0( - nullptr, nullptr, static_cast(OSThreadState::SLEEPING))); + nullptr, nullptr, JNI_FALSE, static_cast(OSThreadState::SLEEPING))); } - Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(nullptr, nullptr); + Java_com_datadoghq_profiler_JavaProfiler_parkEnter0( + nullptr, nullptr, nullptr, JNI_FALSE); return ProfiledThread::current()->snapshotForTest().park_block_token; } void exitHook(u64 token) { if (GetParam() == RestartHook::BLOCK) { Java_com_datadoghq_profiler_JavaProfiler_blockExit0( - nullptr, nullptr, static_cast(token)); + nullptr, nullptr, JNI_FALSE, static_cast(token), 0, 0); } else { - Java_com_datadoghq_profiler_JavaProfiler_parkExit0(nullptr, nullptr, 0, 0); + Java_com_datadoghq_profiler_JavaProfiler_parkExit0( + nullptr, nullptr, nullptr, JNI_FALSE, 0, 0); } } @@ -100,6 +97,27 @@ TEST_P(WallClockBlockRestartTest, HookWithoutRestartHasNormalLifecycle) { EXPECT_EQ(OSThreadState::UNKNOWN, block->activeBlockState()); } +TEST_P(WallClockBlockRestartTest, AbandonedHookDoesNotBlockNextRecording) { + u64 old_token = enterHook(); + ASSERT_NE(0u, old_token); + int slot_id = current->filterSlotId(); + auto* identity = registry->slotForId(slot_id); + u64 lifecycle = identity->lifecycleGeneration(); + + // No exit for the old interval: terminal notifications may be disabled + // during stop. The surviving owner must still be able to open a new run. + registry->deactivateRecording(); + registry->init("", true); + u64 new_token = enterHook(); + ASSERT_NE(0u, new_token); + EXPECT_NE(old_token, new_token); + EXPECT_EQ(slot_id, current->filterSlotId()); + EXPECT_EQ(identity, registry->activeSlotForId(slot_id, current->tid())); + EXPECT_EQ(lifecycle, identity->lifecycleGeneration()); + exitHook(new_token); + EXPECT_EQ(BlockRunOwner::NONE, tracker->slotForId(slot_id)->activeBlockOwner()); +} + TEST_P(WallClockBlockRestartTest, PausedHookDoesNotMutateReplacementAcrossRestart) { AdmissionPause pause; tracker->setBlockAdmissionHookForTest(AdmissionPause::pause, &pause); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockProducerCompatibilityTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockProducerCompatibilityTest.java new file mode 100644 index 0000000000..4b2fce127f --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockProducerCompatibilityTest.java @@ -0,0 +1,132 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.JfrEvents; +import com.datadoghq.profiler.ProfilerOwnedBlockHooks; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Verifies producer compatibility with retained identities and context-window transitions. */ +public class TaskBlockProducerCompatibilityTest extends AbstractProfilerTest { + private static final int SLEEPING = 7; + private static final long BLOCKER = 0x7401L; + private static final long UNBLOCKING_SPAN_ID = 0x7402L; + + /** A surviving worker can record again even when its previous producer exit was missed. */ + @ParameterizedTest + @ValueSource(strings = {"paired", "block", "park"}) + public void abandonedIntervalDoesNotPreventRecordingAfterRestart(String producer) + throws Exception { + ExecutorService worker = Executors.newSingleThreadExecutor(); + Path recording = Files.createTempFile("taskblock-producer-restart-", ".jfr"); + boolean started = false; + try { + long oldToken = worker.submit(() -> enter(producer)).get(5, TimeUnit.SECONDS); + assertTrue(oldToken != 0, "The original interval must be armed"); + stopProfiler(); + profiler.execute("start," + getProfilerCommand() + ",jfr,file=" + recording); + started = true; + worker.submit(() -> { + // Refresh through a different hook first: producer TLS must independently + // notice the recording epoch even if registry membership is already current. + profiler.addThread(); + profiler.removeThread(); + long token = enter(producer); + assertTrue(token != 0, "An abandoned interval must not strand producer TLS"); + if ("paired".equals(producer)) { + assertFalse(profiler.endTaskBlock(oldToken, BLOCKER, UNBLOCKING_SPAN_ID), + "The previous recording's token must not close the new interval"); + } + Thread.sleep(30L); + exit(producer, token); + return null; + }).get(5, TimeUnit.SECONDS); + profiler.stop(); + started = false; + + JfrEvents events = verifyEvents(recording, "datadog.TaskBlock", true); + TaskBlockAssertions.assertContains(events, 0L, 0L, BLOCKER, UNBLOCKING_SPAN_ID); + TaskBlockAssertions.assertContainsStackTrace(events); + TaskBlockAssertions.assertContainsJavaType(events, "TaskBlockProducerCompatibilityTest"); + } finally { + worker.shutdownNow(); + worker.awaitTermination(5, TimeUnit.SECONDS); + if (started) profiler.stop(); + Files.deleteIfExists(recording); + } + } + + /** Context entry retires a generic hook without poisoning a subsequent public API interval. */ + @Test + public void pairedApiRecoversAfterAbandonedGenericHookCrossesContextWindow() throws Exception { + ExecutorService worker = Executors.newSingleThreadExecutor(); + try { + worker.submit(() -> { + assertTrue(ProfilerOwnedBlockHooks.blockEnter(profiler, SLEEPING) != 0); + profiler.addThread(); + profiler.removeThread(); + long token = profiler.beginTaskBlock(); + assertTrue(token != 0, "The registry retired the old generic run on context entry"); + Thread.sleep(30L); + assertTrue(profiler.endTaskBlock(token, BLOCKER, UNBLOCKING_SPAN_ID)); + return null; + }).get(5, TimeUnit.SECONDS); + } finally { + worker.shutdownNow(); + worker.awaitTermination(5, TimeUnit.SECONDS); + } + stopProfiler(); + TaskBlockAssertions.assertContains( + verifyEvents("datadog.TaskBlock"), 0L, 0L, BLOCKER, UNBLOCKING_SPAN_ID); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,filter=,wallprecheck=true"; + } + + /** Opens a producer interval on the calling worker; parks use a boolean ownership marker. */ + private long enter(String producer) { + switch (producer) { + case "paired": + return profiler.beginTaskBlock(); + case "block": + return ProfilerOwnedBlockHooks.blockEnter(profiler, SLEEPING); + case "park": + return ProfilerOwnedBlockHooks.parkEnter(profiler) ? 1L : 0L; + default: + throw new IllegalArgumentException(producer); + } + } + + /** Closes the selected producer with metadata that must survive into its JFR event. */ + private void exit(String producer, long token) { + switch (producer) { + case "paired": + assertTrue(profiler.endTaskBlock(token, BLOCKER, UNBLOCKING_SPAN_ID)); + break; + case "block": + ProfilerOwnedBlockHooks.blockExit(profiler, token, BLOCKER, UNBLOCKING_SPAN_ID); + break; + case "park": + ProfilerOwnedBlockHooks.parkExit(profiler, BLOCKER, UNBLOCKING_SPAN_ID); + break; + default: + throw new IllegalArgumentException(producer); + } + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnownedTailBeforeTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnownedTailBeforeTaskBlockTest.java new file mode 100644 index 0000000000..afa5c1fbf5 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnownedTailBeforeTaskBlockTest.java @@ -0,0 +1,54 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.JavaProfilerTestSupport; +import com.datadoghq.profiler.JfrEvent; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Verifies that entering an owned block preserves the preceding weighted wall-clock tail. */ +public class UnownedTailBeforeTaskBlockTest extends AbstractProfilerTest { + /** All producer paths must replay suppressed observations with a stack retained across a dump. */ + @ParameterizedTest + @ValueSource(ints = {0, 1, 2, 3}) + public void ownedBlockPreservesUnownedTailAcrossDump(int producer) throws Exception { + Assumptions.assumeTrue(isDebugBuild(), "Requires synchronous debug sampling driver"); + long threadId = Thread.currentThread().getId(); + Path snapshot = Files.createTempFile("unowned-before-taskblock-", ".jfr"); + try { + assertTrue(JavaProfilerTestSupport.recordUnownedWallTailBeforeOwnedBlockForTest( + snapshot.toString(), producer)); + stopProfiler(); + long tailWeight = 0; + for (JfrEvent event : verifyEvents("datadog.MethodSample")) { + if (Long.valueOf(threadId).equals(event.getThreadJavaId("eventThread")) + && "SLEEPING".equals(event.getString("state")) + && "UNKNOWN".equals(event.getString("mode"))) { + tailWeight += event.getLong(WEIGHT, 0); + String stack = event.getStackTraceString(); + assertTrue(stack != null && stack.contains("UnownedTailBeforeTaskBlockTest"), + "The replayed tail must retain its stack across chunk rotation"); + } + } + assertEquals(5L, tailWeight, "Every suppressed observation must survive owned admission"); + } finally { + Files.deleteIfExists(snapshot); + } + } + + @Override + protected String getProfilerCommand() { + return "wall=1s,filter=,wallprecheck=true"; + } +} From 0bacae674570f0dc265ec8fdc20272f31b5eb4ec Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Wed, 21 Oct 2026 23:14:08 +0200 Subject: [PATCH 6/9] test: make monitor contention threshold coverage deterministic --- .../src/test/cpp/taskBlockRecorder_ut.cpp | 21 +++++++++++++++++++ .../wallclock/MonitorTaskBlockTest.java | 18 +++++++++------- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp index 9b555544b3..981f134848 100644 --- a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp +++ b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp @@ -85,6 +85,27 @@ TEST_F(TaskBlockRecorderTest, DurationThresholdIncludesExactBoundary) { EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_SKIPPED_TOO_SHORT)); } +TEST_F(TaskBlockRecorderTest, ShortContentionIsNotRecordedAndDoesNotSuppressNextEvent) { + Context context{}; + u64 start_ticks = TSC::ticks(); + u64 one_millisecond = TSC::frequency() / 1000; + ASSERT_GT(one_millisecond, 1ULL); + u64 passing_end = start_ticks + one_millisecond; + + EXPECT_FALSE(recordTaskBlockIfEligible( + 12345, nullptr, 0, start_ticks, passing_end - 1, context, 0, 0, + OSThreadState::MONITOR_WAIT)); + EXPECT_EQ(0, g_record_calls.load(std::memory_order_acquire)); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_SKIPPED_TOO_SHORT)); + EXPECT_EQ(0, Counters::getCounter(TASK_BLOCK_EMITTED)); + + EXPECT_TRUE(recordTaskBlockIfEligible( + 12345, nullptr, 0, start_ticks, passing_end, context, 0, 0, + OSThreadState::MONITOR_WAIT)); + EXPECT_EQ(1, g_record_calls.load(std::memory_order_acquire)); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_EMITTED)); +} + TEST_F(TaskBlockRecorderTest, RotationRejectsNewActivity) { Profiler* profiler = Profiler::instance(); profiler->beginTaskBlockRotationForTest(); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java index 967624a18c..32158cf216 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Assumptions; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -79,13 +80,15 @@ public void monitorContentionEmitsTaskBlockOutsideContextWindow() throws Excepti TaskBlockAssertions.assertContainsObservedState(events, "CONTENDED"); } + /** Verifies that repeated monitor contention leaves the producer able to record later blocks. */ @Test - public void shortMonitorContentionIsFilteredAndDoesNotSuppressLongerOnes() throws Exception { - Object shortMonitor = new Object(); + public void contentionBurstDoesNotSuppressLongerContention() throws Exception { + Object burstMonitor = new Object(); Object longMonitor = new Object(); - // Burst of genuinely contended, microsecond-long enters: two threads hammer the same - // monitor with an empty critical section, so no interval can reach the 1ms threshold. + // Exercise repeated contention before the positive control. An empty critical section + // can still exceed 1ms if a thread is descheduled, so burst events are allowed here. + // TaskBlockRecorderTest checks duration filtering with explicit timestamps. CountDownLatch start = new CountDownLatch(1); AtomicReference burstFailure = new AtomicReference<>(); int[] counter = new int[1]; @@ -95,20 +98,21 @@ public void shortMonitorContentionIsFilteredAndDoesNotSuppressLongerOnes() throw try { assertTrue(start.await(5, TimeUnit.SECONDS)); for (int n = 0; n < 20_000; n++) { - synchronized (shortMonitor) { + synchronized (burstMonitor) { counter[0]++; } } } catch (Throwable t) { burstFailure.set(t); } - }, "taskblock-short-contention-" + i); + }, "taskblock-burst-contention-" + i); burst[i].start(); } start.countDown(); for (Thread thread : burst) { assertCompleted(thread, burstFailure); } + assertEquals(40_000, counter[0], "contention burst did not complete"); // One long contended enter, the positive control: it proves the producer was alive. CountDownLatch attempting = new CountDownLatch(1); @@ -134,8 +138,6 @@ public void shortMonitorContentionIsFilteredAndDoesNotSuppressLongerOnes() throw JfrEvents events = verifyEvents("datadog.TaskBlock"); assertTrue(TaskBlockAssertions.containsBlocker(events, identityHash(longMonitor)), "long contention was not emitted"); - assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(shortMonitor)), - "sub-threshold contention must be filtered"); assertTaskBlockStackReference(events); } From d6afb4bffeb203cf04e3a0576a918a84d9dcd451 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 10:10:53 +0200 Subject: [PATCH 7/9] fix: fix api drift and classload failure in chaos tests --- ddprof-stresstest/build.gradle.kts | 11 ++- ddprof-stresstest/src/chaos/README.md | 10 +++ .../chaos/ParkBlockChurnAntagonist.java | 43 +++++------ .../profiler/chaos/ProfilerAccess.java | 46 ++++++++++++ .../chaos/ReapplyContextValueAntagonist.java | 62 +++++++++------ .../profiler/chaos/ProfilerAccessTest.java | 75 +++++++++++++++++++ 6 files changed, 200 insertions(+), 47 deletions(-) create mode 100644 ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ProfilerAccess.java create mode 100644 ddprof-stresstest/src/chaosTest/java/com/datadoghq/profiler/chaos/ProfilerAccessTest.java diff --git a/ddprof-stresstest/build.gradle.kts b/ddprof-stresstest/build.gradle.kts index b4753c7308..2f4ad287cf 100644 --- a/ddprof-stresstest/build.gradle.kts +++ b/ddprof-stresstest/build.gradle.kts @@ -86,8 +86,8 @@ dependencies { // dd-trace-api: annotations only at compile time. The patched dd-java-agent // provides the (relocated) runtime classes and intercepts @Trace. "chaosCompileOnly"(libs.dd.trace.api) - // ddprof-lib public API: compile-only; the patched dd-java-agent provides the - // classes at runtime for antagonists that call JavaProfiler directly. + // ddprof-lib public API: compile-only. At runtime ProfilerAccess binds calls + // through the patched agent's isolated loader instead of the application loader. "chaosCompileOnly"(project(mapOf("path" to ":ddprof-lib", "configuration" to "debug"))) "chaosTestImplementation"(libs.bundles.testing) "chaosTestImplementation"(project(mapOf("path" to ":ddprof-lib", "configuration" to "debug"))) @@ -113,6 +113,13 @@ tasks.register("testOwnedBlockPrecheck") { testClassesDirs = sourceSets["chaosTest"].output.classesDirs classpath = sourceSets["chaosTest"].runtimeClasspath setExecutable(PlatformUtils.testJavaExecutable()) + dependsOn("chaosJar") + systemProperty("chaos.harness.jar", layout.buildDirectory.file("libs/chaos.jar").get().asFile.absolutePath) + providers.gradleProperty("chaosAgentJar").orNull?.let { + val agentJar = file(it) + systemProperty("chaos.agent.jar", agentJar.absolutePath) + inputs.file(agentJar).withPropertyName("chaosAgentJar") + } useJUnitPlatform() } diff --git a/ddprof-stresstest/src/chaos/README.md b/ddprof-stresstest/src/chaos/README.md index 06b2dee22e..f43398eb76 100644 --- a/ddprof-stresstest/src/chaos/README.md +++ b/ddprof-stresstest/src/chaos/README.md @@ -8,6 +8,16 @@ built `ddprof.jar` (see `utils/patch-dd-java-agent.sh`). Native crashes and workload failures produce a non-zero exit. Native crashes also leave an `hs_err_pid*.log` captured by the runner script. +The profiler-aware workloads bind method handles through the attached agent's +isolated class loader. The application jar does not contain a second copy of +`JavaProfiler`. Standalone native tests resolve the profiler on their test classpath. +To also check the packaged harness with a real patched agent in both profiler-only +and profiler-plus-tracer modes, pass its path to the test task: + +```bash +./.claude/commands/build-and-summarize :ddprof-stresstest:testOwnedBlockPrecheck -PchaosAgentJar=/path/to/dd-java-agent-patched.jar +``` + ## Antagonist roster | Name | Targets | diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ParkBlockChurnAntagonist.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ParkBlockChurnAntagonist.java index 49e4c7f7ab..8659d2b84f 100644 --- a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ParkBlockChurnAntagonist.java +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ParkBlockChurnAntagonist.java @@ -9,10 +9,7 @@ */ package com.datadoghq.profiler.chaos; -import com.datadoghq.profiler.JavaProfiler; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.reflect.Method; import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -47,7 +44,7 @@ public final class ParkBlockChurnAntagonist implements Antagonist { private final AtomicLong parkCalls = new AtomicLong(); private final AtomicReference failure = new AtomicReference<>(); private final Object monitor = new Object(); - private JavaProfiler profiler; + private MethodHandle getDebugCounters; private MethodHandle blockEnter; private MethodHandle blockExit; private MethodHandle parkEnter; @@ -74,15 +71,16 @@ public String name() { @Override public void start() { try { - profiler = JavaProfiler.getInstance(); - blockEnter = bindHook("blockEnter", int.class); - blockExit = bindHook("blockExit", long.class); - parkEnter = bindHook("parkEnter"); - parkExit = bindHook("parkExit", long.class, long.class); + Object profiler = ProfilerAccess.getInstance(); + blockEnter = ProfilerAccess.bind(profiler, "blockEnter", int.class); + blockExit = ProfilerAccess.bind(profiler, "blockExit", long.class, long.class, long.class); + parkEnter = ProfilerAccess.bind(profiler, "parkEnter"); + parkExit = ProfilerAccess.bind(profiler, "parkExit", long.class, long.class); + getDebugCounters = ProfilerAccess.bind(profiler, "getDebugCounters"); } catch (Exception e) { throw new IllegalStateException("Cannot bind profiler owned-block hooks", e); } - countersBefore = profiler.getDebugCounters(); + countersBefore = debugCounters(); running = true; driver = new Thread(() -> { try { @@ -110,7 +108,7 @@ public void stopGracefully(Duration timeout) { if (failure.get() != null) throw new IllegalStateException("Owned-block worker failed", failure.get()); System.out.println("[chaos] park-block-churn completed=" + totalBlocks.get() + " armed_sleeps=" + armedSleepingRuns.get() + " park_calls=" + parkCalls.get() - + " suppressed=" + counterDelta("wc_signals_suppressed_sampled_run")); + + " suppressed=" + counterDelta("wc_signals_suppressed_owned_block")); } /** Requires native hooks, unfiltered timer lookups, and timer-side suppression to run. */ @@ -120,7 +118,7 @@ public void verifyPrecheckCoverage() { } for (String counter : new String[] { "wc_precheck_registry_lookups", "wc_precheck_candidates_rejected", - "wc_signals_suppressed_sampled_run"}) { + "wc_signals_suppressed_owned_block"}) { long delta = counterDelta(counter); if (delta <= 0) { throw new IllegalStateException("No precheck coverage for " + counter @@ -129,16 +127,19 @@ public void verifyPrecheckCoverage() { } } - /** Resolves hooks on JavaProfiler's runtime class, including the injected agent's loader. */ - private MethodHandle bindHook(String name, Class... parameterTypes) throws ReflectiveOperationException { - Method method = JavaProfiler.class.getDeclaredMethod(name, parameterTypes); - method.setAccessible(true); - return MethodHandles.lookup().unreflect(method).bindTo(profiler); + /** Reads counters from the same profiler instance as the owned-block hooks. */ + @SuppressWarnings("unchecked") + private Map debugCounters() { + try { + return (Map) getDebugCounters.invokeExact(); + } catch (Throwable t) { + throw new IllegalStateException("Cannot read profiler counters", t); + } } private long counterDelta(String name) { long before = countersBefore.getOrDefault(name, -1L); - long after = profiler.getDebugCounters().getOrDefault(name, -1L); + long after = debugCounters().getOrDefault(name, -1L); return before < 0 || after < 0 ? -1 : after - before; } @@ -190,12 +191,12 @@ private void block(int mode) { sleepOwned(blockMillis); break; case 1: - parkEnter.invokeExact(); + boolean ownsPark = (boolean) parkEnter.invokeExact(); parkCalls.incrementAndGet(); try { LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(blockMillis)); } finally { - parkExit.invokeExact(0L, 0L); + if (ownsPark) parkExit.invokeExact(0L, 0L); } break; case 2: @@ -228,7 +229,7 @@ private void sleepOwned(long millis) throws Throwable { try { Thread.sleep(millis); } finally { - blockExit.invokeExact(token); + blockExit.invokeExact(token, 0L, 0L); } } } diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ProfilerAccess.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ProfilerAccess.java new file mode 100644 index 0000000000..cd5f74c14c --- /dev/null +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ProfilerAccess.java @@ -0,0 +1,46 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package com.datadoghq.profiler.chaos; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; +import java.lang.reflect.Method; + +/** Binds calls to the agent's profiler without loading another copy on the application classpath. */ +final class ProfilerAccess { + private ProfilerAccess() {} + + /** Uses the agent's isolated loader when attached, or the test classpath in standalone runs. */ + static Object getInstance() throws ReflectiveOperationException { + ClassLoader loader = ProfilerAccess.class.getClassLoader(); + Class agent; + try { + agent = Class.forName("datadog.trace.bootstrap.Agent", false, loader); + } catch (ClassNotFoundException standalone) { + return Class.forName("com.datadoghq.profiler.JavaProfiler", true, loader) + .getMethod("getInstance").invoke(null); + } + + // Agent stores shared/*.classdata behind this loader; adding ddprof.jar to the + // application classpath would create a separate JavaProfiler singleton instead. + Field field = agent.getDeclaredField("AGENT_CLASSLOADER"); + field.setAccessible(true); + ClassLoader agentLoader = (ClassLoader) field.get(null); + if (agentLoader == null) { + throw new IllegalStateException("Datadog agent class loader is not initialized"); + } + return Class.forName("com.datadoghq.profiler.JavaProfiler", true, agentLoader) + .getMethod("getInstance").invoke(null); + } + + /** Resolves a hook once, retaining its exact signature and avoiding reflective call allocations. */ + static MethodHandle bind(Object profiler, String name, Class... parameterTypes) + throws ReflectiveOperationException { + Method method = profiler.getClass().getDeclaredMethod(name, parameterTypes); + method.setAccessible(true); + return MethodHandles.lookup().unreflect(method).bindTo(profiler); + } +} diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextValueAntagonist.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextValueAntagonist.java index a8a84320ee..8cd8a30b55 100644 --- a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextValueAntagonist.java +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextValueAntagonist.java @@ -9,7 +9,7 @@ */ package com.datadoghq.profiler.chaos; -import com.datadoghq.profiler.JavaProfiler; +import java.lang.invoke.MethodHandle; import java.time.Duration; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -42,6 +42,10 @@ public final class ReapplyContextValueAntagonist implements Antagonist { private final int workerCount; private final ExecutorService pool; private volatile boolean running; + private MethodHandle setTraceContext; + private MethodHandle clearTraceContext; + private MethodHandle setContextValue; + private MethodHandle clearContextValue; public ReapplyContextValueAntagonist() { this(8); @@ -64,8 +68,20 @@ public String name() { return "reapply-context-value"; } + /** Resolves context hooks before launching workers so class-loading failures fail startup. */ @Override public void start() { + try { + Object profiler = ProfilerAccess.getInstance(); + setTraceContext = ProfilerAccess.bind(profiler, "setTraceContext", + long.class, long.class, long.class, long.class, + int.class, CharSequence.class, int.class, CharSequence.class); + clearTraceContext = ProfilerAccess.bind(profiler, "clearTraceContext"); + setContextValue = ProfilerAccess.bind(profiler, "setContextValue", int.class, String.class); + clearContextValue = ProfilerAccess.bind(profiler, "clearContextValue", int.class); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException("Cannot bind profiler context hooks", e); + } running = true; for (int i = 0; i < workerCount; i++) { pool.execute(this::workerLoop); @@ -84,32 +100,30 @@ public void stopGracefully(Duration timeout) { } private void workerLoop() { - JavaProfiler profiler; try { - profiler = JavaProfiler.getInstance(); - } catch (Exception e) { - System.err.println("[chaos] reapply-context-value: failed to get profiler: " + e); - return; - } - - long spanId = Thread.currentThread().getId() + 1; - long localRootSpanId = spanId * 31L; - long traceIdLow = spanId * 6364136223846793005L + 1442695040888963407L; + long spanId = Thread.currentThread().getId() + 1; + long localRootSpanId = spanId * 31L; + long traceIdLow = spanId * 6364136223846793005L + 1442695040888963407L; - while (running) { - // Span activation resets all custom slots. - profiler.setTraceContext(localRootSpanId, spanId, 0, traceIdLow, -1, null, -1, null); - for (int i = 0; i < SLOT_COUNT; i++) { - checkSetContextValue(profiler.setContextValue(i, ROUTES[i % ROUTES.length]), i); + while (running) { + // Span activation resets all custom slots. + setTraceContext.invokeExact(localRootSpanId, spanId, 0L, traceIdLow, + -1, (CharSequence) null, -1, (CharSequence) null); + for (int i = 0; i < SLOT_COUNT; i++) { + checkSetContextValue((boolean) setContextValue.invokeExact(i, ROUTES[i % ROUTES.length]), i); + } + for (int i = 0; i < SLOT_COUNT; i += 2) { + clearContextValue.invokeExact(i); + } + for (int i = 0; i < SLOT_COUNT; i += 2) { + int route = (i + ThreadLocalRandom.current().nextInt(ROUTES.length)) % ROUTES.length; + checkSetContextValue((boolean) setContextValue.invokeExact(i, ROUTES[route]), i); + } + clearTraceContext.invokeExact(); } - for (int i = 0; i < SLOT_COUNT; i += 2) { - profiler.clearContextValue(i); - } - for (int i = 0; i < SLOT_COUNT; i += 2) { - int route = (i + ThreadLocalRandom.current().nextInt(ROUTES.length)) % ROUTES.length; - checkSetContextValue(profiler.setContextValue(i, ROUTES[route]), i); - } - profiler.clearTraceContext(); + } catch (Throwable t) { + t.printStackTrace(System.err); + Runtime.getRuntime().halt(1); } } diff --git a/ddprof-stresstest/src/chaosTest/java/com/datadoghq/profiler/chaos/ProfilerAccessTest.java b/ddprof-stresstest/src/chaosTest/java/com/datadoghq/profiler/chaos/ProfilerAccessTest.java new file mode 100644 index 0000000000..f54937d697 --- /dev/null +++ b/ddprof-stresstest/src/chaosTest/java/com/datadoghq/profiler/chaos/ProfilerAccessTest.java @@ -0,0 +1,75 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package com.datadoghq.profiler.chaos; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.datadoghq.profiler.JavaProfiler; +import java.lang.invoke.MethodHandle; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** Exercises standalone resolution and the packaged harness under an actual patched Java agent. */ +class ProfilerAccessTest { + /** Standalone bindings must use the singleton already loaded by the test classpath. */ + @Test + void standaloneBindingsUseExistingProfiler() throws Throwable { + JavaProfiler profiler = JavaProfiler.getInstance(); + Object resolved = ProfilerAccess.getInstance(); + assertSame(profiler, resolved); + MethodHandle setContextValue = ProfilerAccess.bind(resolved, "setContextValue", int.class, String.class); + MethodHandle clearContextValue = ProfilerAccess.bind(resolved, "clearContextValue", int.class); + assertTrue((boolean) setContextValue.invokeExact(0, "chaos-loader-test")); + clearContextValue.invokeExact(0); + } + + /** Both CI modes must run the two profiler-aware workloads without an application ddprof jar. */ + @ParameterizedTest + @ValueSource(booleans = {false, true}) + @Timeout(90) + void packagedHarnessUsesAgentProfiler(boolean tracing) throws Exception { + String agent = System.getProperty("chaos.agent.jar"); + Assumptions.assumeTrue(agent != null, "Supply -PchaosAgentJar= for agent integration"); + Path output = Files.createTempFile("chaos-agent-loader-", ".log"); + Process process = null; + try { + process = new ProcessBuilder( + Paths.get(System.getProperty("java.home"), "bin", "java").toString(), + "-javaagent:" + agent, + "-Ddd.trace.enabled=" + tracing, + "-Ddd.profiling.enabled=true", + "-Ddd.profiling.start-force-first=true", + "-Ddd.profiling.ddprof.wall.enabled=true", + "-Ddd.telemetry.enabled=false", + "-Ddd.remote_config.enabled=false", + "-Xmx256m", + "-jar", System.getProperty("chaos.harness.jar"), + "--duration", "2s", "--antagonists", "reapply-context-value,park-block-churn") + .redirectErrorStream(true).redirectOutput(output.toFile()).start(); + assertTrue(process.waitFor(60, TimeUnit.SECONDS), "Patched-agent chaos process did not terminate: " + output); + String log = new String(Files.readAllBytes(output), StandardCharsets.UTF_8); + assertEquals(0, process.exitValue(), log); + assertTrue(log.contains("antagonist started: reapply-context-value"), log); + assertTrue(log.contains("park-block-churn completed="), log); + assertTrue(log.contains("completed cleanly"), log); + } finally { + if (process != null && process.isAlive()) { + process.destroyForcibly(); + process.waitFor(5, TimeUnit.SECONDS); + } + Files.deleteIfExists(output); + } + } +} From 7ae01fa11cc43430a5ce0150159f4b5433b1f0b7 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 18:01:13 +0200 Subject: [PATCH 8/9] fix: address openj9 vt recognition issue --- ddprof-lib/src/main/cpp/jvmSupport.cpp | 1 - ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | 21 +++- ddprof-lib/src/main/cpp/taskBlockRecorder.h | 5 + ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | 90 ++++------------- .../src/test/cpp/taskBlockProducers_ut.cpp | 99 +++++++++++++++++++ 5 files changed, 140 insertions(+), 76 deletions(-) diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 2a13bcf6eb..ef5fb54786 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -43,7 +43,6 @@ bool JVMSupport::isPlatformThread(JNIEnv* jni, jthread thread) { jint jni_version = jni->GetVersion(); if (jni_version <= 0) return false; if (jni_version < JNI_VERSION_19_VALUE) return true; - if (!VM::isHotspot()) return true; const JniFunction* functions = reinterpret_cast(jni->functions); diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp index 506a953d8b..ef62d590e3 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp @@ -120,6 +120,21 @@ bool finishTaskBlockAtExit(ProfiledThread* current, blocker, unblocking_span_id, snapshot.active_state, true); } +#ifdef UNIT_TEST +static u64 g_monitor_ticks_for_test = 0; + +void setMonitorTaskBlockTicksForTest(u64 ticks) { + g_monitor_ticks_for_test = ticks; +} +#endif + +static inline u64 monitorTicks() { +#ifdef UNIT_TEST + if (g_monitor_ticks_for_test != 0) return g_monitor_ticks_for_test; +#endif + return TSC::ticks(); +} + static u64 monitorBlockerHash(jvmtiEnv *jvmti, jobject object) { if (object == NULL) return 0; jint hash = 0; @@ -151,7 +166,7 @@ static void monitorBlockEnter(JNIEnv *jni, jthread thread, ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current); if (!tf->unfilteredWallTrackingActive() || slot_id < 0) return; current->refreshBlockState(tf->slotForId(slot_id)->recordingEpoch()); - if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { + if (!current->monitorEnter(monitorTicks(), context, /*blocker=*/0, state)) { u64 token = current->monitorBlockToken(); bool current_owner = false; if (token != 0) { @@ -171,7 +186,7 @@ static void monitorBlockEnter(JNIEnv *jni, jthread thread, return; } current->clearMonitorBlock(); - if (!current->monitorEnter(TSC::ticks(), context, /*blocker=*/0, state)) { + if (!current->monitorEnter(monitorTicks(), context, /*blocker=*/0, state)) { return; } } @@ -212,7 +227,7 @@ static void monitorBlockExit(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread, // an identity hash also prevents the object from being biased again. // These conditions mirror taskBlockPassesBasicEligibility, and the same // end_ticks is handed down so there is no boundary drift. - u64 end_ticks = TSC::ticks(); + u64 end_ticks = monitorTicks(); if (profiler->taskBlockEnabled() && profiler->nativeMonitorTaskBlockEnabled() && context.spanId == 0 && exceedsMinTaskBlockDuration(start_ticks, end_ticks)) { blocker = monitorBlockerHash(jvmti, object); diff --git a/ddprof-lib/src/main/cpp/taskBlockRecorder.h b/ddprof-lib/src/main/cpp/taskBlockRecorder.h index f400e30f70..df91045f18 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -15,6 +15,11 @@ void initializeTaskBlockDurationThreshold(); bool exceedsMinTaskBlockDuration(u64 start_ticks, u64 end_ticks); +#ifdef UNIT_TEST +// Overrides monitor callback timestamps; zero restores the real clock. +void setMonitorTaskBlockTicksForTest(u64 ticks); +#endif + // Starts a producer-owned run after flushing the preceding unowned wall tail. // Called on the slot's owning thread, so pausing fallback accumulation also // prevents a profiling signal from adding weight between the flush and admission. diff --git a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp index ceb5b728aa..d833d86e81 100644 --- a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp @@ -67,7 +67,8 @@ class JVMSupportTestAccessor { static constexpr LoadState FullyLoaded() { return JVMSupport::Fully_loaded; } }; -class JvmSupportThreadClassificationTest : public ::testing::Test { +// Run the JNI classification contract with VM::isHotspot() both false and true. +class JvmSupportThreadClassificationTest : public ::testing::TestWithParam { protected: using JniFunction = void (JNICALL*)(); @@ -96,7 +97,7 @@ class JvmSupportThreadClassificationTest : public ::testing::Test { void SetUp() override { _orig_hotspot = VMTestAccessor::getHotspot(); - VMTestAccessor::setHotspot(true); + VMTestAccessor::setHotspot(GetParam()); jni_version = 0x00150000; virtual_thread = JNI_FALSE; is_virtual_thread_calls = 0; @@ -114,32 +115,35 @@ class JvmSupportThreadClassificationTest : public ::testing::Test { } }; -TEST_F(JvmSupportThreadClassificationTest, NullInputsFailClosed) { +INSTANTIATE_TEST_SUITE_P(JvmImplementations, JvmSupportThreadClassificationTest, + ::testing::Bool()); + +TEST_P(JvmSupportThreadClassificationTest, NullInputsFailClosed) { EXPECT_FALSE(JVMSupport::isPlatformThread(nullptr, thread)); EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, nullptr)); } -TEST_F(JvmSupportThreadClassificationTest, InvalidJniVersionFailsClosed) { +TEST_P(JvmSupportThreadClassificationTest, InvalidJniVersionFailsClosed) { jni_version = 0; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(0, is_virtual_thread_calls); } -TEST_F(JvmSupportThreadClassificationTest, PreJni19ThreadIsPlatform) { +TEST_P(JvmSupportThreadClassificationTest, PreJni19ThreadIsPlatform) { jni_version = 0x000a0000; function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(0, is_virtual_thread_calls); } -TEST_F(JvmSupportThreadClassificationTest, Jni19PlatformThreadIsAccepted) { +TEST_P(JvmSupportThreadClassificationTest, Jni19PlatformThreadIsAccepted) { jni_version = 0x00130000; EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(1, is_virtual_thread_calls); EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, Jni19VirtualThreadIsRejected) { +TEST_P(JvmSupportThreadClassificationTest, Jni19VirtualThreadIsRejected) { jni_version = 0x00130000; virtual_thread = JNI_TRUE; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); @@ -147,21 +151,21 @@ TEST_F(JvmSupportThreadClassificationTest, Jni19VirtualThreadIsRejected) { EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, MissingJni19FunctionFailsClosed) { +TEST_P(JvmSupportThreadClassificationTest, MissingJni19FunctionFailsClosed) { jni_version = 0x00130000; function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(0, is_virtual_thread_calls); } -TEST_F(JvmSupportThreadClassificationTest, Jni20PlatformThreadIsAccepted) { +TEST_P(JvmSupportThreadClassificationTest, Jni20PlatformThreadIsAccepted) { jni_version = 0x00140000; EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(1, is_virtual_thread_calls); EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, Jni20VirtualThreadIsRejected) { +TEST_P(JvmSupportThreadClassificationTest, Jni20VirtualThreadIsRejected) { jni_version = 0x00140000; virtual_thread = JNI_TRUE; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); @@ -169,90 +173,32 @@ TEST_F(JvmSupportThreadClassificationTest, Jni20VirtualThreadIsRejected) { EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, MissingJni20FunctionFailsClosed) { +TEST_P(JvmSupportThreadClassificationTest, MissingJni20FunctionFailsClosed) { jni_version = 0x00140000; function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(0, is_virtual_thread_calls); } -TEST_F(JvmSupportThreadClassificationTest, Jni21PlatformThreadIsAccepted) { +TEST_P(JvmSupportThreadClassificationTest, Jni21PlatformThreadIsAccepted) { EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(1, is_virtual_thread_calls); EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, Jni21VirtualThreadIsRejected) { +TEST_P(JvmSupportThreadClassificationTest, Jni21VirtualThreadIsRejected) { virtual_thread = JNI_TRUE; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(1, is_virtual_thread_calls); EXPECT_EQ(thread, last_thread); } -TEST_F(JvmSupportThreadClassificationTest, MissingJni21FunctionFailsClosed) { +TEST_P(JvmSupportThreadClassificationTest, MissingJni21FunctionFailsClosed) { function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); EXPECT_EQ(0, is_virtual_thread_calls); } -// --------------------------------------------------------------------------- -// JvmSupportNonHotspotTest — verifies isPlatformThread() short-circuits to -// true on non-HotSpot JVMs at JNI>=19 without ever indexing the HotSpot-only -// IsVirtualThread vtable slot (which may not even be a valid function -// pointer there). -// --------------------------------------------------------------------------- -class JvmSupportNonHotspotTest : public ::testing::Test { -protected: - using JniFunction = void (JNICALL*)(); - - static constexpr int GET_VERSION_INDEX = 4; - static constexpr int IS_VIRTUAL_THREAD_INDEX = 234; - static constexpr int FUNCTION_TABLE_SIZE = IS_VIRTUAL_THREAD_INDEX + 1; - - inline static jint jni_version; - inline static int is_virtual_thread_calls; - - JniFunction function_table[FUNCTION_TABLE_SIZE]{}; - JNIEnv jni{}; - _jobject thread_object; - jthread thread = &thread_object; - bool _orig_hotspot = false; - - static jint JNICALL getVersion(JNIEnv*) { return jni_version; } - static jboolean JNICALL isVirtualThread(JNIEnv*, jobject) { - is_virtual_thread_calls++; - return JNI_TRUE; - } - - void SetUp() override { - _orig_hotspot = VMTestAccessor::getHotspot(); - VMTestAccessor::setHotspot(false); - jni_version = 0x00150000; - is_virtual_thread_calls = 0; - function_table[GET_VERSION_INDEX] = - reinterpret_cast(&getVersion); - function_table[IS_VIRTUAL_THREAD_INDEX] = - reinterpret_cast(&isVirtualThread); - jni.functions = - reinterpret_cast(function_table); - } - - void TearDown() override { - VMTestAccessor::setHotspot(_orig_hotspot); - } -}; - -TEST_F(JvmSupportNonHotspotTest, Jni21ThreadIsAcceptedWithoutIndexingVtable) { - EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); - EXPECT_EQ(0, is_virtual_thread_calls); -} - -TEST_F(JvmSupportNonHotspotTest, PreJni19ThreadIsStillAccepted) { - jni_version = 0x000a0000; - EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); - EXPECT_EQ(0, is_virtual_thread_calls); -} - // --------------------------------------------------------------------------- // ProfilerTestAccessor — friend of Profiler, lets tests force the internal // state machine to a known value so checkState()/start()/check() can be diff --git a/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp b/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp index 95e1915596..4878b37377 100644 --- a/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp +++ b/ddprof-lib/src/test/cpp/taskBlockProducers_ut.cpp @@ -192,6 +192,105 @@ TEST_F(TaskBlockProducersTest, InflightMonitorExitCleansUpAfterAdmissionCloses) EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); } +class MonitorTaskBlockHashTest : public TaskBlockProducersTest, + public ::testing::WithParamInterface { +protected: + inline static int hash_calls; + inline static jobject hashed_object; + inline static int record_calls; + inline static TaskBlockEvent recorded_event; + static constexpr jint MONITOR_HASH = -12345; + _jobject monitor; + + void SetUp() override { + TaskBlockProducersTest::SetUp(); + Counters::reset(); + initializeTaskBlockDurationThreshold(); + hash_calls = 0; + hashed_object = nullptr; + record_calls = 0; + recorded_event = {}; + jvmti_functions.GetObjectHashCode = [](jvmtiEnv*, jobject object, jint* hash) -> jvmtiError { + hash_calls++; + hashed_object = object; + *hash = MONITOR_HASH; + return JVMTI_ERROR_NONE; + }; + Profiler::setTaskBlockRecordOverrideForTest( + [](int, jthread, int, TaskBlockEvent* event) { + record_calls++; + recorded_event = *event; + return Profiler::TaskBlockRecordResult::RECORDED; + }); + } + + void TearDown() override { + setMonitorTaskBlockTicksForTest(0); + TaskBlockProducersTest::TearDown(); + Counters::reset(); + } + + void enterMonitor() { + if (GetParam()) { + MonitorWait(&jvmti, &jni, testThread(), &monitor, 100); + } else { + MonitorContendedEnter(&jvmti, &jni, testThread(), &monitor); + } + } + + void exitMonitor() { + if (GetParam()) { + MonitorWaited(&jvmti, &jni, testThread(), &monitor, JNI_FALSE); + } else { + MonitorContendedEntered(&jvmti, &jni, testThread(), &monitor); + } + } +}; + +INSTANTIATE_TEST_SUITE_P(MonitorCallbacks, MonitorTaskBlockHashTest, ::testing::Bool()); + +TEST_P(MonitorTaskBlockHashTest, ShortBlockSkipsHashAndNextEligibleBlockHashesOnce) { + u64 min_ticks = TSC::frequency() / 1000; + ASSERT_GT(min_ticks, 1u); + u64 start_ticks = 100; + setMonitorTaskBlockTicksForTest(start_ticks); + enterMonitor(); + ASSERT_NE(0u, current->monitorBlockToken()); + EXPECT_EQ(0, hash_calls) << "Entry must not resolve the monitor hash"; + + setMonitorTaskBlockTicksForTest(start_ticks + min_ticks - 1); + exitMonitor(); + EXPECT_EQ(0, hash_calls) << "A below-threshold exit must not resolve the monitor hash"; + EXPECT_EQ(0, record_calls); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_SKIPPED_TOO_SHORT)); + EXPECT_EQ(0u, current->monitorBlockToken()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); + + start_ticks += min_ticks + 100; + setMonitorTaskBlockTicksForTest(start_ticks); + enterMonitor(); + ASSERT_NE(0u, current->monitorBlockToken()); + EXPECT_EQ(0, hash_calls); + + setMonitorTaskBlockTicksForTest(start_ticks + min_ticks); + exitMonitor(); + EXPECT_EQ(1, hash_calls); + EXPECT_EQ(&monitor, hashed_object); + EXPECT_EQ(1, record_calls); + EXPECT_EQ(static_cast(static_cast(MONITOR_HASH)), recorded_event._blocker); + EXPECT_EQ(start_ticks, recorded_event._start); + EXPECT_EQ(start_ticks + min_ticks, recorded_event._end); + EXPECT_EQ(GetParam() ? OSThreadState::OBJECT_WAIT : OSThreadState::MONITOR_WAIT, + recorded_event._observedBlockingState); + EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_EMITTED)); + EXPECT_EQ(0u, current->monitorBlockToken()); + EXPECT_EQ(BlockRunOwner::NONE, block()->activeBlockOwner()); + + exitMonitor(); + EXPECT_EQ(1, hash_calls); + EXPECT_EQ(1, record_calls); +} + TEST_F(TaskBlockProducersTest, ParkLifecycleDoesNotRequireRegistryAdmission) { ProfilerTestAccessor::setProducerState(profiler, false); registry->deactivateRecording(); From f90f10400071d79e5a67f9f8ece3a9a929957fa2 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 18:01:59 +0200 Subject: [PATCH 9/9] fix: address weak tests --- .../datadoghq/profiler/ExternalLauncher.java | 22 ++++---- .../datadoghq/profiler/JavaProfilerTest.java | 51 ++++++++++++------- .../wallclock/MonitorTaskBlockTest.java | 41 +++++++++++++-- 3 files changed, 81 insertions(+), 33 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java index f3fa9c3781..d6e8b26499 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -72,8 +72,8 @@ *

  • profiler-java-delegation-reuse:<delegated> - verifies compatible Java singleton ownership reuse
    • *
  • profiler-java-delegation-conflict:<initial>:<requested> - verifies conflicting Java singleton ownership requests
    • *
  • profiler-java-delegation-legacy-reuse:<initial> - verifies the legacy no-preference overloads reuse the existing singleton
    • - *
  • profiler-preexisting-monitor-wait - exercises Object.wait on a thread created before profiler initialization
    • - *
  • profiler-preexisting-monitor-contention - exercises monitor contention on a thread created before profiler initialization
    • + *
  • profiler-preexisting-monitor-wait <recording path> - records Object.wait on a thread created before profiler initialization
    • + *
  • profiler-preexisting-monitor-contention <recording path> - records monitor contention on a thread created before profiler initialization
    • * */ public class ExternalLauncher { @@ -178,15 +178,15 @@ private static String[] unpackScenarioArgs(String mode, String[] args) { } /** Runs one native monitor callback lifecycle on a platform thread created before JNI load. */ - private static void runPreExistingMonitorCallback(boolean contention) throws Exception { + private static void runPreExistingMonitorCallback(boolean contention, Path recording) + throws Exception { ExecutorService executor = Executors.newSingleThreadExecutor(task -> { Thread thread = new Thread(task, "preexisting-monitor-callback"); thread.setDaemon(true); return thread; }); - executor.submit(Thread::currentThread).get(5, TimeUnit.SECONDS); + Thread worker = executor.submit(Thread::currentThread).get(5, TimeUnit.SECONDS); - Path recording = Files.createTempFile("preexisting-monitor-callback", ".jfr"); JavaProfiler profiler = null; boolean started = false; try { @@ -194,7 +194,6 @@ private static void runPreExistingMonitorCallback(boolean contention) throws Exc profiler.execute("start,wall=1ms,filter=,wallprecheck=true,jfr,file=" + recording.toAbsolutePath()); started = true; - long before = profiler.getDebugCounters().getOrDefault("task_block_emitted", 0L); Object monitor = new Object(); if (contention) { @@ -222,15 +221,16 @@ private static void runPreExistingMonitorCallback(boolean contention) throws Exc }).get(5, TimeUnit.SECONDS); } - long emitted = profiler.getDebugCounters().getOrDefault("task_block_emitted", 0L) - before; - System.out.println("[preexisting-monitor-events] " + emitted); + profiler.stop(); + started = false; + System.out.println("[preexisting-monitor] " + worker.getId() + " " + + Integer.toUnsignedLong(System.identityHashCode(monitor))); } finally { if (started) { profiler.stop(); } executor.shutdownNow(); executor.awaitTermination(5, TimeUnit.SECONDS); - Files.deleteIfExists(recording); } } @@ -329,9 +329,9 @@ public static void main(String[] args) throws Exception { System.out.println("[agent-compatible] " + profiler.isMonitorWaitEventsDelegated()); } else if (args[0].equals("profiler-preexisting-monitor-wait")) { - runPreExistingMonitorCallback(false); + runPreExistingMonitorCallback(false, Paths.get(args[1])); } else if (args[0].equals("profiler-preexisting-monitor-contention")) { - runPreExistingMonitorCallback(true); + runPreExistingMonitorCallback(true, Paths.get(args[1])); } else if (args[0].equals("profiler")) { JavaProfiler instance = JavaProfiler.getInstance(); if (args.length == 2) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java index 54be8b9fd5..5d68021da9 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java @@ -326,31 +326,48 @@ private void assertJavaDelegationScenario( @Test void preExistingThreadObjectWaitUsesNativeMonitorCallbacks() throws Exception { - assertPreExistingMonitorCallback("profiler-preexisting-monitor-wait"); + assertPreExistingMonitorCallback("profiler-preexisting-monitor-wait", "WAITING"); } @Test void preExistingThreadContentionUsesNativeMonitorCallbacks() throws Exception { - assertPreExistingMonitorCallback("profiler-preexisting-monitor-contention"); + assertPreExistingMonitorCallback("profiler-preexisting-monitor-contention", "CONTENDED"); } /** Verifies that a pre-JNI-load worker emits a TaskBlock through its first monitor callback. */ - private void assertPreExistingMonitorCallback(String target) throws Exception { - AtomicReference resultLine = new AtomicReference<>(); - LaunchResult result = launch(target, Collections.emptyList(), "", line -> { - if (line.startsWith("[preexisting-monitor-events]")) { - resultLine.set(line); - return LineConsumerResult.STOP; + private void assertPreExistingMonitorCallback(String target, String expectedState) throws Exception { + Path recording = Files.createTempFile("preexisting-monitor-callback-", ".jfr"); + try { + AtomicReference resultLine = new AtomicReference<>(); + LaunchResult result = launch(target, Collections.emptyList(), + recording.toAbsolutePath().toString(), line -> { + if (line.startsWith("[preexisting-monitor] ")) { + resultLine.set(line); + return LineConsumerResult.STOP; + } + return LineConsumerResult.CONTINUE; + }, null); + + assertTrue(result.inTime); + assertEquals(0, result.exitCode); + assertNotNull(resultLine.get(), "Pre-existing monitor callback did not report its identity"); + String[] identity = resultLine.get().substring("[preexisting-monitor] ".length()).split(" "); + assertEquals(2, identity.length); + long threadId = Long.parseLong(identity[0]); + long blocker = Long.parseLong(identity[1]); + boolean found = false; + for (JfrEvent event : JfrEvents.load(recording, "datadog.TaskBlock")) { + if (event.getLong("blocker", Long.MIN_VALUE) == blocker) { + assertEquals(Long.valueOf(threadId), event.getThreadJavaId("eventThread")); + assertEquals("preexisting-monitor-callback", event.getThreadName("eventThread")); + assertEquals(expectedState, event.getString("observedBlockingState")); + found = true; + } } - return LineConsumerResult.CONTINUE; - }, null); - - assertTrue(result.inTime); - assertEquals(0, result.exitCode); - assertNotNull(resultLine.get(), "Pre-existing monitor callback did not report a result"); - long emitted = Long.parseLong(resultLine.get().substring( - "[preexisting-monitor-events] ".length())); - assertTrue(emitted > 0, "Pre-existing thread emitted no native monitor TaskBlock event"); + assertTrue(found, "No TaskBlock matched the pre-existing worker's monitor"); + } finally { + Files.deleteIfExists(recording); + } } @Test diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java index 32158cf216..d4cf818270 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java @@ -6,6 +6,7 @@ package com.datadoghq.profiler.wallclock; import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.JfrEvent; import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; @@ -255,6 +256,7 @@ public void staleWaitStateIsRecoveredAfterProfilerRestart() throws Exception { } } + /** Verifies virtual-thread suppression while preserving platform-thread IDs and names. */ @Test public void virtualMonitorCallbacksDoNotEmitCarrierTaskBlocks() throws Exception { Method startVirtualThread; @@ -296,11 +298,24 @@ public void virtualMonitorCallbacksDoNotEmitCarrierTaskBlocks() throws Exception } assertCompleted(contender, failure); + Object platformWaitMonitor = new Object(); + AtomicReference platformFailure = new AtomicReference<>(); + Thread platformWaiter = new Thread(() -> { + try { + synchronized (platformWaitMonitor) { + platformWaitMonitor.wait(100); + } + } catch (Throwable t) { + platformFailure.set(t); + } + }, "taskblock-control-object-wait"); + platformWaiter.start(); + assertCompleted(platformWaiter, platformFailure); + // Positive control: the same contention shape on a platform thread must be produced, // so this test fails when carrier suppression breaks *and* when production breaks. Object platformMonitor = new Object(); CountDownLatch platformAttempting = new CountDownLatch(1); - AtomicReference platformFailure = new AtomicReference<>(); Thread platformContender; synchronized (platformMonitor) { platformContender = new Thread(() -> { @@ -321,10 +336,12 @@ public void virtualMonitorCallbacksDoNotEmitCarrierTaskBlocks() throws Exception stopProfiler(); JfrEvents events = verifyEvents("datadog.TaskBlock"); - assertTrue(TaskBlockAssertions.containsBlocker(events, identityHash(platformMonitor)), - "platform control was not produced"); - assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(waitMonitor))); - assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(contentionMonitor))); + assertRecordedThread(events, platformWaitMonitor, platformWaiter); + assertRecordedThread(events, platformMonitor, platformContender); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(waitMonitor)), + "virtual-thread wait was recorded as a carrier TaskBlock"); + assertFalse(TaskBlockAssertions.containsBlocker(events, identityHash(contentionMonitor)), + "virtual-thread contention was recorded as a carrier TaskBlock"); } @Override @@ -349,6 +366,20 @@ protected void assertTaskBlockStackReference(JfrEvents events) { TaskBlockAssertions.assertNoCorrelationId(events); } + /** Verifies every event for the monitor uses the blocking platform thread's identity. */ + private static void assertRecordedThread(JfrEvents events, Object monitor, Thread thread) { + long blocker = identityHash(monitor); + boolean found = false; + for (JfrEvent event : events) { + if (event.getLong("blocker", Long.MIN_VALUE) == blocker) { + assertEquals(Long.valueOf(thread.getId()), event.getThreadJavaId("eventThread")); + assertEquals(thread.getName(), event.getThreadName("eventThread")); + found = true; + } + } + assertTrue(found, "platform control was not produced for " + thread.getName()); + } + private static void assertCompleted(Thread thread, AtomicReference failure) throws InterruptedException { thread.join(5_000);