Fix JFR startup deadlock without disabling built-in JFR events (#11957)

Actually initialize JFR Handlers class early to avoid deadlock

PR #10096 attempted to preload jdk.jfr.events.Handlers via
ClassLoader.loadClass() to prevent an AB-BA deadlock between the JFR
Utils monitor and the Handlers class-initialization lock (JDK-8371889).

However, ClassLoader.loadClass() only loads the class; it does not run
the static initializer and does not acquire the class-initialization
lock. Initialization is still deferred to first active use, inside the
racing window, so the deadlock is not actually prevented.

Use Class.forName(name, true, loader) instead, which forces <clinit> to
run on this thread before the JFR code acquires the Utils monitor. Also
widen the catch to Throwable, since forcing initialization can surface
Errors (ExceptionInInitializerError, NoClassDefFoundError, LinkageError)
that the previous catch (Exception) would not have handled.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Address review: cover renamed holder class and init before all JFR registration

Two follow-ups from PR review:

- The JFR event-holder class was renamed across JDK versions. It is
  jdk.jfr.events.Handlers only on JDK 17-18; on JDK 19-22 it is
  jdk.jfr.events.EventConfigurations (same Utils-based eager init, same
  deadlock); on JDK 23+ the pattern was removed. Targeting only Handlers
  meant the workaround silently no-op'd (ClassNotFoundException) on JDK
  19-22, which are affected. Try both class names.

- registerDeadlockDetectionEvent() also registers a JFR event and runs
  before registerSmapEntryEvent(). Hoist the holder-class initialization
  into startJmx() ahead of both registrations so it is always fully
  initialized before any JFR registration acquires the Utils monitor.

Extracted the logic into initializeJfrEventHolderClass().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Register JDK JFR events before forcing holder init, to avoid null handlers

Forcing the holder class's <clinit> (jdk.jfr.events.Handlers on JDK 17-18,
EventConfigurations on 19-22) before the JDK's built-in events are
registered caches null into its static-final handler fields permanently,
silently disabling the built-in socket/file/exception JFR events.
Empirically verified on JDK 17.0.17 and 21.0.9 (7/7 fields null when the
holder is initialized before FlightRecorder; 0/7 when after).

In the default profiling.start-force-first=false config the profiler
defers FlightRecorder initialization until after startJmx(), so the
previous placement would poison the handlers. Force FlightRecorder init
(which runs JDKEvents.initialize()) before the holder <clinit>, mirroring
the JDK's own JDK-8371889 fix which initializes the holder only after
JDKEvents.initialize(). If FlightRecorder init fails there is nothing to
protect, so the holder init is skipped. Collapsed the handling into a
single catch(Throwable) with a narrow inner ClassNotFoundException for the
Handlers -> EventConfigurations version fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add regression test for JFR handler null-poisoning

Adds JfrEventHolderInitForkedTest, which runs the production
initialization path (Agent.initializeJfrEventHolderClass) in a fresh JVM
and asserts the JDK's built-in JFR event-holder fields are not poisoned to
null. This guards the ordering invariant that FlightRecorder must be
initialized (registering the built-in events) before the holder class's
<clinit> runs; without it the static-final handler fields cache null
permanently and silently disable socket/file/exception JFR events.

Verified: passes on JDK 17 and 21 with the correct ordering, and fails
when the ordering is reversed. Auto-skips on JDKs without the holder class
(JDK 8 and 23+). Behavior underpinning the test was reproduced standalone
on JDK 17.0.17 and 21.0.9 (7/7 fields null when initialized too early,
0/7 when after FlightRecorder).

To make the logic testable, initializeJfrEventHolderClass() takes the
ClassLoader as an argument (the production no-arg overload passes
AGENT_CLASSLOADER). build.gradle adds --add-opens
jdk.jfr/jdk.jfr.events=ALL-UNNAMED so the test can read the holder fields
reflectively.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Route JFR event registration through a single init-first choke point

Both registerDeadlockDetectionEvent() and registerSmapEntryEvent() shared
the same reflective "invoke Factory.registerEvents()" pattern. Extract it
into registerJfrEvents(), which force-initializes the JDK JFR event-holder
class before registering anything. Routing all startup JFR registration
through this one method enforces the ordering invariant structurally, so a
future event registration cannot reintroduce the JDK-8371889 deadlock by
running before the holder is initialized. The explicit pre-init call in
startJmx() is now redundant and removed.

