From 4d1e4defd462e5d6732bf703c39f36dd56055d5f Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Thu, 29 Jan 2026 16:35:37 -0500 Subject: [PATCH 1/3] Avoid cloning final fields --- .../java/datadog/trace/util/UnsafeUtils.java | 8 ++-- .../datadog/trace/util/UnsafeUtilsTest.groovy | 42 +++++++++++++++---- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java index 06cd5f832a0..5261a04d56d 100644 --- a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java @@ -1,6 +1,5 @@ package datadog.trace.util; -import de.thetaphi.forbiddenapis.SuppressForbidden; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import org.slf4j.Logger; @@ -58,11 +57,12 @@ public static T tryShallowClone(T original) { } } - // TODO: JEP 500 - avoid mutating final fields - @SuppressForbidden private static void cloneFields(Class clazz, Object original, Object clone) throws Exception { for (Field field : clazz.getDeclaredFields()) { - if ((field.getModifiers() & Modifier.STATIC) != 0) { + if ((field.getModifiers() & Modifier.FINAL) != 0) { + log.debug( + "Skipping cloning final field {}. Final fields cannot be mutated. See JEP 500 for more details.", + field.getName()); continue; } field.setAccessible(true); diff --git a/internal-api/src/test/groovy/datadog/trace/util/UnsafeUtilsTest.groovy b/internal-api/src/test/groovy/datadog/trace/util/UnsafeUtilsTest.groovy index 7bfa3976f83..8a75618e2a3 100644 --- a/internal-api/src/test/groovy/datadog/trace/util/UnsafeUtilsTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/util/UnsafeUtilsTest.groovy @@ -4,7 +4,7 @@ import spock.lang.Specification class UnsafeUtilsTest extends Specification { - def "test try shallow clone"() { + def "test try shallow clone does not clone final fields"() { given: def inner = new MyClass("a", false, [], "b", 2, null, null) def instance = new MyClass("aaa", true, [ 4, 5, 6, ], "ddd", 1, new int[] { @@ -16,13 +16,27 @@ class UnsafeUtilsTest extends Specification { then: clone !== instance - clone.a === instance.a - clone.b == instance.b - clone.c === instance.c - clone.d === instance.d - clone.e == instance.e - clone.f === instance.f - clone.g === instance.g + clone.a == null + clone.b == false + clone.c == null + clone.d == null + clone.e == 0 + clone.f == null + clone.g == null + } + + def "test try shallow clone clones non-final fields"() { + given: + def instance = new MyNonFinalClass("a", 1, [2, 3, 4]) + + when: + def clone = UnsafeUtils.tryShallowClone(instance) + + then: + clone !== instance + clone.h == instance.h + clone.j == instance.j + clone.k === instance.k } private static class MyParentClass { @@ -65,4 +79,16 @@ class UnsafeUtilsTest extends Specification { this.g = g } } + + private static class MyNonFinalClass { + private String h + private int j + private List k + + MyNonFinalClass(String h, int j, List k) { + this.h = h + this.j = j + this.k = k + } + } } From f66b1b55ebe7d467201b03eb807f997e2d24cccb Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Thu, 29 Jan 2026 16:52:50 -0500 Subject: [PATCH 2/3] Oops still need to suppressforbidden --- .../src/main/java/datadog/trace/util/UnsafeUtils.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java index 5261a04d56d..5c2e50c90b3 100644 --- a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import de.thetaphi.forbiddenapis.SuppressForbidden; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import org.slf4j.Logger; @@ -57,6 +58,10 @@ public static T tryShallowClone(T original) { } } + // Field::set() is forbidden because it may be used to mutate final fields, disallowed by + // https://openjdk.org/jeps/500. + // However, in this case we skip final fields, so it is safe. + @SuppressForbidden private static void cloneFields(Class clazz, Object original, Object clone) throws Exception { for (Field field : clazz.getDeclaredFields()) { if ((field.getModifiers() & Modifier.FINAL) != 0) { From 47184656aa0d65c27fca2294cf0f882b318be71a Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Fri, 30 Jan 2026 17:01:46 -0500 Subject: [PATCH 3/3] First attempt at nonfinal cloning --- internal-api/build.gradle.kts | 2 + .../java/datadog/trace/util/UnsafeUtils.java | 53 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/internal-api/build.gradle.kts b/internal-api/build.gradle.kts index b78e5b38b14..8ad3716f97e 100644 --- a/internal-api/build.gradle.kts +++ b/internal-api/build.gradle.kts @@ -270,6 +270,8 @@ dependencies { // it contains annotations that are also present in the instrumented application classes api("com.datadoghq:dd-javac-plugin-client:0.2.2") + implementation(libs.bytebuddy) + testImplementation("org.snakeyaml:snakeyaml-engine:2.9") testImplementation(project(":utils:test-utils")) testImplementation(libs.bundles.junit5) diff --git a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java index 5c2e50c90b3..0b4f983233a 100644 --- a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java @@ -1,8 +1,16 @@ package datadog.trace.util; +import static net.bytebuddy.matcher.ElementMatchers.isFinal; + import de.thetaphi.forbiddenapis.SuppressForbidden; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.asm.ModifierAdjustment; +import net.bytebuddy.description.modifier.FieldManifestation; +import net.bytebuddy.dynamic.DynamicType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sun.misc.Unsafe; @@ -13,6 +21,8 @@ public abstract class UnsafeUtils { private static final Unsafe UNSAFE = getUnsafe(); + private static final Map> CLASS_CACHE = new ConcurrentHashMap<>(); + private static Unsafe getUnsafe() { try { Field f = Unsafe.class.getDeclaredField("theUnsafe"); @@ -37,7 +47,7 @@ private static Unsafe getUnsafe() { * @param Type of the object being cloned */ @SuppressWarnings("unchecked") - public static T tryShallowClone(T original) { + public static T originalTryShallowClone(T original) { if (UNSAFE == null) { log.debug("Unsafe is unavailable, {} will not be cloned", original); return original; @@ -58,16 +68,49 @@ public static T tryShallowClone(T original) { } } + public static T tryShallowClone(T original) { + if (UNSAFE == null) { + log.debug("Unsafe is unavailable, {} will not be cloned", original); + return original; + } + try { + Class clazz = original.getClass(); + if (!CLASS_CACHE.containsKey(clazz.getName())) { + CLASS_CACHE.put(clazz.getName(), createNonFinalSubclass(clazz)); + } + Class nonFinalSubclass = CLASS_CACHE.get(clazz.getName()); + + T clone = (T) UNSAFE.allocateInstance(nonFinalSubclass); + + while (clazz != Object.class) { + cloneFields(clazz, original, clone); + clazz = clazz.getSuperclass(); + } + return clone; + + } catch (Throwable t) { + log.debug("Error while cloning {}: {}", original, t); + t.printStackTrace(); + return original; + } + } + + private static Class createNonFinalSubclass(Class clazz) throws Exception { + DynamicType.Unloaded dynamicType = + new ByteBuddy() + .subclass(clazz) + .visit(new ModifierAdjustment().withFieldModifiers(isFinal(), FieldManifestation.PLAIN)) + .make(); + return dynamicType.load(clazz.getClassLoader()).getLoaded(); + } + // Field::set() is forbidden because it may be used to mutate final fields, disallowed by // https://openjdk.org/jeps/500. // However, in this case we skip final fields, so it is safe. @SuppressForbidden private static void cloneFields(Class clazz, Object original, Object clone) throws Exception { for (Field field : clazz.getDeclaredFields()) { - if ((field.getModifiers() & Modifier.FINAL) != 0) { - log.debug( - "Skipping cloning final field {}. Final fields cannot be mutated. See JEP 500 for more details.", - field.getName()); + if ((field.getModifiers() & Modifier.STATIC) != 0) { continue; } field.setAccessible(true);