DataDog · r1viollet · Jun 23, 2026 · Jun 24, 2026 · Jun 25, 2026 · Jun 25, 2026
@@ -28,6 +28,20 @@ class CTimer : public Engine {
 protected:
   // This is accessed from signal handlers, so must be async-signal-safe
   static bool _enabled;
+
+public:
+  // Count of signal handlers currently executing past the _enabled check.
+  // stop() drains this to zero before tearing down JFR structures, closing
+  // the TOCTOU window between the _enabled check and JFR buffer access.
+  // Public so InflightGuard (in guards.cpp) can access it.
+  // Placed on its own cache line to avoid false sharing with _enabled:
+  // _enabled is read-only on the hot path; _inflight is read-write.
+  alignas(64) static int _inflight;
+
+  // Returns true if any handlers are still in-flight (stuck after timeout)
+  static bool hasInflightHandlers() {
+    return __atomic_load_n(&_inflight, __ATOMIC_ACQUIRE) > 0;
+  }
   static long _interval;
   static CStack _cstack;
   static int _signal;
@@ -57,6 +71,13 @@ class CTimer : public Engine {
     __atomic_store_n(&_enabled, enabled, __ATOMIC_RELEASE);
   }
 
+  // Spin until all signal handlers that passed the _enabled=true check have
+  // returned. Must be called with _enabled already false (after disableEngines()),
+  // before any JFR teardown that handlers could race against.
+  // Returns true if all handlers drained successfully, false if timeout fired.
+  // Caller must NOT proceed with JFR teardown if this returns false.
+  static bool drainInflight();
+
   // Get the signal number used by CTimer (0 if not initialized)
   static int getSignal() { return _signal; }
 };
@@ -92,6 +113,9 @@ class CTimer : public Engine {
   }
 
   static bool supported() { return false; }
+
+  // No-op on non-Linux platforms
+  static bool drainInflight() { return true; }
 };
 
 class CTimerJvmti : public Engine {

@@ -34,6 +34,7 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include <time.h>
+#include <time.h>
 #include <unistd.h>
 
 #ifndef SIGEV_THREAD_ID
@@ -49,6 +50,7 @@ int CTimer::_max_timers = 0;
 int *CTimer::_timers = NULL;
 CStack CTimer::_cstack;
 bool CTimer::_enabled = false;
+alignas(64) int CTimer::_inflight = 0;
 int CTimer::_signal;
 
 int CTimer::registerThread(int tid) {
@@ -176,10 +178,47 @@ Error CTimer::start(Arguments &args) {
   return Error::OK;
 }
 
+// 200 ms: long enough for any legitimate recordSample() call to finish,
+// short enough to avoid a perceptible hang if a handler is somehow stuck.
+static const long DRAIN_TIMEOUT_NS = 200000000L;
+
+bool CTimer::drainInflight() {
+  if (__atomic_load_n(&_inflight, __ATOMIC_ACQUIRE) == 0) {
+    return true; // fast path: nothing in flight
+  }
+
+  struct timespec deadline;
+  clock_gettime(CLOCK_MONOTONIC, &deadline);
+  deadline.tv_nsec += DRAIN_TIMEOUT_NS;
+  if (deadline.tv_nsec >= 1000000000L) {
+    deadline.tv_sec += 1;
+    deadline.tv_nsec -= 1000000000L;
+  }
+
+  while (__atomic_load_n(&_inflight, __ATOMIC_ACQUIRE) > 0) {
+    struct timespec now;
+    clock_gettime(CLOCK_MONOTONIC, &now);
+    if (now.tv_sec > deadline.tv_sec ||
+        (now.tv_sec == deadline.tv_sec && now.tv_nsec >= deadline.tv_nsec)) {
+      int remaining = __atomic_load_n(&_inflight, __ATOMIC_ACQUIRE);
+      Log::error("CTimer::drainInflight: timed out after %ldms waiting for "
+                 "%d in-flight SIGPROF handler(s). Skipping JFR teardown to "
+                 "prevent use-after-free. This indicates a stuck signal handler.",
+                 DRAIN_TIMEOUT_NS / 1000000L, remaining);
+      return false; // timeout: handlers still in-flight
+    }
+    sched_yield();
+  }
+  return true; // success: all handlers drained
+}
+
 void CTimer::stop() {
   for (int i = 0; i < _max_timers; i++) {
     unregisterThread(i);
   }
+  // Note: drainInflight() is called by Profiler::stop() after all engines
+  // have stopped, not here. Profiler needs the return value to decide whether
+  // to proceed with JFR teardown.
 }
 
 Error CTimerJvmti::check(Arguments &args) {
@@ -210,6 +249,8 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   }
   Counters::increment(CTIMER_SIGNAL_OWN);
 
+  InflightGuard inflight;
+
   CriticalSection cs;
   if (!cs.entered()) {
     return;
@@ -261,6 +302,8 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   }
   Counters::increment(CTIMER_SIGNAL_OWN);
 
+  InflightGuard inflight;
+
   // Atomically try to enter critical section - prevents all reentrancy races
   CriticalSection cs;
   if (!cs.entered()) {
@@ -270,8 +313,9 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   int saved_errno = errno;
   // we want to ensure memory order because of the possibility the instance gets
   // cleared
-  if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE))
+  if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) {
     return;
+  }
   int tid = 0;
   ProfiledThread *current = ProfiledThread::currentSignalSafe();
   assert(current == nullptr || !current->isDeepCrashHandler());