Also unifies exception handling: NoClassDefFoundError/ClassNotFoundException/
UnsupportedClassVersionError -> "not supported" (debug); any other Throwable
-> error. This is slightly more robust for the smap path, which previously
caught only Exception and would have let an Error (e.g. NoClassDefFoundError)
propagate out of startup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Merge branch 'master' into rkennke/fix-jfr-handlers-clinit-deadlock

Remove duplicated comment in initializeJfrEventHolderClass

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Address pre-review: docs, version accuracy, log wording, test caveats

- Correct the Handlers version range to JDK 15-18 (it is absent on 11-14),
  and note that JDK 11 LTS and earlier predate the holder class, so they
  are unaffected.
- Move the explanatory Javadoc onto the initializeJfrEventHolderClass
  implementation (it was on the no-arg wrapper, while @link targets and the
  real logic are on the ClassLoader overload).
- Reference the SCP-1278 deadlock mechanism (smap registration vs. a
  concurrent JMXFetch ByteBuddy transform) and note the event registration
  would trigger FlightRecorder init anyway; we only order it first.
- Capitalize the smap log description so "Smap entry scraping not supported"
  reads correctly.
- Document the regression test's scope: it checks the ordering invariant
  (non-null handlers), not end-to-end event emission, and assumes a fresh
  forked JVM that has not started JFR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Select JFR holder class by version instead of catching CNFE

Per review, replace the exception-driven "try Handlers, catch
ClassNotFoundException, try EventConfigurations" control flow with an
explicit version check. New jfrEventHolderClassName() maps the running JDK
to its holder class (Handlers on 15-18, EventConfigurations on 19-22, null
otherwise) using JavaVirtualMachine.isJavaVersionAtLeast(), and is the
single source of truth shared by the production init path and the
regression test (which now uses it to decide the target class and skip
JDKs without a holder). The remaining try/catch(Throwable) is genuine
error handling (JFR disabled / init failure), not control flow.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Merge branch 'master' into rkennke/fix-jfr-handlers-clinit-deadlock

Merge branch 'master' into rkennke/fix-jfr-handlers-clinit-deadlock

Scope the JFR holder-class workaround to HotSpot

The build pipeline's test_base:[semeru17] job failed: on JDK 17 the
version check selected jdk.jfr.events.Handlers, but Eclipse OpenJ9 / IBM
Semeru ships a different JFR implementation where the holder / Utils
handler mechanism does not behave like HotSpot, so the regression test's
non-null-handler assertion did not hold. (semeru8/semeru11 passed because
those versions have no holder and the test skips.)

JDK-8371889 is a HotSpot JFR bug, so gate jfrEventHolderClassName() on
JavaVirtualMachine.isHotspot(): it now returns null on non-HotSpot VMs,
making the production init a no-op there and the regression test skip.
Verified the test still runs (not skipped) and passes on HotSpot JDK 17.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

Author: rkennke

dd-java-agent/agent-bootstrap/build.gradle

@@ -72,4 +72,9 @@ tasks.withType(Test).configureEach {
     JavaVersion.VERSION_16,
     ['--add-opens', 'java.base/java.net=ALL-UNNAMED'] // for HostNameResolverForkedTest
     )
+  conditionalJvmArgs(
+    it,
+    JavaVersion.VERSION_11,
+    ['--add-opens', 'jdk.jfr/jdk.jfr.events=ALL-UNNAMED'] // for JfrEventHolderInitForkedTest
+    )
 }

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -1,5 +1,6 @@
 package datadog.trace.bootstrap;
 
+import static datadog.environment.JavaVirtualMachine.isHotspot;
 import static datadog.environment.JavaVirtualMachine.isJavaVersionAtLeast;
 import static datadog.environment.JavaVirtualMachine.isOracleJDK8;
 import static datadog.trace.api.Config.isExplicitlyDisabled;
