fix(profiler): name native threads periodically (PROF-15139)

ddprof-lib/src/main/cpp/libraries.cpp

@@ -7,6 +7,7 @@
 #include "log.h"
 #include "mallocTracer.h"
 #include "os.h"
+#include "profiler.h"
 #include "symbols.h"
 #include "symbols_linux.h"
 #include "vmEntry.h"
@@ -18,6 +19,13 @@
 // in typical sampling, and the refresher only wakes once per tick (cheap).
 static constexpr u64 REFRESH_INTERVAL_NS = 500ULL * 1'000'000ULL;
 
+// Cadence for native (non-Java) thread-name resolution piggy-backed on the
+// refresher thread (PROF-15139).  Each pass enumerates /proc/self/task and
+// reads comm for unknown tids, so it is decimated relative to the 500 ms
+// library-refresh tick to bound that cost on high-thread-count processes.
+// 2 s is well under the lifetime of long-lived JIT/GC threads we want to name.
+static constexpr u64 NATIVE_THREAD_NAME_INTERVAL_NS = 2ULL * 1000ULL * 1'000'000ULL;
+
 void Libraries::mangle(const char *name, char *buf, size_t size) {
   char *buf_end = buf + size;
   strcpy(buf, "_ZN");
@@ -90,6 +98,11 @@ void *Libraries::refresherLoop(void *arg) {
   // Publish our TID so sampler thread-list enumerations can skip us.
   self->_refresher_tid.store(OS::threadId(), std::memory_order_release);
 
+  // Timestamp of the last native-thread-name pass; 0 makes the first eligible
+  // tick run it.  Tracked with a monotonic clock so it is robust to early
+  // wakeups from stopRefresher()'s SIGIO.
+  u64 last_native_name_ns = 0;
+
   while (self->_refresher_running.load(std::memory_order_acquire)) {
     // Absolute-deadline sleep that resumes across EINTR (SIGCHLD, debugger
     // SIGSTOP/SIGCONT, etc.) and wakes early when stopRefresher() flips
@@ -101,6 +114,20 @@ void *Libraries::refresherLoop(void *arg) {
     if (self->_dirty.load(std::memory_order_acquire)) {
       self->refresh();
     }
+    // Name native (non-Java) threads while they are still alive. JIT/GC and
+    // other non-Java threads get no JVMTI ThreadStart, so they are otherwise
+    // named only at dump time; transient ones that exit before the dump fall
+    // back to "[tid=N]" (PROF-15139).  Gated on isRunning() so we do no work
+    // before the profiler reaches RUNNING (startRefresher precedes that), and
+    // decimated to NATIVE_THREAD_NAME_INTERVAL_NS to bound the /proc scan cost.
+    u64 now = OS::nanotime();
+    if (Profiler::instance()->isRunning() &&
+        now - last_native_name_ns >= NATIVE_THREAD_NAME_INTERVAL_NS) {
+      last_native_name_ns = now;
+      // Defer threads still showing the inherited process name; the dump-time
+      // pass (which does not defer) records any that never set a real name.
+      Profiler::instance()->updateNativeThreadNames(true);
+    }
   }
   return nullptr;
 }

ddprof-lib/src/main/cpp/os_linux.cpp

@@ -238,7 +238,18 @@ bool OS::threadName(int thread_id, char* name_buf, size_t name_len) {
     close(fd);
 
     if (r > 0) {
-        name_buf[r - 1] = 0;
+        // /proc comm is newline-terminated; strip the trailing newline.
+        // Otherwise NUL-terminate after the last byte when there is room,
+        // only truncating the final byte if the buffer is completely full.
+        // This guarantees NUL-termination without silently dropping a real
+        // last character.
+        if (name_buf[r - 1] == '\n') {
+            name_buf[r - 1] = 0;
+        } else if ((size_t)r < name_len) {
+            name_buf[r] = 0;
+        } else {
+            name_buf[name_len - 1] = 0;
+        }
         return true;
     }
     return false;

ddprof-lib/src/main/cpp/profiler.cpp

@@ -1052,23 +1052,40 @@ void Profiler::updateJavaThreadNames() {
   jvmti->Deallocate((unsigned char *)thread_objects);
 }
 
-void Profiler::updateNativeThreadNames() {
-    ThreadList *thread_list = OS::listThreads();
-    constexpr size_t buffer_size = 64;
-    char name_buf[buffer_size];  // Stack-allocated buffer
-
-    while (thread_list->hasNext()) { 
-        int tid = thread_list->next(); 
-        _thread_info.updateThreadName(
-                tid, [&](int tid) -> std::string {
-                    if (OS::threadName(tid, name_buf, buffer_size)) {
-                        return std::string(name_buf, buffer_size);
-                    }
-                    return std::string();
-                });
-    }
-
-    delete thread_list;
+void Profiler::updateNativeThreadNames(bool defer_initializing) {
+  ThreadList *thread_list = OS::listThreads();
+  constexpr size_t buffer_size = 64;
+  char name_buf[buffer_size];  // Stack-allocated buffer
+
+  // A freshly cloned thread inherits the creating thread's comm until it sets
+  // its own name; for the threads we want here that creator is typically the
+  // main thread, so the inherited name is the process name. When deferring, we
+  // skip recording it and let a later pass capture the final name.
+  char proc_name[buffer_size];
+  bool have_proc_name =
+      defer_initializing && OS::threadName(OS::processId(), proc_name, buffer_size);
+
+  while (thread_list->hasNext()) {
+    int tid = thread_list->next();
+    _thread_info.updateThreadName(
+        tid, [&](int tid) -> std::string {
+          if (OS::threadName(tid, name_buf, buffer_size)) {
+            // Skip a thread still showing the inherited process name: it is
+            // probably mid-initialization. Recording it would latch a
+            // provisional name (updateThreadName is first-writer-wins).
+            if (have_proc_name && strcmp(name_buf, proc_name) == 0) {
+              return std::string();
+            }
+            // name_buf is NUL-terminated by OS::threadName; let
+            // std::string find the length rather than storing the
+            // full 64-byte buffer (NUL + trailing garbage).
+            return std::string(name_buf);
+          }
+          return std::string();
+        });
+  }
+
+  delete thread_list;
 }
 
 Engine *Profiler::selectCpuEngine(Arguments &args) {

ddprof-lib/src/main/cpp/profiler.h

@@ -138,7 +138,6 @@ class alignas(alignof(SpinLock)) Profiler {
   void updateThreadName(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread,
                         bool self = false);
   void updateJavaThreadNames();
-  void updateNativeThreadNames();
   void mangle(const char *name, char *buf, size_t size);
 
   Engine *selectCpuEngine(Arguments &args);
@@ -206,6 +205,20 @@ class alignas(alignof(SpinLock)) Profiler {
     return _instance;
   }
 
+  // Resolve names of native (non-Java) threads from /proc. Idempotent and
+  // allocation-light (no-op for already-named tids), so it is safe to call
+  // periodically from the Libraries refresher thread to capture transient
+  // compiler/GC threads before they exit. Must NOT be called from a signal
+  // handler: thread enumeration uses opendir/readdir/malloc.
+  //
+  // When defer_initializing is true (periodic refresher), a thread whose comm
+  // still equals the process's own (inherited) name is skipped: it is most
+  // likely still initializing and has not yet set its final pthread name.
+  // Recording it now would latch that provisional name permanently
+  // (ThreadInfo::updateThreadName is first-writer-wins). A later scan, or the
+  // dump-time pass (which passes false), records the final name instead.
+  void updateNativeThreadNames(bool defer_initializing = false);
+
 
   inline void incFailure(int type) {
     if (type < ASGCT_FAILURE_TYPES) {
@@ -410,6 +423,15 @@ class alignas(alignof(SpinLock)) Profiler {
   // Profiler::unregisterThread correctly without needing live engine instances.
   static int  lastUnregisteredTidForTest();
   static void resetUnregisterObservableForTest();
+
+  // Reads back the name recorded for a tid in _thread_info, or an empty string
+  // if none was recorded. Lets integration tests observe the result of
+  // updateNativeThreadNames() (notably the defer_initializing skip) without
+  // exposing the private _thread_info. Compiled only into gtest binaries.
+  std::string threadNameForTest(int tid) {
+    std::pair<std::shared_ptr<std::string>, u64> info = _thread_info.get(tid);
+    return info.first != nullptr ? *info.first : std::string();
+  }
 #endif
 
 

ddprof-lib/src/main/cpp/threadInfo.cpp

@@ -67,16 +67,27 @@ int ThreadInfo::size() {
 }
 
 void ThreadInfo::updateThreadName(
-        int tid, std::function<std::string(int)> resolver) {
+    int tid, std::function<std::string(int)> resolver) {
+  // Fast path: bail out if the name is already known, holding the lock only
+  // for the lookup.
+  {
     MutexLocker ml(_ti_lock);
-    auto it = _thread_names.find(tid);
-    if (it == _thread_names.end()) {
-        // Thread ID not found, insert new entry
-        std::string name = resolver(tid);
-        if (!name.empty()) {
-            _thread_names.emplace(tid, std::move(name));
-        }
+    if (_thread_names.find(tid) != _thread_names.end()) {
+      return;
     }
+  }
+  // Resolve OUTSIDE the lock: the resolver may perform blocking I/O (e.g.
+  // reading /proc/self/task/<tid>/comm). Holding _ti_lock across that would
+  // stall every concurrent set/get/clearAll caller for the duration of the
+  // syscall, once per unknown tid.
+  std::string name = resolver(tid);
+  if (name.empty()) {
+    return;
+  }
+  // emplace is a no-op if a concurrent caller inserted this tid in the
+  // meantime, so the brief unlocked window is harmless.
+  MutexLocker ml(_ti_lock);
+  _thread_names.emplace(tid, std::move(name));
 }
 
 void ThreadInfo::reportCounters() {

ddprof-lib/src/test/cpp/nativeThreadNames_ut.cpp

@@ -0,0 +1,141 @@
+/*
+ * 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.
+ */
+
+// Integration tests for Profiler::updateNativeThreadNames(bool), PROF-15139.
+//
+// The periodic refresher calls updateNativeThreadNames(true), which must skip
+// a thread whose /proc comm still equals the process's own (inherited) name:
+// such a thread is likely mid-initialization and recording it would latch a
+// provisional name permanently (ThreadInfo is first-writer-wins). The dump-time
+// pass calls updateNativeThreadNames(false), which records the name regardless.
+//
+// These tests spawn real parked threads and observe the recorded names via the
+// UNIT_TEST-only Profiler::threadNameForTest hook, so they exercise the actual
+// /proc resolution and defer predicate rather than re-stating the ThreadInfo
+// contract (covered by threadInfo_ut.cpp). Linux-only: the deferral depends on
+// /proc comm and pthread name inheritance.
+
+#ifdef __linux__
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
+#include <gtest/gtest.h>
+#include <pthread.h>
+#include <time.h>
+#include <atomic>
+#include <string>
+#include "../../main/cpp/profiler.h"
+#include "../../main/cpp/os.h"
+
+namespace {
+
+// A thread that optionally sets its own pthread name, publishes its tid, then
+// parks until released. Parking keeps it visible in OS::listThreads() across
+// the updateNativeThreadNames() scans under test.
+struct ParkedThread {
+    std::string set_name;              // empty => leave the inherited comm
+    std::atomic<int> tid{-1};
+    std::atomic<bool> ready{false};
+    std::atomic<bool> stop{false};
+    pthread_t handle{};
+
+    static void* run(void* arg) {
+        ParkedThread* self = static_cast<ParkedThread*>(arg);
+        if (!self->set_name.empty()) {
+            // Linux caps the pthread name at 16 bytes incl. NUL; callers keep
+            // set_name short enough to survive without truncation.
+            pthread_setname_np(pthread_self(), self->set_name.c_str());
+        }
+        self->tid.store(OS::threadId(), std::memory_order_release);
+        self->ready.store(true, std::memory_order_release);
+        while (!self->stop.load(std::memory_order_acquire)) {
+            struct timespec ts{0, 1000000};  // 1ms
+            nanosleep(&ts, nullptr);
+        }
+        return nullptr;
+    }
+
+    void start() {
+        ASSERT_EQ(0, pthread_create(&handle, nullptr, &ParkedThread::run, this));
+        // Bounded spin: the thread publishes ready almost immediately.
+        for (int i = 0; i < 5000 && !ready.load(std::memory_order_acquire); i++) {
+            struct timespec ts{0, 1000000};  // 1ms
+            nanosleep(&ts, nullptr);
+        }
+        ASSERT_TRUE(ready.load(std::memory_order_acquire));
+        ASSERT_GT(tid.load(std::memory_order_acquire), 0);
+    }
+
+    void join() {
+        stop.store(true, std::memory_order_release);
+        pthread_join(handle, nullptr);
+    }
+};
+
+std::string processComm() {
+    char buf[64];
+    EXPECT_TRUE(OS::threadName(OS::processId(), buf, sizeof(buf)));
+    return std::string(buf);
+}
+
+}  // namespace
+
+// A thread that has set its own (non-inherited) name is recorded by the
+// deferring periodic scan: it is not mid-initialization.
+TEST(NativeThreadNamesTest, deferringScanRecordsRealName) {
+    const std::string real_name = "ut-real-name";  // 12 chars, fits the 16B cap
+    ASSERT_NE(real_name, processComm());
+
+    ParkedThread t;
+    t.set_name = real_name;
+    t.start();
+    int tid = t.tid.load();
+
+    Profiler::instance()->updateNativeThreadNames(/*defer_initializing=*/true);
+
+    EXPECT_EQ(real_name, Profiler::instance()->threadNameForTest(tid));
+
+    t.join();
+}
+
+// A thread still showing the inherited process name is skipped by the deferring
+// scan (so the provisional name is not latched), but recorded by the dump-time
+// scan, which does not defer.
+TEST(NativeThreadNamesTest, deferringScanSkipsInheritedNameDumpScanRecordsIt) {
+    const std::string proc_comm = processComm();
+
+    ParkedThread t;  // no set_name => inherits the parent's (process) comm
+    t.start();
+    int tid = t.tid.load();
+    // Sanity: the child really is showing the inherited process name.
+    char buf[64];
+    ASSERT_TRUE(OS::threadName(tid, buf, sizeof(buf)));
+    ASSERT_EQ(proc_comm, std::string(buf));
+
+    // Deferring scan: must skip it, leaving the tid unrecorded.
+    Profiler::instance()->updateNativeThreadNames(/*defer_initializing=*/true);
+    EXPECT_EQ("", Profiler::instance()->threadNameForTest(tid));
+
+    // Dump-time scan: records the name even though it equals the process comm.
+    Profiler::instance()->updateNativeThreadNames(/*defer_initializing=*/false);
+    EXPECT_EQ(proc_comm, Profiler::instance()->threadNameForTest(tid));
+
+    t.join();
+}
+
+#endif  // __linux__