From 918ea612390d4ba2437d45cc2ed482db6c4fb820 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Wed, 15 Jul 2026 11:16:23 +0200 Subject: [PATCH 1/5] feat(taskblock): capture stacks synchronously at block exit --- ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | 77 +++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp index b1863f72ad..9850b3796b 100644 --- a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp @@ -38,6 +38,83 @@ class JvmSupportGlobalSetup { }; static JvmSupportGlobalSetup jvm_support_global_setup; +class JvmSupportThreadClassificationTest : 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 jboolean virtual_thread; + inline static int is_virtual_thread_calls; + inline static jobject last_thread; + + JniFunction function_table[FUNCTION_TABLE_SIZE]{}; + JNIEnv jni{}; + _jobject thread_object; + jthread thread = &thread_object; + + static jint JNICALL getVersion(JNIEnv*) { return jni_version; } + + static jboolean JNICALL isVirtualThread(JNIEnv*, jobject candidate) { + is_virtual_thread_calls++; + last_thread = candidate; + return virtual_thread; + } + + void SetUp() override { + jni_version = 0x00150000; + virtual_thread = JNI_FALSE; + is_virtual_thread_calls = 0; + last_thread = nullptr; + function_table[GET_VERSION_INDEX] = + reinterpret_cast(&getVersion); + function_table[IS_VIRTUAL_THREAD_INDEX] = + reinterpret_cast(&isVirtualThread); + jni.functions = + reinterpret_cast(function_table); + } +}; + +TEST_F(JvmSupportThreadClassificationTest, NullInputsFailClosed) { + EXPECT_FALSE(JVMSupport::isPlatformThread(nullptr, thread)); + EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, nullptr)); +} + +TEST_F(JvmSupportThreadClassificationTest, InvalidJniVersionFailsClosed) { + jni_version = 0; + EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); + EXPECT_EQ(0, is_virtual_thread_calls); +} + +TEST_F(JvmSupportThreadClassificationTest, PreJni21ThreadIsPlatform) { + 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, Jni21PlatformThreadIsAccepted) { + EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); + EXPECT_EQ(1, is_virtual_thread_calls); + EXPECT_EQ(thread, last_thread); +} + +TEST_F(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) { + function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; + EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); + EXPECT_EQ(0, is_virtual_thread_calls); +} + // --------------------------------------------------------------------------- // VMTestAccessor — friend of VM, lets tests swap VM::_jvmti/_hotspot for a // mock/forced value so JVM-vendor-dependent code paths can be exercised From 4929ce42e3ce15df94a31ffe1e4d03a32ea4ce6c Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Wed, 19 Aug 2026 15:25:51 +0200 Subject: [PATCH 2/5] feat: squash commit for PR 664 rebase --- .../native/config/ConfigurationPresets.kt | 15 + ddprof-lib/src/main/cpp/javaApi.cpp | 90 ++-- 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/threadFilter.h | 7 + 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/jvmSupport_ut.cpp | 82 --- ddprof-lib/src/test/cpp/park_state_ut.cpp | 73 +++ .../src/test/cpp/taskBlockRecorder_ut.cpp | 97 ++++ ddprof-lib/src/test/cpp/threadFilter_ut.cpp | 5 +- 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 + 23 files changed, 2006 insertions(+), 150 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 06bd64d4dd..8ec470b078 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 d6baa382bf..1fdb60a1a5 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) { @@ -389,44 +405,55 @@ 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) { - current->setParkBlockToken( - tf->enterBlockedRun(slot_id, OSThreadState::CONDVAR_WAIT)); + current->setParkBlockToken(tf->enterBlockedRun( + slot_id, OSThreadState::CONDVAR_WAIT, BlockRunOwner::JAVA)); } } - 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 = ThreadFilter::tokenSlotId(park_block_token); - if (tf->activeSlotForId(current->filterSlotId(), current->tid()) != nullptr && - current->filterSlotId() == slot_id) { - tf->exitBlockedRun(slot_id, ThreadFilter::tokenGeneration(park_block_token)); - } - } + finishTaskBlockAtExit(current, Profiler::instance()->threadFilter(), thread, + 1, park_block_token, start_ticks, context, + static_cast(blocker), + static_cast(unblockingSpanId)); } static bool decodeJavaBlockState(jint state, OSThreadState &decoded) { @@ -454,13 +481,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; @@ -480,9 +508,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 72fb1bc8b0..3c5f67b24c 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 3503673d8f..13b01c7b0b 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1541,6 +1541,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(); @@ -1876,9 +1896,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); @@ -1903,7 +1922,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 @@ -2090,7 +2109,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 759531e5ea..e743ed3e88 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -133,6 +133,7 @@ class alignas(alignof(SpinLock)) Profiler { alignas(DEFAULT_CACHE_LINE_SIZE) u64 _failures[ASGCT_FAILURE_TYPES]; bool _wall_precheck = 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}; @@ -185,6 +186,7 @@ class alignas(alignof(SpinLock)) Profiler { void lockAll(); void unlockAll(); + void setTaskBlockEnabled(bool enabled); bool beginTaskBlockRotation(); void endTaskBlockRotation(); @@ -494,6 +496,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/threadFilter.h b/ddprof-lib/src/main/cpp/threadFilter.h index efba474c55..e029ec7323 100644 --- a/ddprof-lib/src/main/cpp/threadFilter.h +++ b/ddprof-lib/src/main/cpp/threadFilter.h @@ -40,6 +40,9 @@ enum class BlockRunOwner : int { struct BlockRunSnapshot { OSThreadState active_state{OSThreadState::UNKNOWN}; + BlockRunOwner owner{BlockRunOwner::NONE}; + u64 generation{0}; + bool active{false}; bool context_eligible{false}; }; @@ -285,6 +288,10 @@ class ThreadFilter { inline BlockRunSnapshot snapshotBlockRun() const { BlockRunSnapshot snapshot; snapshot.active_state = activeBlockState(); + snapshot.owner = activeBlockOwner(); + snapshot.generation = blockGeneration(); + snapshot.active = snapshot.owner != BlockRunOwner::NONE && + snapshot.active_state != OSThreadState::UNKNOWN; snapshot.context_eligible = activeBlockRemainedOutsideContextWindow(); return snapshot; } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index 96ba77eaa5..1509cd497e 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -56,7 +56,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, @@ -86,10 +87,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 uint8_t _signal_depth; // Nested signal-handler depth (see SignalHandlerScope) @@ -112,8 +120,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), @@ -412,11 +423,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; } @@ -439,16 +463,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 ef1561a8d3..6583b0d129 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,10 +16,13 @@ #include "jvmThread.h" #include "libraries.h" #include "log.h" +#include "mutex.h" #include "os.h" #include "profiler.h" #include "safeAccess.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). @@ -48,8 +52,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; @@ -67,6 +79,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 @@ -385,6 +509,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; @@ -443,15 +572,31 @@ bool VM::initializeRequestStackTrace() { return false; } -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 @@ -482,6 +627,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; @@ -498,11 +645,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(); } @@ -519,6 +668,12 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { callbacks.SampledObjectAlloc = ObjectSampler::SampledObjectAlloc; callbacks.GarbageCollectionFinish = LivenessTracker::GarbageCollectionFinish; 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); @@ -571,7 +726,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 @@ -708,7 +926,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 b9237e8cca..3bfa4e147d 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(); private static native boolean recordTrace0(long rootSpanId, String endpoint, String operation, int sizeLimit); @@ -520,13 +560,13 @@ public Map getDebugCounters() { 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/jvmSupport_ut.cpp b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp index 9850b3796b..c8801376bd 100644 --- a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp @@ -38,88 +38,6 @@ class JvmSupportGlobalSetup { }; static JvmSupportGlobalSetup jvm_support_global_setup; -class JvmSupportThreadClassificationTest : 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 jboolean virtual_thread; - inline static int is_virtual_thread_calls; - inline static jobject last_thread; - - JniFunction function_table[FUNCTION_TABLE_SIZE]{}; - JNIEnv jni{}; - _jobject thread_object; - jthread thread = &thread_object; - - static jint JNICALL getVersion(JNIEnv*) { return jni_version; } - - static jboolean JNICALL isVirtualThread(JNIEnv*, jobject candidate) { - is_virtual_thread_calls++; - last_thread = candidate; - return virtual_thread; - } - - void SetUp() override { - jni_version = 0x00150000; - virtual_thread = JNI_FALSE; - is_virtual_thread_calls = 0; - last_thread = nullptr; - function_table[GET_VERSION_INDEX] = - reinterpret_cast(&getVersion); - function_table[IS_VIRTUAL_THREAD_INDEX] = - reinterpret_cast(&isVirtualThread); - jni.functions = - reinterpret_cast(function_table); - } -}; - -TEST_F(JvmSupportThreadClassificationTest, NullInputsFailClosed) { - EXPECT_FALSE(JVMSupport::isPlatformThread(nullptr, thread)); - EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, nullptr)); -} - -TEST_F(JvmSupportThreadClassificationTest, InvalidJniVersionFailsClosed) { - jni_version = 0; - EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); - EXPECT_EQ(0, is_virtual_thread_calls); -} - -TEST_F(JvmSupportThreadClassificationTest, PreJni21ThreadIsPlatform) { - 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, Jni21PlatformThreadIsAccepted) { - EXPECT_TRUE(JVMSupport::isPlatformThread(&jni, thread)); - EXPECT_EQ(1, is_virtual_thread_calls); - EXPECT_EQ(thread, last_thread); -} - -TEST_F(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) { - function_table[IS_VIRTUAL_THREAD_INDEX] = nullptr; - EXPECT_FALSE(JVMSupport::isPlatformThread(&jni, thread)); - EXPECT_EQ(0, is_virtual_thread_calls); -} - -// --------------------------------------------------------------------------- -// VMTestAccessor — friend of VM, lets tests swap VM::_jvmti/_hotspot for a -// mock/forced value so JVM-vendor-dependent code paths can be exercised -// deterministically without a live JVM. -// --------------------------------------------------------------------------- class VMTestAccessor { public: static jvmtiEnv* getJvmti() { return VM::_jvmti; } diff --git a/ddprof-lib/src/test/cpp/park_state_ut.cpp b/ddprof-lib/src/test/cpp/park_state_ut.cpp index 5a236994e0..a7f558fc45 100644 --- a/ddprof-lib/src/test/cpp/park_state_ut.cpp +++ b/ddprof-lib/src/test/cpp/park_state_ut.cpp @@ -137,6 +137,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) { ThreadFilter::Slot slot; diff --git a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp index 652f4e6bb6..d16910dc68 100644 --- a/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp +++ b/ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp @@ -209,6 +209,103 @@ TEST_F(TaskBlockRecorderTest, RotationRejectsEndWithoutStrandingLifecycle) { EXPECT_EQ(1, Counters::getCounter(TASK_BLOCK_DROPPED_ROTATION)); } +TEST_F(TaskBlockRecorderTest, RotationRejectsParkExitWithoutBlockingOrStranding) { + constexpr int tid = 12346; + ThreadFilter filter; + 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 = filter.enterBlockedRun( + 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, nullptr, 1, exit_token, start_ticks, + exit_context, 0, 0); + }); + + EXPECT_EQ(std::future_status::ready, + result.wait_for(std::chrono::seconds(1))); + ThreadFilter::Slot* slot = filter.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; + 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 = filter.enterBlockedRun( + 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, nullptr, 0, exit_token, start_ticks, + exit_context, blocker, 0); + }); + + EXPECT_EQ(std::future_status::ready, + result.wait_for(std::chrono::seconds(1))); + ThreadFilter::Slot* slot = filter.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/threadFilter_ut.cpp b/ddprof-lib/src/test/cpp/threadFilter_ut.cpp index 057ac0086f..b0ab362e5a 100644 --- a/ddprof-lib/src/test/cpp/threadFilter_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadFilter_ut.cpp @@ -723,11 +723,14 @@ TEST_F(ThreadFilterTest, SnapshotCapturesOwnedLifecycle) { ASSERT_NE(0ULL, token); BlockRunSnapshot snapshot = slot->snapshotBlockRun(); + EXPECT_TRUE(snapshot.active); EXPECT_EQ(OSThreadState::SLEEPING, snapshot.active_state); + EXPECT_EQ(BlockRunOwner::JAVA, snapshot.owner); + EXPECT_EQ(ThreadFilter::tokenGeneration(token), snapshot.generation); ASSERT_TRUE(filter->snapshotAndExitBlockedRun( slot_id, ThreadFilter::tokenGeneration(token), &snapshot)); - EXPECT_EQ(OSThreadState::UNKNOWN, slot->snapshotBlockRun().active_state); + EXPECT_FALSE(slot->snapshotBlockRun().active); } TEST_F(ThreadFilterTest, OwnedBlockSuppressesOnlyAfterSuccessfulWallSample) { 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 2dbb429668..5268c050bd 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -9,7 +9,14 @@ import java.lang.management.ManagementFactory; import java.lang.management.ThreadMXBean; import java.lang.reflect.Method; +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; /** @@ -30,6 +37,14 @@ * CPU concurrently on the main thread, on a plain {@code new Thread(Runnable)} and on a * two-level {@link Thread} subclass, and stops the profiler again. The resulting recording * holds samples rooted at each of the three thread entry points; see {@code EntryFrameTest} + *

  • 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 { @@ -119,6 +134,63 @@ private static void entryFrameBurn(long millis) { entryFrameSink = acc; } + /** 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 { @@ -139,6 +211,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 058bd52944..c74e26fa29 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -14,6 +14,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 { @@ -31,6 +32,15 @@ public void taskBlockApiIsPublicButInternalHooksRemainPackageScoped() throws Exc .getModifiers())); } + @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 71ecd07288873ff1f02eee2638faf7beb50223ba Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 20 Aug 2026 14:25:10 +0200 Subject: [PATCH 3/5] 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 1fdb60a1a5..eeb76176fd 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -406,8 +406,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(); @@ -434,9 +436,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(); @@ -481,10 +483,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(); @@ -508,9 +510,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 13b01c7b0b..b93283f0c3 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1555,10 +1555,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 2a82c67dce..ecfd37f29a 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.cpp @@ -42,7 +42,8 @@ bool finishTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter, 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; @@ -70,6 +71,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 172b0f43c1..cec8e51d2a 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -24,11 +24,14 @@ 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, 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 6583b0d129..4fd06e2271 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -86,8 +86,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() || @@ -102,8 +106,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; @@ -123,8 +126,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; } } @@ -148,13 +150,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) || @@ -162,32 +166,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 3bfa4e147d..a0f99ff644 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); } /** @@ -560,13 +582,13 @@ public Map getDebugCounters() { 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 5268c050bd..d97dc74033 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -43,6 +43,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
    • * @@ -250,6 +251,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 e0b664cc8bc985bd7075a32f485a545b7efa66c1 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 26 Aug 2026 10:37:51 +0200 Subject: [PATCH 4/5] 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 eeb76176fd..faba00a4f4 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -463,6 +463,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 a0f99ff644..a603d49770 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 507cc5ec95..7ba08632d4 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 @@ -28,6 +28,7 @@ */ public class PrecheckTest extends AbstractProfilerTest { private static final int OSTHREAD_STATE_SLEEPING = 7; + 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; @@ -63,6 +64,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 968862100c975ece55a145a199ec253839e37bd1 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 1 Sep 2026 17:56:28 +0200 Subject: [PATCH 5/5] 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 | 83 +++++++-- ddprof-lib/src/main/cpp/jvmSupport.cpp | 7 +- ddprof-lib/src/main/cpp/profiler.cpp | 16 +- ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | 145 +++++++++++++++ ddprof-lib/src/main/cpp/taskBlockRecorder.h | 18 ++ ddprof-lib/src/main/cpp/threadFilter.h | 7 +- ddprof-lib/src/main/cpp/threadLocalData.cpp | 2 +- ddprof-lib/src/main/cpp/threadLocalData.h | 171 +++++++++-------- 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/threadFilter_ut.cpp | 4 +- ddprof-lib/src/test/cpp/vmEntry_ut.cpp | 10 +- .../WallClockPrecheckBenchmarkHooks.java | 2 +- .../profiler/JavaProfilerApiSurfaceTest.java | 3 +- .../profiler/ProfilerOwnedBlockHooks.java | 7 +- 18 files changed, 436 insertions(+), 283 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 8ec470b078..09e00d21a9 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 8420c03caf..93d232cd21 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -92,6 +92,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 faba00a4f4..b8761d9a3d 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"); @@ -416,15 +419,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) { current->setParkBlockToken(tf->enterBlockedRun( @@ -487,8 +493,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; @@ -497,25 +502,64 @@ 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; - return static_cast(tf->enterBlockedRun(slot_id, decoded)); + u64 token = tf->enterBlockedRun(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 ThreadFilter + // 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 = ThreadFilter::tokenSlotId(stale_token); + ThreadFilter::Slot *stale_slot = current->filterSlotId() == stale_slot_id + ? tf->activeSlotForId(stale_slot_id, current->tid()) + : nullptr; + if (stale_slot != nullptr) { + BlockRunSnapshot snapshot = stale_slot->snapshotBlockRun(); + current_owner = snapshot.isActive() && + snapshot.owner == BlockRunOwner::JAVA && + snapshot.generation == ThreadFilter::tokenGeneration(stale_token); + } + } + if (current_owner) { + tf->exitBlockedRun(slot_id, ThreadFilter::tokenGeneration(token)); + return 0; + } + current->clearTaskBlock(); + if (!current->taskBlockEnter(token, TSC::ticks(), context)) { + tf->exitBlockedRun(slot_id, ThreadFilter::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; @@ -531,6 +575,19 @@ 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, /*thread=*/nullptr, 1, block_token, + start_ticks, context, static_cast(blocker), + static_cast(unblockingSpanId)); + return; + } + if (tf->registryActive()) { tf->exitBlockedRun(slot_id, ThreadFilter::tokenGeneration(block_token)); } diff --git a/ddprof-lib/src/main/cpp/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 3c5f67b24c..9291f57895 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 b93283f0c3..df2223142e 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1545,9 +1545,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); @@ -1558,10 +1556,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) { @@ -2111,10 +2109,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 ecfd37f29a..af69957842 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; @@ -74,3 +79,143 @@ 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 = 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.isActive() && + 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) { + 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(), 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 cec8e51d2a..3feb46f6bb 100644 --- a/ddprof-lib/src/main/cpp/taskBlockRecorder.h +++ b/ddprof-lib/src/main/cpp/taskBlockRecorder.h @@ -33,6 +33,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/threadFilter.h b/ddprof-lib/src/main/cpp/threadFilter.h index e029ec7323..22603e9980 100644 --- a/ddprof-lib/src/main/cpp/threadFilter.h +++ b/ddprof-lib/src/main/cpp/threadFilter.h @@ -42,8 +42,11 @@ struct BlockRunSnapshot { OSThreadState active_state{OSThreadState::UNKNOWN}; BlockRunOwner owner{BlockRunOwner::NONE}; u64 generation{0}; - bool active{false}; bool context_eligible{false}; + + inline bool isActive() const { + return owner != BlockRunOwner::NONE && active_state != OSThreadState::UNKNOWN; + } }; class ThreadFilter { @@ -290,8 +293,6 @@ class ThreadFilter { snapshot.active_state = activeBlockState(); snapshot.owner = activeBlockOwner(); snapshot.generation = blockGeneration(); - snapshot.active = snapshot.owner != BlockRunOwner::NONE && - snapshot.active_state != OSThreadState::UNKNOWN; snapshot.context_eligible = activeBlockRemainedOutsideContextWindow(); return snapshot; } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 07c63bf74c..cabe9f6cf7 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -158,7 +158,7 @@ void ProfiledThread::unclaimAndReset() { _wall_epoch = 0; _call_trace_id = 0; _recording_epoch = 0; - _park_block_token = 0; + _park_slot.block_token = 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 1509cd497e..abcbefe730 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -58,6 +58,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, @@ -87,17 +88,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 uint8_t _signal_depth; // Nested signal-handler depth (see SignalHandlerScope) @@ -120,10 +124,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), @@ -220,7 +221,7 @@ 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, _filter_slot_id, _init_window, + _park_slot.block_token, _filter_slot_id, _init_window, __atomic_load_n(&_signal_depth, __ATOMIC_RELAXED), _in_critical_section, _otel_ctx_initialized, _otel_local_root_span_id }; @@ -424,17 +425,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 @@ -442,38 +434,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 @@ -488,52 +486,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 4fd06e2271..0f9bfcbf29 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -52,14 +52,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; @@ -79,132 +79,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 @@ -524,7 +398,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; } @@ -588,17 +462,21 @@ bool VM::initializeRequestStackTrace() { 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; } @@ -682,7 +560,7 @@ ProfilerBridgeInitResult VM::initProfilerBridge(JavaVM *vm, bool attach, callbacks.SampledObjectAlloc = ObjectSampler::SampledObjectAlloc; callbacks.GarbageCollectionFinish = LivenessTracker::GarbageCollectionFinish; 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; @@ -740,17 +618,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 @@ -760,7 +639,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; @@ -770,7 +649,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; @@ -778,8 +657,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 a603d49770..8cf8008a1e 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); } /** @@ -586,9 +593,10 @@ public Map getDebugCounters() { 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/threadFilter_ut.cpp b/ddprof-lib/src/test/cpp/threadFilter_ut.cpp index b0ab362e5a..61ace9899a 100644 --- a/ddprof-lib/src/test/cpp/threadFilter_ut.cpp +++ b/ddprof-lib/src/test/cpp/threadFilter_ut.cpp @@ -723,14 +723,14 @@ TEST_F(ThreadFilterTest, SnapshotCapturesOwnedLifecycle) { ASSERT_NE(0ULL, token); BlockRunSnapshot snapshot = slot->snapshotBlockRun(); - EXPECT_TRUE(snapshot.active); + EXPECT_TRUE(snapshot.isActive()); EXPECT_EQ(OSThreadState::SLEEPING, snapshot.active_state); EXPECT_EQ(BlockRunOwner::JAVA, snapshot.owner); EXPECT_EQ(ThreadFilter::tokenGeneration(token), snapshot.generation); ASSERT_TRUE(filter->snapshotAndExitBlockedRun( slot_id, ThreadFilter::tokenGeneration(token), &snapshot)); - EXPECT_FALSE(slot->snapshotBlockRun().active); + EXPECT_FALSE(slot->snapshotBlockRun().isActive()); } TEST_F(ThreadFilterTest, OwnedBlockSuppressesOnlyAfterSuccessfulWallSample) { 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 c74e26fa29..2c502e2759 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -24,7 +24,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) {