@@ -908,6 +909,8 @@ private static synchronized void startJmx() {
       }
     }
     if (profilingEnabled) {
+      // Both of these register JFR events through registerJfrEvents(), which force-initializes the
+      // JDK's JFR event-holder class first (see initializeJfrEventHolderClass) to avoid a deadlock.
       registerDeadlockDetectionEvent();
       registerSmapEntryEvent();
       if (PROFILER_INIT_AFTER_JMX != null) {
@@ -937,40 +940,143 @@ When getJmxStartDelay() is set to 0 we will attempt to initialize the JMX subsys
 
   private static synchronized void registerDeadlockDetectionEvent() {
     log.debug("Initializing JMX thread deadlock detector");
+    registerJfrEvents(
+        "com.datadog.profiling.controller.openjdk.events.DeadlockEventFactory",
+        "JMX thread deadlock detection");
+  }
+
+  private static void initializeJfrEventHolderClass() {
+    initializeJfrEventHolderClass(AGENT_CLASSLOADER);
+  }
+
+  /**
+   * Force-initializes the JDK's JFR event-holder class early to avoid an ABBA deadlock between
+   * {@code jdk.jfr.internal.Utils}'s monitor and the holder class's initialization lock. See <a
+   * href="https://bugs.openjdk.org/browse/JDK-8371889">JDK-8371889</a> and the SCP-1278 thread
+   * dump.
+   *
+   * <p>The holder's {@code <clinit>} looks up the JDK's built-in event handlers through {@code
+   * jdk.jfr.internal.Utils} (taking its monitor); conversely, JFR event registration initializes
+   * the holder while holding that same monitor. The deadlock forms when one thread holds the {@code
+   * Utils} monitor and wants the holder's class-init lock, while another thread holds the
+   * class-init lock (running {@code <clinit>}) and wants the {@code Utils} monitor. In SCP-1278
+   * this was the profiler's smap-event registration (holding {@code Utils}) against a concurrent
+   * JMXFetch ByteBuddy transform that had triggered the holder {@code <clinit>}. Running the {@code
+   * <clinit>} here, on this thread, before we register our own JFR events, means the holder is
+   * already initialized by then, so the cycle cannot form.
+   *
+   * <p>Ordering matters twice over:
+   *
+   * <ul>
+   *   <li>We force {@code FlightRecorder} initialization first, which registers the JDK's built-in
+   *       events (via {@code JDKEvents.initialize()}). The holder's fields are {@code static final}
+   *       and are populated from {@code Utils} at {@code <clinit>} time; running {@code <clinit>}
+   *       before the events are registered would cache {@code null} into those fields permanently
+   *       and silently disable the built-in socket/file/exception JFR events (verified on JDK
+   *       17.0.17 and 21.0.9). This mirrors the JDK's own fix, which initializes the holder only
+   *       after {@code JDKEvents.initialize()}. The event registration below would trigger the same
+   *       {@code FlightRecorder} initialization anyway; we merely order it ahead of the holder
+   *       {@code <clinit>}. If {@code FlightRecorder} init fails, JFR is unavailable and there is
+   *       nothing to protect against, so we skip the holder init entirely.
+   *   <li>{@link Class#forName(String, boolean, ClassLoader)} with {@code initialize=true} is
+   *       required: {@link ClassLoader#loadClass(String)} would only load the class without running
+   *       {@code <clinit>}, so it would neither take the class-init lock early nor prevent the
+   *       deadlock.
+   * </ul>
+   *
+   * <p>The holder class was renamed across JDK versions, so it is selected by version (see {@link
+   * #jfrEventHolderClassName()}): {@code jdk.jfr.events.Handlers} on JDK 15-18, {@code
+   * jdk.jfr.events.EventConfigurations} on JDK 19-22. Earlier JDKs (including 11 LTS) predate the
+   * holder and JDK 23+ removed the eager-init pattern; on those this method does nothing. On
+   * patched JDKs (the JDK-8371889 fix was backported to 21.0.11) the JDK already initializes the
+   * holder safely during {@code FlightRecorder} startup, so forcing it here is a harmless no-op.
+   *
+   * @param loader class loader used to resolve the JFR classes (package-private for testing; see
+   *     JfrEventHolderInitForkedTest)
+   */
+  static void initializeJfrEventHolderClass(final ClassLoader loader) {
+    final String holderClassName = jfrEventHolderClassName();
+    if (holderClassName == null) {
+      return; // no eager-init holder on this JDK, so there is no deadlock to prevent
+    }
     try {
-      final Class<?> deadlockFactoryClass =
-          AGENT_CLASSLOADER.loadClass(
-              "com.datadog.profiling.controller.openjdk.events.DeadlockEventFactory");
-      final Method registerMethod = deadlockFactoryClass.getMethod("registerEvents");
-      registerMethod.invoke(null);
-    } catch (final NoClassDefFoundError
-        | ClassNotFoundException
-        | UnsupportedClassVersionError ignored) {
-      log.debug("JMX deadlock detection not supported");
-    } catch (final Throwable ex) {
-      log.error("Unable to initialize JMX thread deadlock detector", ex);
+      // Register the JDK's built-in JFR events first, so the holder's <clinit> below sees non-null
+      // handlers instead of caching null.
+      Class.forName("jdk.jfr.FlightRecorder", true, loader)
+          .getMethod("getFlightRecorder")
+          .invoke(null);
+      // Force the holder's <clinit>.
+      Class.forName(holderClassName, true, loader);
+    } catch (final Throwable ignored) {
+      // JFR unavailable/disabled or initialization failed: nothing (left) to do. We catch Throwable
+      // rather than Exception because forcing initialization can surface Errors such as
+      // ExceptionInInitializerError, NoClassDefFoundError or LinkageError.
+    }
+  }
+
+  /**
+   * Returns the fully-qualified name of the JDK's JFR event-holder class for the running JVM, or
+   * {@code null} if this JDK has no such class. The class is selected by JDK version rather than by
+   * probing with {@link ClassNotFoundException} so the mapping is explicit:
+   *
+   * <ul>
+   *   <li>JDK 15-18: {@code jdk.jfr.events.Handlers}
+   *   <li>JDK 19-22: {@code jdk.jfr.events.EventConfigurations}
+   *   <li>otherwise (JDK 14 and earlier predate the holder; JDK 23+ removed the eager-init
+   *       pattern): {@code null}
+   * </ul>
+   *
+   * <p>Also returns {@code null} on non-HotSpot VMs: JDK-8371889 is a HotSpot JFR bug, and other
+   * VMs (e.g. Eclipse OpenJ9 / IBM Semeru) ship a different JFR implementation where this holder /
+   * {@code Utils} mechanism does not apply.
+   */
+  static String jfrEventHolderClassName() {
+    if (!isHotspot()) {
+      return null;
     }
+    if (isJavaVersionAtLeast(15) && !isJavaVersionAtLeast(19)) {
+      return "jdk.jfr.events.Handlers";
+    }
+    if (isJavaVersionAtLeast(19) && !isJavaVersionAtLeast(23)) {
+      return "jdk.jfr.events.EventConfigurations";
+    }
+    return null;
   }
 
   private static synchronized void registerSmapEntryEvent() {
     log.debug("Initializing smap entry scraping");
+    registerJfrEvents(
+        "com.datadog.profiling.controller.openjdk.events.SmapEntryFactory", "Smap entry scraping");
+  }
 
-    // Load JFR Handlers class early, if present (it has been moved and renamed in JDK23+).
-    // This prevents a deadlock. See https://bugs.openjdk.org/browse/JDK-8371889.
-    try {
-      AGENT_CLASSLOADER.loadClass("jdk.jfr.events.Handlers");
-    } catch (Exception e) {
-      // Ignore when the class is not found or anything else goes wrong.
-    }
-
+  /**
+   * Registers a profiling JFR event factory's events, after force-initializing the JDK's JFR
+   * event-holder class (see {@link #initializeJfrEventHolderClass(ClassLoader)}).
+   *
+   * <p><strong>All JFR event registration during agent startup must go through this
+   * method.</strong> Registering a JFR event triggers the holder class's initialization while
+   * holding the {@code jdk.jfr.internal.Utils} monitor; unless the holder has already been fully
+   * initialized, that can deadlock (JDK-8371889). Routing every registration through here
+   * guarantees the holder is initialized first, so future event registrations cannot reintroduce
+   * the deadlock by running before it.
+   *
+   * @param factoryClassName fully-qualified name of the event factory with a static {@code
+   *     registerEvents()} method
+   * @param description human-readable name used in log messages
+   */
+  private static void registerJfrEvents(final String factoryClassName, final String description) {
+    // Enforce the ordering invariant: the holder must be initialized before any JFR event is
+    // registered. Idempotent and cheap once done, so it is safe to call before every registration.
+    initializeJfrEventHolderClass();
     try {
-      final Class<?> smapFactoryClass =
-          AGENT_CLASSLOADER.loadClass(
-              "com.datadog.profiling.controller.openjdk.events.SmapEntryFactory");
-      final Method registerMethod = smapFactoryClass.getMethod("registerEvents");
-      registerMethod.invoke(null);
-    } catch (final Exception ignored) {
-      log.debug("Smap entry scraping not supported");
+      final Class<?> factoryClass = AGENT_CLASSLOADER.loadClass(factoryClassName);
+      factoryClass.getMethod("registerEvents").invoke(null);
+    } catch (final NoClassDefFoundError
+        | ClassNotFoundException
+        | UnsupportedClassVersionError ignored) {
+      log.debug("{} not supported", description);
+    } catch (final Throwable ex) {
+      log.error("Unable to initialize {}", description, ex);
     }
   }
 

dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/JfrEventHolderInitForkedTest.java

@@ -0,0 +1,81 @@
+package datadog.trace.bootstrap;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression test for the JFR startup-deadlock workaround in {@link Agent}.
+ *
+ * <p>{@link Agent#initializeJfrEventHolderClass(ClassLoader)} force-initializes the JDK's JFR
+ * event-holder class ({@code jdk.jfr.events.Handlers} on JDK 15-18, {@code
+ * jdk.jfr.events.EventConfigurations} on JDK 19-22) to avoid an ABBA deadlock. Its static-final
+ * handler fields are populated from {@code jdk.jfr.internal.Utils} at {@code <clinit>} time and
+ * stay {@code null} forever if the class is initialized before the JDK's built-in events are
+ * registered (i.e. before {@code FlightRecorder} initialization). A null-poisoned holder silently
+ * disables the built-in socket/file/exception JFR events.
+ *
+ * <p>This test runs the production initialization path and asserts none of the handler fields were
+ * poisoned. It is a {@code ForkedTest} because JFR/class initialization happens once per JVM, so
+ * the ordering under test is only exercised in a fresh JVM that has not yet touched JFR.
+ *
+ * <p>Scope and limitations:
+ *
+ * <ul>
+ *   <li>It verifies the ordering <em>invariant</em> (non-null handler fields), not end-to-end event
+ *       emission. That keeps it fast and non-flaky; the deadlock itself is timing-dependent and is
+ *       defended structurally (see {@code Agent#registerJfrEvents}) rather than reproduced here.
+ *   <li>It assumes the forked JVM has not initialized JFR before the test runs. If the test JVM is
+ *       ever launched with JFR already started (e.g. {@code -XX:StartFlightRecording} or an
+ *       attached JFR profiler), the holder would already hold valid handlers and the test would
+ *       pass without exercising the ordering. In the standard forked test JVM nothing touches JFR
+ *       first, so the test is meaningful (confirmed: it fails if the FlightRecorder-before-holder
+ *       ordering is reversed).
+ * </ul>
+ *
+ * <p>Requires {@code --add-opens jdk.jfr/jdk.jfr.events=ALL-UNNAMED} (configured in build.gradle)
+ * to read the holder's fields reflectively. Automatically skipped where there is no holder to
+ * initialize: JDK 14 and earlier, JDK 23+ (eager-init pattern removed), and non-HotSpot VMs such as
+ * Eclipse OpenJ9 / IBM Semeru (different JFR implementation).
+ */
+public class JfrEventHolderInitForkedTest {
+
+  @Test
+  public void productionInitOrderingDoesNotPoisonHandlers() throws Exception {
+    final ClassLoader loader = getClass().getClassLoader();
+
+    // The holder class (if any) is selected by JDK version; skip when this JDK has none (JDK 8,
+    // 23+).
+    final String holderName = Agent.jfrEventHolderClassName();
+    assumeTrue(holderName != null, "No JFR event-holder class on this JDK; nothing to test");
+
+    // Exercise the exact production path: FlightRecorder init first, then holder <clinit>.
+    Agent.initializeJfrEventHolderClass(loader);
+
+    // The holder is now initialized; read its static handler fields and check none are null.
+    final Class<?> holder = Class.forName(holderName, false, loader);
+    final List<String> nullFields = new ArrayList<>();
+    int handlerFields = 0;
+    for (final Field field : holder.getDeclaredFields()) {
+      if (!Modifier.isStatic(field.getModifiers()) || field.getType().isPrimitive()) {
+        continue;
+      }
+      handlerFields++;
+      field.setAccessible(true);
+      if (field.get(null) == null) {
+        nullFields.add(field.getName());
+      }
+    }
+
+    assertTrue(handlerFields > 0, "expected " + holderName + " to declare handler fields");
+    assertTrue(
+        nullFields.isEmpty(),
+        "JFR handler fields were poisoned to null (holder initialized before FlightRecorder): "
+            + nullFields);
+  }
+}