fix(ctimer): align _inflight to own cache line to prevent false sharing

CI failure revealed _inflight was protected, preventing InflightGuard access.
Also, having _inflight adjacent to _enabled creates false sharing:
- _enabled is read (ACQUIRE) by every signal handler on every thread
- _inflight is written (modify-release) by every signal handler
Sharing a cache line causes unnecessary cache line bouncing.

Solution: Move _inflight to public and align to 64-byte cache line boundary
(alignas(64)). This separates the read-only _enabled hot path from the
read-write _inflight counter, reducing cross-thread traffic.

Note: The counter is still globally updated, but the separate cache line
means _enabled reads no longer compete with _inflight writes.

Author: r1viollet

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

@@ -28,10 +28,15 @@ 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.
-  static int _inflight;
+  // 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;
   static long _interval;
   static CStack _cstack;
   static int _signal;

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

@@ -50,7 +50,7 @@ int CTimer::_max_timers = 0;
 int *CTimer::_timers = NULL;
 CStack CTimer::_cstack;
 bool CTimer::_enabled = false;
-int CTimer::_inflight = 0;
+alignas(64) int CTimer::_inflight = 0;
 int CTimer::_signal;
 
 int CTimer::registerThread(int tid) {
@@ -250,11 +250,11 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   }
   Counters::increment(CTIMER_SIGNAL_OWN);
 
-  InflightGuard inflight;  // Increment on entry, decrement on all exit paths
+  InflightGuard inflight;
 
   CriticalSection cs;
   if (!cs.entered()) {
-    return;  // inflight guard decrements automatically
+    return;
   }
   int saved_errno = errno;
   if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) {
@@ -268,7 +268,7 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
       && current->inInitWindow()) {
     current->tickInitWindow();
     errno = saved_errno;
-    return;  // inflight guard decrements automatically — this was the bug!
+    return;
   }
   if (current != NULL) {
     current->noteCPUSample(Profiler::instance()->recordingEpoch());
@@ -303,7 +303,7 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   }
   Counters::increment(CTIMER_SIGNAL_OWN);
 
-  InflightGuard inflight;  // Increment on entry, decrement on all exit paths
+  InflightGuard inflight;
 
   // Atomically try to enter critical section - prevents all reentrancy races
   CriticalSection cs;

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

@@ -119,6 +119,11 @@ uint32_t CriticalSection::hash_tid(int tid) {
 }
 
 // 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__
 

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

@@ -107,6 +107,9 @@ void signalHandlerUnwindAfterLongjmp();
  *
  * 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.

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

@@ -1410,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;
@@ -1468,14 +1474,15 @@ Error Profiler::start(Arguments &args, bool reset) {
     // TODO: find a better way to resolve the thread name.
     onThreadStart(nullptr, nullptr, nullptr);
 
-    // enableEngines() (_enabled=true) is intentionally deferred until here,
-    // after _jfr.start() and all engine starts. Previously it was called
-    // before _jfr.start(), creating a TOCTOU race: a SIGPROF delivered
-    // between enableEngines() and _jfr.start() completing would enter
-    // recordSample() on partially-initialized JFR structures.
-    // Paired with drainInflight() in CTimer::stop() which closes the
-    // symmetric race on the stop side.
-    enableEngines();
+    // 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);