Fix DDTraceId/DD64bTraceId class-initialization deadlock

dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/AbstractDatadogSparkListener.java

@@ -444,7 +444,7 @@ private void addDatabricksSpecificTags(
       AgentSpanContext parentContext =
           new DatabricksParentContext(databricksJobId, databricksJobRunId, databricksTaskRunId);
 
-      if (parentContext.getTraceId() != DDTraceId.ZERO) {
+      if (!parentContext.getTraceId().isZero()) {
         if (withParentContext) {
           builder.asChildOf(parentContext);
         } else {

dd-trace-api/build.gradle.kts

@@ -16,6 +16,7 @@ val excludedClassesCoverage by extra(
     "datadog.trace.api.DDTags",
     "datadog.trace.api.DDTraceApiInfo",
     "datadog.trace.api.DDTraceId",
+    "datadog.trace.api.DDTraceId.ConstantId",
     "datadog.trace.api.EventTracker",
     "datadog.trace.api.GlobalTracer*",
     "datadog.trace.api.PropagationStyle",

dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java

@@ -59,11 +59,10 @@ public static DD64bTraceId fromHex(String s) throws NumberFormatException {
   }
 
   static DD64bTraceId create(long id, String str) {
-    // ZERO constant is created and stored by the parent class as part of its API contract
-    // But initialized by this 64-bit child class. Ensures uniqueness of ZERO once created.
-    if (id == 0 && ZERO != null) {
-      return (DD64bTraceId) ZERO;
-    } else if (id == -1) {
+    // -1 (all bits set) reuses the MAX singleton. 0 is not special-cased: DDTraceId.ZERO is a
+    // sibling type (not a DD64bTraceId), so it cannot be returned here; a zero 64-bit id is simply
+    // a DD64bTraceId(0), and callers detect zero via DDTraceId.isZero() rather than by identity.
+    if (id == -1) {
       return MAX;
     } else {
       return new DD64bTraceId(id, str);

dd-trace-api/src/main/java/datadog/trace/api/DDTraceId.java

@@ -9,11 +9,25 @@
  * The string representations are either kept from parsing, or generated on demand and cached.
  */
 public abstract class DDTraceId {
-  /** Invalid TraceId value used to denote no TraceId. */
-  public static final DDTraceId ZERO = from(0);
+  /**
+   * Invalid TraceId value used to denote no TraceId.
+   *
+   * <p>This is an instance of a private {@link DDTraceId} subtype (a sibling of {@link
+   * DD64bTraceId}), not a {@code DD64bTraceId}, and that is deliberate. {@code DD64bTraceId} is a
+   * subclass of {@code DDTraceId}, so the JVM must initialize {@code DDTraceId} before {@code
+   * DD64bTraceId}. If this constant were built via {@code DD64bTraceId.from(0)} (as it once was),
+   * {@code DDTraceId.<clinit>} would initialize {@code DD64bTraceId} while holding the {@code
+   * DDTraceId} init lock, and two threads first touching the classes from opposite ends would
+   * deadlock on the two class-initialization locks. Building it from a sibling type keeps {@code
+   * DDTraceId.<clinit>} free of any reference to the subclass.
+   *
+   * <p>To test whether an id is zero, prefer {@link #isZero()} over {@code == DDTraceId.ZERO}: a
+   * zero id parsed via the 64-bit factories is a distinct instance, not this singleton.
+   */
+  public static final DDTraceId ZERO = new ConstantId(0, "0");
 
-  /** Convenience constant used from tests */
-  public static final DDTraceId ONE = from(1);
+  /** Convenience constant used from tests. See {@link #ZERO} for why this is a sibling type. */
+  public static final DDTraceId ONE = new ConstantId(1, "1");
 
   /**
    * Creates a new {@link DD64bTraceId 64-bit TraceId} from the given {@code long} interpreted as
@@ -102,4 +116,70 @@ public static DDTraceId fromHex(String s) throws NumberFormatException {
    *     <code>0</code> for 64-bit {@link DDTraceId} only.
    */
   public abstract long toHighOrderLong();
+
+  /**
+   * Returns whether this {@link DDTraceId} is zero, the value used to denote no/invalid TraceId.
+   *
+   * <p>Prefer this over {@code == DDTraceId.ZERO}: it is value-based, so it recognizes a zero id
+   * regardless of its concrete type or how it was created (e.g. a zero parsed via the 64-bit
+   * factories, which is a distinct instance from the {@link #ZERO} singleton).
+   *
+   * @return {@code true} if both the high- and low-order 64 bits are zero.
+   */
+  public boolean isZero() {
+    return toHighOrderLong() == 0 && toLong() == 0;
+  }
+
+  /**
+   * Minimal concrete {@link DDTraceId} backing the {@link #ZERO} and {@link #ONE} constants. It is
+   * a sibling of {@link DD64bTraceId} (it extends {@link DDTraceId} directly), so constructing
+   * these constants in {@code DDTraceId.<clinit>} never initializes {@code DD64bTraceId} (which
+   * would create a class-initialization deadlock; see {@link #ZERO}). It represents a 64-bit id and
+   * formats identically to the equivalent {@link DD64bTraceId}.
+   */
+  private static final class ConstantId extends DDTraceId {
+    private final long id;
+    private final String str;
+    private String hexStr; // cache for hex string representation
+
+    private ConstantId(long id, String str) {
+      this.id = id;
+      this.str = str;
+    }
+
+    @Override
+    public String toString() {
+      return this.str;
+    }
+
+    @Override
+    public String toHexString() {
+      String hexStr = this.hexStr;
+      // This race condition is intentional and benign.
+      // The worst that can happen is that an identical value is produced and written into the
+      // field.
+      if (hexStr == null) {
+        this.hexStr = hexStr = LongStringUtils.toHexStringPadded(this.id, 32);
+      }
+      return hexStr;
+    }
+
+    @Override
+    public String toHexStringPadded(int size) {
+      if (size > 16) {
+        return toHexString();
+      }
+      return LongStringUtils.toHexStringPadded(this.id, size);
+    }
+
+    @Override
+    public long toLong() {
+      return this.id;
+    }
+
+    @Override
+    public long toHighOrderLong() {
+      return 0;
+    }
+  }
 }

dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdClinitDeadlockForkedTest.java

@@ -0,0 +1,88 @@
+package datadog.trace.api;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicReference;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+/**
+ * Regression test for the {@code DDTraceId} <-> {@code DD64bTraceId} class-initialization
+ * deadlock.
+ *
+ * <p>{@code DD64bTraceId} is a subclass of {@code DDTraceId}, so the JVM must initialize {@code
+ * DDTraceId} before {@code DD64bTraceId}. The bug was that {@code DDTraceId.<clinit>} in turn
+ * initialized {@code DD64bTraceId} by building its {@code ZERO}/{@code ONE} constants via {@code
+ * DD64bTraceId.from(...)}. When the two classes were first touched concurrently from opposite ends
+ * (one thread initializing {@code DDTraceId}, another initializing {@code DD64bTraceId}), each
+ * thread held one class-initialization lock and waited for the other, hanging trace creation. This
+ * surfaced as 30s {@code LogInjectionSmokeTest} timeouts in CI.
+ *
+ * <p>{@code DDTraceId.ZERO}/{@code ONE} are now instances of a private sibling type (not {@code
+ * DD64bTraceId}), so {@code DDTraceId.<clinit>} no longer references {@code DD64bTraceId} and the
+ * cycle is gone. This test initializes the two classes for the first time concurrently from
+ * opposite ends and asserts neither thread hangs.
+ *
+ * <p>Runs forked ({@code forkEvery = 1}) so it gets a fresh JVM in which these classes have not yet
+ * been initialized by another test. Without the fix it deadlocks and fails via the join check (and
+ * the {@code @Timeout} backstop); with the fix it completes immediately.
+ */
+class DDTraceIdClinitDeadlockForkedTest {
+
+  @Test
+  @Timeout(value = 60, unit = SECONDS) // backstop; the join below is the primary guard
+  void traceIdClassPairInitializesConcurrentlyWithoutDeadlock() throws Exception {
+    final ClassLoader cl = getClass().getClassLoader();
+    final CyclicBarrier barrier = new CyclicBarrier(2);
+    final AtomicReference<Throwable> error = new AtomicReference<>();
+
+    // One thread enters via the superclass (mirrors blackholeSpan() -> DDTraceId.ZERO), the other
+    // via the subclass (mirrors IdGenerationStrategy.generateTraceId() -> DD64bTraceId.from()).
+    Thread viaSuper =
+        new Thread(
+            () -> {
+              try {
+                barrier.await();
+                Class.forName("datadog.trace.api.DDTraceId", true, cl);
+              } catch (Throwable t) {
+                error.compareAndSet(null, t);
+              }
+            },
+            "init-DDTraceId");
+    Thread viaSub =
+        new Thread(
+            () -> {
+              try {
+                barrier.await();
+                Class.forName("datadog.trace.api.DD64bTraceId", true, cl);
+              } catch (Throwable t) {
+                error.compareAndSet(null, t);
+              }
+            },
+            "init-DD64bTraceId");
+    // Daemon so a deadlock cannot block forked-JVM shutdown.
+    viaSuper.setDaemon(true);
+    viaSub.setDaemon(true);
+
+    viaSuper.start();
+    viaSub.start();
+    viaSuper.join(SECONDS.toMillis(15));
+    viaSub.join(SECONDS.toMillis(15));
+
+    if (viaSuper.isAlive() || viaSub.isAlive()) {
+      fail(
+          "DDTraceId/DD64bTraceId class-initialization deadlock: DDTraceId.<clinit> must not "
+              + "reference DD64bTraceId (init-DDTraceId.alive="
+              + viaSuper.isAlive()
+              + ", init-DD64bTraceId.alive="
+              + viaSub.isAlive()
+              + ").");
+    }
+    if (error.get() != null) {
+      throw new AssertionError(
+          "Unexpected error during concurrent class initialization", error.get());
+    }
+  }
+}

dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdConstantsTest.java

@@ -0,0 +1,40 @@
+package datadog.trace.api;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+
+/** Behavior of the {@link DDTraceId#ZERO}/{@link DDTraceId#ONE} sibling constants. */
+class DDTraceIdConstantsTest {
+
+  @Test
+  void zeroAndOneFormatLikeTheEquivalentDD64bTraceId() {
+    assertEquals("0", DDTraceId.ZERO.toString());
+    assertEquals("00000000000000000000000000000000", DDTraceId.ZERO.toHexString());
+    assertEquals("0000000000000000", DDTraceId.ZERO.toHexStringPadded(16));
+    assertEquals("00000000000000000000000000000000", DDTraceId.ZERO.toHexStringPadded(32));
+    assertEquals(0L, DDTraceId.ZERO.toLong());
+    assertEquals(0L, DDTraceId.ZERO.toHighOrderLong());
+
+    assertEquals("1", DDTraceId.ONE.toString());
+    assertEquals("00000000000000000000000000000001", DDTraceId.ONE.toHexString());
+    assertEquals("0000000000000001", DDTraceId.ONE.toHexStringPadded(16));
+    assertEquals(1L, DDTraceId.ONE.toLong());
+    assertEquals(0L, DDTraceId.ONE.toHighOrderLong());
+  }
+
+  @Test
+  void isZeroReflectsTheValue() {
+    assertTrue(DDTraceId.ZERO.isZero());
+    assertTrue(DDTraceId.from(0).isZero());
+    assertTrue(DDTraceId.from("0").isZero());
+    assertTrue(DDTraceId.fromHex("0").isZero());
+    assertTrue(DD64bTraceId.from(0).isZero());
+
+    assertFalse(DDTraceId.ONE.isZero());
+    assertFalse(DDTraceId.from(1).isZero());
+    assertFalse(DD64bTraceId.from(42).isZero());
+  }
+}

dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java

@@ -2,8 +2,8 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -175,10 +175,11 @@ void failParsingIllegal128BitIdHexadecimalStringRepresentationFromPartialString(
   @Test
   void checkZeroConstantInitialization() {
     DDTraceId zero = DDTraceId.ZERO;
-    DDTraceId fromZero = DDTraceId.from(0);
 
     assertNotNull(zero);
-    assertSame(fromZero, zero);
+    assertTrue(zero.isZero());
+    // from(0) is a zero-valued id but, by design, no longer the ZERO singleton; use isZero().
+    assertTrue(DDTraceId.from(0).isZero());
   }
 
   private static String leftPadWithZeros(String value, int size) {

dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java

@@ -2025,7 +2025,7 @@ protected static final DDSpanContext buildSpanContext(
           propagationTags = extractedContext.getPropagationTags();
         } else if (resolvedParentContext != null) {
           traceId =
-              resolvedParentContext.getTraceId() == DDTraceId.ZERO
+              resolvedParentContext.getTraceId().isZero()
                   ? tracer.idGenerationStrategy.generateTraceId()
                   : resolvedParentContext.getTraceId();
           parentSpanId = resolvedParentContext.getSpanId();

dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java

@@ -250,7 +250,7 @@ public ContextInterpreter reset(TraceConfig traceConfig) {
 
   protected TagContext build() {
     if (valid) {
-      if (fullContext && !DDTraceId.ZERO.equals(traceId)) {
+      if (fullContext && !traceId.isZero()) {
         if (propagationTags == null) {
           propagationTags = propagationTagsFactory.empty();
         }
@@ -305,7 +305,7 @@ private TagContext.HttpHeaders getHeaders() {
   }
 
   private int samplingPriorityOrDefault(DDTraceId traceId, int samplingPriority) {
-    return samplingPriority == PrioritySampling.UNSET || DDTraceId.ZERO.equals(traceId)
+    return samplingPriority == PrioritySampling.UNSET || traceId.isZero()
         ? defaultSamplingPriority()
         : samplingPriority;
   }

dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java

@@ -234,7 +234,7 @@ private long extractEndToEndStartTime(String value) {
     private void restore128bTraceId() {
       long highOrderBits;
       // Check if the low-order 64 bits of the TraceId, and propagation tags were parsed
-      if (traceId != DDTraceId.ZERO
+      if (!traceId.isZero()
           && propagationTags != null
           && (highOrderBits = propagationTags.getTraceIdHighOrderBits()) != 0) {
         // Restore the 128-bit TraceId

dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java

@@ -12,7 +12,6 @@
 import datadog.trace.api.DD64bTraceId;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTags;
-import datadog.trace.api.DDTraceId;
 import datadog.trace.api.TraceConfig;
 import datadog.trace.api.TracePropagationStyle;
 import datadog.trace.api.sampling.PrioritySampling;
@@ -212,7 +211,10 @@ static void handleXRayTraceHeader(ContextInterpreter interpreter, String value)
           }
           String part = value.substring(startPart, endPart).trim();
           if (part.startsWith(ROOT_PREFIX)) {
-            if (interpreter.traceId == null || interpreter.traceId == DDTraceId.ZERO) {
+            // isZero() (not == DDTraceId.ZERO): DD64bTraceId.fromHex below returns a non-singleton
+            // zero-valued id, so a zero Root must still be treated as "unset" and overwritten by a
+            // later valid Root in the same header.
+            if (interpreter.traceId == null || interpreter.traceId.isZero()) {
               interpreter.traceId =
                   DD64bTraceId.fromHex(part.substring(ROOT_PREAMBLE + TRACE_ID_PADDING.length()));
             }

dd-trace-core/src/test/java/datadog/trace/api/TraceIdIsZeroTest.java

@@ -0,0 +1,40 @@
+package datadog.trace.api;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import datadog.trace.core.propagation.B3TraceId;
+import org.junit.jupiter.api.Test;
+
+/**
+ * {@link DDTraceId#isZero()} across every {@link DDTraceId} subtype, including the {@code ZERO}/
+ * {@code ONE} sibling constants and {@link B3TraceId} (defined in dd-trace-core).
+ *
+ * <p>{@code isZero()} is the value-based replacement for the old {@code == DDTraceId.ZERO} identity
+ * checks, so it must recognize a zero id regardless of concrete type or how it was created (a zero
+ * parsed via the 64-bit factories is a distinct instance from the {@code ZERO} singleton).
+ */
+class TraceIdIsZeroTest {
+
+  @Test
+  void zeroValuedIdsOfEveryTypeAreZero() {
+    assertTrue(DDTraceId.ZERO.isZero());
+    assertTrue(DDTraceId.from(0).isZero());
+    assertTrue(DDTraceId.from("0").isZero());
+    assertTrue(DDTraceId.fromHex("0").isZero());
+    assertTrue(DD64bTraceId.from(0).isZero());
+    assertTrue(DD128bTraceId.from(0, 0).isZero());
+    assertTrue(B3TraceId.fromHex("0").isZero());
+  }
+
+  @Test
+  void nonZeroIdsAreNotZero() {
+    assertFalse(DDTraceId.ONE.isZero());
+    assertFalse(DDTraceId.from(1).isZero());
+    assertFalse(DD64bTraceId.from(42).isZero());
+    assertFalse(DD128bTraceId.from(0, 1).isZero());
+    // High-order bits set, low-order zero: still not a zero TraceId.
+    assertFalse(DD128bTraceId.from(7, 0).isZero());
+    assertFalse(B3TraceId.fromHex("2a").isZero());
+  }
+}

internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java

@@ -62,7 +62,7 @@ static AgentSpan fromSpanContext(AgentSpanContext spanContext) {
    * @return {@code true} if the span is considered valid, {@code false} otherwise.
    */
   default boolean isValid() {
-    return getTraceId() != DDTraceId.ZERO && getSpanId() != DDSpanId.ZERO;
+    return !getTraceId().isZero() && getSpanId() != DDSpanId.ZERO;
   }
 
   @Override