jbachorik's comments

DataDog · zhengyu123 · Jul 9, 2025 · Jun 30, 2025 · Jun 30, 2025 · Jun 30, 2025
commit e9803d18bb6d35cf98dbe40a04cfe117eb2129d0
@@ -625,9 +625,9 @@ u32 Profiler::recordJVMTISample(u64 counter, int tid, jthread thread, jint event
     if (duration > 0) {
       Counters::increment(UNWINDING_TIME_JVMTI, duration); // increment the JVMTI specific counter
     }
-    if (!deferred) {
-      _jfr.recordEvent(lock_index, tid, call_trace_id, event_type, event);
-    }
+  }
+  if (!deferred) {
+    _jfr.recordEvent(lock_index, tid, call_trace_id, event_type, event);
   }
 
   _locks[lock_index].unlock();

@@ -22,6 +22,7 @@ SafeAccess::SafeFetch32 SafeAccess::_safeFetch32Func = nullptr;
 void SafeAccess::initSafeFetch(CodeCache* libjvm) {
   // Hotspot JVM's safefetch implementation appears better, e.g. it actually returns errorValue,
   // when the address is invalid. let's use it whenever possible.
+  // When the methods are not available, fallback to SafeAccess::load32() implementation.
   _safeFetch32Func = (SafeFetch32)libjvm->findSymbol("SafeFetch32_impl");
   if (_safeFetch32Func == nullptr && !WX_MEMORY) {
     // jdk11 stub implementation other than Macosx/aarch64

@@ -37,7 +37,7 @@ class SafeAccess {
 public:
   static void initSafeFetch(CodeCache* libjvm);
 
-  static inline int safeFetch(int* ptr, int errorValue) {
+  static inline int safeFetch32(int* ptr, int errorValue) {
     assert(_safeFetch32Func != nullptr);
     return _safeFetch32Func(ptr, errorValue);
   }

@@ -143,23 +143,11 @@ namespace ddprof {
     }
 
     OSThreadState osThreadState() {
-      if (ddprof::VMStructs::thread_osthread_offset() >= 0 && ddprof::VMStructs::osthread_state_offset() >= 0) {
-        const char *osthread = *(const char **)at(ddprof::VMStructs::thread_osthread_offset());
-        if (osthread != nullptr) {
-          return static_cast<OSThreadState>(
-              *(int *)(osthread + ddprof::VMStructs::osthread_state_offset()));
-        }
-      }
-      return OSThreadState::UNKNOWN;
-    }
-
-    // Safer version
-    OSThreadState osThreadStateSafe() {
       if (ddprof::VMStructs::thread_osthread_offset() >= 0 && ddprof::VMStructs::osthread_state_offset() >= 0) {
         const char *osthread = *(char **)at(ddprof::VMStructs::thread_osthread_offset());
         if (osthread != nullptr) {
-          // If the location is accessible, the thread must have been terminated
-          int value = SafeAccess::safeFetch((int*)(osthread + ddprof::VMStructs::osthread_state_offset()),
+          // If the location is not accessible, the thread must have been terminated
+          int value = SafeAccess::safeFetch32((int*)(osthread + ddprof::VMStructs::osthread_state_offset()),
                                             static_cast<int>(OSThreadState::TERMINATED));
           // Checking for bad data
           if (value > static_cast<int>(OSThreadState::SYSCALL)) {
@@ -171,26 +159,26 @@ namespace ddprof {
      return OSThreadState::UNKNOWN;
     }
 
-    int osThreadIdSafe() {
+    int osThreadId() {
       if (ddprof::VMStructs::thread_osthread_offset() >= 0 && ddprof::VMStructs::osthread_id_offset() >=0) {
         const char* osthread = *(const char**) at(ddprof::VMStructs::thread_osthread_offset());
         if (osthread == nullptr) {
           return -1;
         } else {
-          return SafeAccess::safeFetch((int*)(osthread + ddprof::VMStructs::osthread_id_offset()), -1);
+          return SafeAccess::safeFetch32((int*)(osthread + ddprof::VMStructs::osthread_id_offset()), -1);
         }
       }
       return -1;
     }
 
-    int stateSafe() {
+    int state() {
       int offset = ddprof::VMStructs::thread_state_offset();
       if (offset >= 0) {
           int* state = (int*)at(offset);
           if (state == nullptr) {
             return 0;
           } else {
-            int value = SafeAccess::safeFetch(state, 0);
+            int value = SafeAccess::safeFetch32(state, 0);
             // Checking for bad data
             if (value > _thread_max_state) {
               value = 0;

@@ -196,8 +196,7 @@ void WallClockJVMTI::timerLoop() {
           if (nThread == nullptr) {
             continue;
           }
-          // Racy, use safer version and check
-          int tid = nThread->osThreadIdSafe();
+          int tid = nThread->osThreadId();
           if (!threadFilter->isValid(tid)) {
             continue;
           }
@@ -212,18 +211,17 @@ void WallClockJVMTI::timerLoop() {
   auto sampleThreads = [&](ThreadEntry& thread_entry, int& num_failures, int& threads_already_exited, int& permission_denied) {
     static jint max_stack_depth = (jint)Profiler::instance()->max_stack_depth();
 
-    // Following code is racy, use safer version to access native structure.
     ExecutionEvent event;
     ddprof::VMThread* vm_thread = thread_entry.native;
-    int raw_thread_state = vm_thread->stateSafe();
+    int raw_thread_state = vm_thread->state();
     bool is_initialized = raw_thread_state >= ddprof::JVMJavaThreadState::_thread_in_native &&
                           raw_thread_state < ddprof::JVMJavaThreadState::_thread_max_state;
     OSThreadState state = OSThreadState::UNKNOWN;
     ExecutionMode mode = ExecutionMode::UNKNOWN;
     if (vm_thread == nullptr || !is_initialized) {
         return false;
     }
-    OSThreadState os_state = vm_thread->osThreadStateSafe();
+    OSThreadState os_state = vm_thread->osThreadState();
     if (state == OSThreadState::TERMINATED) {
       return false;
     } else if (state == OSThreadState::UNKNOWN) {