@@ -18,6 +18,9 @@
 #include "common.h"
 #include "os.h"
 #include "thread.h"
+#ifdef __linux__
+#include "ctimer.h"
+#endif
 
 // Signal-context tracking — backed by ProfiledThread::_signal_depth; see
 // the comment block in guards.h for the rationale (initial-exec TLS was
@@ -114,3 +117,34 @@ CriticalSection::~CriticalSection() {
 uint32_t CriticalSection::hash_tid(int tid) {
     return static_cast<uint32_t>(tid * KNUTH_MULTIPLICATIVE_CONSTANT);
 }
+
+// InflightGuard implementation — Linux-specific CTimer race mitigation
+// Uses CTimer::_inflight counter which is placed on its own cache line (alignas(64))
+// to avoid false sharing with _enabled. While the counter is still globally updated,
+// the separate cache line means:
+// - _enabled remains read-only on its cache line (fast, no bouncing)
+// - _inflight writes don't invalidate the _enabled cache line
+
+#ifdef __linux__
+
+InflightGuard::InflightGuard() : _active(true) {
+    // Directly access CTimer's static _inflight counter with ACQUIRE semantics
+    // so drainInflight()'s ACQUIRE load will see all writes before this increment.
+    __atomic_fetch_add(&CTimer::_inflight, 1, __ATOMIC_ACQUIRE);
+}
+
+InflightGuard::~InflightGuard() {
+    if (_active) {
+        // RELEASE semantics: all signal handler writes become visible to
+        // drainInflight() before the counter decrements.
+        __atomic_fetch_sub(&CTimer::_inflight, 1, __ATOMIC_RELEASE);
+    }
+}
+
+#else
+
+// No-op implementation for non-Linux platforms where CTimer doesn't exist
+InflightGuard::InflightGuard() : _active(false) {}
+InflightGuard::~InflightGuard() {}
+
+#endif
@@ -102,6 +102,39 @@ class SignalHandlerScope {
 void signalHandlerUnwindAfterLongjmp();
 #define SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP() signalHandlerUnwindAfterLongjmp()
 
+/**
+ * RAII guard for CTimer signal handler in-flight tracking.
+ *
+ * Increments CTimer::_inflight on construction and decrements on destruction,
+ * ensuring the counter is always balanced even if the handler returns early.
+ * CTimer::_inflight is cache-line-aligned (alignas(64)) to avoid false sharing
+ * with _enabled, minimizing cache line bouncing.
+ *
+ * This closes the TOCTOU race between disableEngines() and _jfr.stop():
+ * CTimer::drainInflight() spins until _inflight reaches zero, guaranteeing
+ * that _jfr.stop() only runs once all handlers have fully exited.
+ *
+ * Usage (at the start of CTimer signal handlers):
+ *   InflightGuard inflight;
+ *   // All return paths automatically decrement the counter
+ *
+ * The guard is a no-op on non-Linux platforms where CTimer is unavailable.
+ */
+class InflightGuard {
+public:
+    InflightGuard();
+    ~InflightGuard();
+
+    // Non-copyable, non-movable
+    InflightGuard(const InflightGuard&) = delete;
+    InflightGuard& operator=(const InflightGuard&) = delete;
+    InflightGuard(InflightGuard&&) = delete;
+    InflightGuard& operator=(InflightGuard&&) = delete;
+
+private:
+    bool _active;
+};
+
 /**
  * Race-free critical section using atomic compare-and-swap.
  *

@@ -1381,8 +1381,6 @@ Error Profiler::start(Arguments &args, bool reset) {
     _libs->updateBuildIds();
   }
 
-  enableEngines();
-
   // Refresher must be running before the trap fires: dlopen_hook's
   // signal-context branch only marks dirty and relies on the refresher
   // to call refresh() within REFRESH_INTERVAL_NS (500 ms).
@@ -1396,7 +1394,6 @@ Error Profiler::start(Arguments &args, bool reset) {
   _num_context_attributes = args._context_attributes.size();
   error = _jfr.start(args, reset);
   if (error) {
-    disableEngines();
     switchLibraryTrap(false);
     _libs->stopRefresher();
     return error;
@@ -1413,9 +1410,15 @@ Error Profiler::start(Arguments &args, bool reset) {
     }
   }
   if ((_event_mask & EM_WALL) && _wall_engine != &noop_engine) {
+    // Enable wall clock BEFORE starting its pthread to close a TOCTOU race:
+    // the pthread enters timerLoopCommon(), checks _enabled, and exits if false.
+    // Wall clock is safe to enable early because it doesn't access JFR in signal
+    // handlers (unlike CPU profiling which must wait until JFR is initialized).
+    _wall_engine->enableEvents(true);
     error = _wall_engine->start(args);
     if (error) {
       Log::warn("%s", error.message());
+      _wall_engine->enableEvents(false);
       error = Error::OK; // recoverable
     } else {
       activated |= EM_WALL;
@@ -1471,6 +1474,16 @@ Error Profiler::start(Arguments &args, bool reset) {
     // TODO: find a better way to resolve the thread name.
     onThreadStart(nullptr, nullptr, nullptr);
 
+    // Enable CPU profiling now that JFR is fully initialized. CPU SIGPROF handlers
+    // access JFR buffers, so this must come after _jfr.start() completes to avoid
+    // a TOCTOU race where a signal arrives on partially-initialized JFR structures.
+    // Wall clock was already enabled before _wall_engine->start() to avoid a
+    // different race where its pthread exits before being enabled (wall clock
+    // doesn't access JFR in signal handlers, so early enable is safe).
+    // Paired with drainInflight() in CTimer::stop() which closes the symmetric
+    // race on the stop side.
+    _cpu_engine->enableEvents(true);
+
     _state.store(RUNNING, std::memory_order_release);
     _start_time = time(NULL);
     __atomic_add_fetch(&_epoch, 1, __ATOMIC_RELAXED);
@@ -1558,11 +1571,22 @@ Error Profiler::stop() {
     }
   }
 
+  // Wait for all SIGPROF handlers to drain before tearing down JFR.
+  // If draining times out (handlers are stuck), skip JFR teardown to prevent
+  // use-after-free. This leaks JFR resources (~few MB) but is far safer than
+  // freeing structures that stuck handlers are still accessing.
+  bool drained = CTimer::drainInflight();
+
   // writing these out before stopping the JFR recording allows to report the
   // correct counts in the recording
   _thread_info.reportCounters();
 
-  rotateDictsAndRun([&]{ _jfr.stop(); });
+  if (drained) {
+    rotateDictsAndRun([&]{ _jfr.stop(); });
+  } else {
+    Log::error("Profiler::stop: skipping JFR teardown due to stuck SIGPROF handlers. "
+               "JFR resources (~few MB) will leak. Recording file may be incomplete.");
+  }
 
   // Unpatch libraries AFTER JFR serialization completes
   // Remote symbolication RemoteFrameInfo structs contain pointers to build-ID strings