Fix JFR startup deadlock without disabling built-in JFR events

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);
+  }
+}