diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index fb66f082a3c..a64805c854b 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -29,6 +29,7 @@ import datadog.trace.bootstrap.instrumentation.jdbc.DBQueryInfo; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionContext; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser; +import datadog.trace.util.ClassLatch; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.sql.ClientInfoStatus; @@ -48,6 +49,15 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final Logger log = LoggerFactory.getLogger(JDBCDecorator.class); + /** Old drivers and pool proxies may not implement getClientInfo at all. */ + private static final ClassLatch CLIENT_INFO_LATCH = + new ClassLatch() { + @Override + protected Properties apply(Connection connection) throws SQLException { + return handleAbstractMethod(connection, "getClientInfo", Connection::getClientInfo); + } + }; + public static final JDBCDecorator DECORATE = new JDBCDecorator(); public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc"); public static final CharSequence DATABASE_QUERY = UTF8BytesString.create("database.query"); @@ -246,9 +256,10 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - clientInfo = connection.getClientInfo(); + clientInfo = CLIENT_INFO_LATCH.tryApply(connection); } catch (final Throwable ex) { - // getClientInfo is likely not allowed, we can still extract info from the url alone + // getClientInfo can fail in many ways (old drivers, pool proxies, test doubles), and we + // can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); } dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo); diff --git a/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java new file mode 100644 index 00000000000..bbf0c6a88e7 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java @@ -0,0 +1,133 @@ +package datadog.trace.instrumentation.jdbc; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.SQLException; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +/** How {@code parseDBInfoFromConnection} copes with connections whose getClientInfo fails. */ +class ParseDBInfoClientInfoTest { + private static final String URL = "jdbc:postgresql://db.example.com:5432/orders"; + + interface ClientInfoAnswer { + Properties get() throws Throwable; + } + + private static Connection connection(AtomicInteger clientInfoCalls, ClientInfoAnswer answer) { + DatabaseMetaData metaData = + (DatabaseMetaData) + Proxy.newProxyInstance( + DatabaseMetaData.class.getClassLoader(), + new Class[] {DatabaseMetaData.class}, + (proxy, method, args) -> "getURL".equals(method.getName()) ? URL : null); + InvocationHandler handler = + (proxy, method, args) -> { + switch (method.getName()) { + case "getMetaData": + return metaData; + case "getClientInfo": + clientInfoCalls.incrementAndGet(); + try { + return answer.get(); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + default: + return null; + } + }; + return (Connection) + Proxy.newProxyInstance( + Connection.class.getClassLoader(), new Class[] {Connection.class}, handler); + } + + @Test + void urlIsStillParsedWhenGetClientInfoThrowsSqlException() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new SQLException("not allowed"); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void urlIsStillParsedWhenGetClientInfoIsUnsupported() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new UnsupportedOperationException(); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void urlIsStillParsedWhenGetClientInfoIsMissing() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new AbstractMethodError("driver predates JDBC 4.0"); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void anyOtherFailureStillYieldsUrlBasedDbInfo() { + AtomicInteger calls = new AtomicInteger(); + for (Throwable failure : + new Throwable[] { + new IllegalStateException("unexpected"), new Throwable("not even an Exception") + }) { + Connection connection = + connection( + calls, + () -> { + throw failure; + }); + + // getClientInfo can fail in any way; the URL alone is still enough for the DB info + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + } + + @Test + void returnsTheClientInfoWhenAvailable() { + AtomicInteger calls = new AtomicInteger(); + Properties clientInfo = new Properties(); + Connection connection = connection(calls, () -> clientInfo); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals(1, calls.get()); + } +} diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java new file mode 100644 index 00000000000..be048bae005 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -0,0 +1,382 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; + +import java.io.File; +import java.io.IOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * What {@link ClassLatch#handleAbstractMethod} saves when an implementation lacks an interface + * method. + * + *

The missing method is real: {@code Impl} is built against an old {@code I}, and the caller + * against a newer {@code I} that added {@code b()}, so the call raises the JVM's own {@link + * AbstractMethodError}. Every arm reaches it through the same {@link MethodHandle}. The latches are + * {@code static final} anonymous subclasses, as they would be at a call site, and each arm has its + * own method so that no arm's profile is shaped by another's. + * + *

    + *
  • {@code unguardedMissing}: the status quo -- throw and catch on every call. + *
  • {@code latchedMissing}: the latch has latched {@code Impl}, so the call is skipped. + *
  • {@code latchedWrapperMissing}: the called object is a wrapper whose delegate lacks the + * method. The error names the delegate, so nothing may be latched and every call still + * throws. This is the price of staying safe; it should match {@code unguardedMissing}. + *
  • {@code unguardedPresent} / {@code latchedPresent}: the method exists. The difference is the + * latch's overhead on the path that works. + *
  • {@code subclassMissing} / {@code subclassPresent}: the same policy written as a reusable + * abstract subclass overriding {@code invoke}, instead of a method reference passed to {@code + * handleAbstractMethod}. It is a benchmark-local copy: it shows whether a dedicated class + * would be worth shipping. + *
+ * + *

The cost of a throw grows with the depth of the stack it fills in, which is why {@code depth} + * is a parameter: a benchmark thread's stack is shallow, a request thread's is not. + * + *

Run with {@code ./gradlew :internal-api:jmh -Pjmh.includes=ClassLatchBenchmark + * -Pjmh.profilers=gc}. + * + *

Results: ops/s, single thread, Zulu 17.0.7, MacBook M1, 2 forks, one run. JDK 8 and x86 are + * not measured. + * + * 

+ * Benchmark                                        (depth)       ops/s   B/op
+ * ClassLatchBenchmark.unguardedMissing                   0     220,977    896
+ * ClassLatchBenchmark.latchedWrapperMissing              0     228,684    896
+ * ClassLatchBenchmark.latchedMissing                     0 201,587,502      0
+ * ClassLatchBenchmark.subclassMissing                    0 201,915,340      0
+ * ClassLatchBenchmark.unguardedPresent                   0 223,537,261      0
+ * ClassLatchBenchmark.latchedPresent                     0 203,305,896      0
+ * ClassLatchBenchmark.subclassPresent                    0 227,339,864      0   (+-22%)
+ *
+ * ClassLatchBenchmark.unguardedMissing                  50     164,166  2,256
+ * ClassLatchBenchmark.latchedWrapperMissing             50     166,284  2,256
+ * ClassLatchBenchmark.unguardedPresent                  50  32,451,009      0
+ * ClassLatchBenchmark.latchedPresent                    50  28,001,510      0
+ *
+ * + * A latched class costs about 5 ns instead of about 4.5 us and allocates nothing instead of 896 B + * per call. A wrapper, which cannot be latched, performs like the status quo. On the working path + * the latch adds about 0.5 ns at depth 0. At depth 0 the method-reference form ({@code + * handleAbstractMethod}) and the dedicated-subclass form are indistinguishable on the latched path. + * + *

Depth 50, latched arms: not reported. Each fork was stable, but forks landed in + * different compiled states. {@code latchedMissing} ran at about 11.6M ops/s in one fork and about + * 31.7M in the other, {@code subclassMissing} at about 11.5M in both, {@code subclassPresent} at + * about 36M and 32M. So the mean and its error are two modes averaged, and the ranking of the two + * forms at depth 50 is not established. Both modes (roughly 85 ns and 32 ns) are far below the + * status quo of about 6 us. The cause was not investigated. + * + *

Results, one run: Zulu 17.0.7 (HotSpot), MacBook M1, single thread, 5 forks, on a laptop with + * normal background activity (load about 4). JDK 8 and x86 are not measured. {@code latched} uses + * {@code handleAbstractMethod} with a method reference; {@code subclass} is a benchmark-local + * dedicated subclass, for comparison. + * + * 

+ * Benchmark                (depth)          ops/s     ns/op    err   B/op
+ * unguardedMissing               0        227,738    4391.0   0.4%    896
+ * latchedWrapperMissing          0        232,316    4304.5   0.5%    896
+ * latchedMissing                 0    205,379,219      4.87   0.6%      0
+ * subclassMissing                0    205,736,674      4.86   0.6%      0
+ * unguardedPresent               0    224,381,889      4.46   0.4%      0
+ * latchedPresent                 0    209,902,233      4.76   0.3%      0
+ * subclassPresent                0    211,697,167      4.72   0.1%      0
+ *
+ * unguardedMissing              50        165,183    6053.9   1.2%   2256
+ * latchedWrapperMissing         50        171,300    5837.7   0.5%   2256
+ * latchedMissing                50     11,741,580      85.2   0.8%      0
+ * subclassMissing               50     11,842,809      84.4   0.3%      0
+ * unguardedPresent              50     35,092,870      28.5   3.4%      0
+ * latchedPresent                50     35,079,191      28.5   3.3%      0
+ * subclassPresent               50     36,004,724      27.8   3.0%      0
+ *
+ * + * A latched class costs about 4.9 ns where the status quo costs about 4.4 us (6.1 us at depth 50), + * and allocates nothing where the status quo allocates 896 B (2,256 B): roughly 900 times cheaper + * at depth 0 and 70 times at depth 50. A wrapper, which cannot be latched, performs like the status + * quo. The method-reference form and the dedicated subclass are indistinguishable (within 1%). The + * latch adds about 0.3 ns on the working path at depth 0 and nothing measurable at depth 50. + * + *

Unexplained: at depth 50 a latched skip (about 85 ns) is slower than a call that runs (about + * 28.5 ns), though at depth 0 the skip is as cheap as expected. All five forks agreed (11.7M to + * 11.9M ops/s per fork for the latched arm), so it is not noise. It may be a JIT effect of the + * benchmark's recursion rather than a property of the latch, but that was not tested. + */ +@Fork(2) +@Warmup(iterations = 3) +@Measurement(iterations = 4) +@Threads(1) +@State(Scope.Benchmark) +public class ClassLatchBenchmark { + + @Param({"0", "50"}) + int depth; + + private Path dir; + private URLClassLoader loader; + private Object impl; + private Object full; + private Object wrapper; + + /** Set in {@link #setup}; every arm and latch calls through it. */ + private static MethodHandle handle; + + private static Object invokeHandle(Object target) { + try { + return (Object) handle.invokeExact(target); + } catch (RuntimeException | Error e) { + throw e; + } catch (Throwable e) { + throw new IllegalStateException(e); + } + } + + private static final ClassLatch MISSING = + new ClassLatch() { + @Override + protected Object apply(Object target) { + return handleAbstractMethod(target, "b", ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch WRAPPER = + new ClassLatch() { + @Override + protected Object apply(Object target) { + return handleAbstractMethod(target, "b", ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch PRESENT = + new ClassLatch() { + @Override + protected Object apply(Object target) { + return handleAbstractMethod(target, "b", ClassLatchBenchmark::invokeHandle); + } + }; + + // the policy as a reusable subclass, for comparison only + private abstract static class SubclassStyle extends ClassLatch { + protected abstract Object invoke(Object target); + + @Override + protected final Object apply(Object target) { + try { + return invoke(target); + } catch (AbstractMethodError e) { + latchIfNamed(target, "b", e); + return null; + } catch (UnsupportedOperationException e) { + return null; + } + } + } + + private static final SubclassStyle SUBCLASS_MISSING = + new SubclassStyle() { + @Override + protected Object invoke(Object target) { + return invokeHandle(target); + } + }; + + private static final SubclassStyle SUBCLASS_PRESENT = + new SubclassStyle() { + @Override + protected Object invoke(Object target) { + return invokeHandle(target); + } + }; + + @Setup + public void setup() throws Throwable { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + if (compiler == null) { + throw new IllegalStateException("needs a JDK to build the classes under test"); + } + dir = Files.createTempDirectory("abstract-method-latch"); + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Full", + "public class Full implements I {" + + " public String a() { return \"a\"; } public String b() { return \"b\"; } }"); + compile( + compiler, + newDir, + newDir, + "Wrapper", + "public class Wrapper implements I { private final I delegate;" + + " public Wrapper(I delegate) { this.delegate = delegate; }" + + " public String a() { return delegate.a(); }" + + " public String b() { return delegate.b(); } }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + // the new interface shadows the old one; Impl was built against the old one + loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + ClassLatchBenchmark.class.getClassLoader()); + Class iface = loader.loadClass("I"); + impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + full = loader.loadClass("Full").getDeclaredConstructor().newInstance(); + wrapper = loader.loadClass("Wrapper").getDeclaredConstructor(iface).newInstance(impl); + + handle = + MethodHandles.publicLookup() + .findStatic( + loader.loadClass("Caller"), "call", MethodType.methodType(String.class, iface)) + .asType(MethodType.methodType(Object.class, Object.class)); + + // reach the steady state: the latch has already met the deficient class + MISSING.tryApply(impl); + if (!MISSING.isLatched(impl)) { + throw new IllegalStateException("expected the latch to latch " + impl.getClass()); + } + WRAPPER.tryApply(wrapper); + if (WRAPPER.isLatched(wrapper) || WRAPPER.isLatched(impl)) { + throw new IllegalStateException("a wrapper must never be latched"); + } + SUBCLASS_MISSING.tryApply(impl); + if (!SUBCLASS_MISSING.isLatched(impl)) { + throw new IllegalStateException( + "expected the subclass-style latch to latch " + impl.getClass()); + } + } + + @TearDown + public void tearDown() throws IOException { + loader.close(); + } + + @Benchmark + public Object unguardedMissing() { + return unguardedMissing(depth); + } + + @Benchmark + public Object latchedMissing() { + return latchedMissing(depth); + } + + @Benchmark + public Object latchedWrapperMissing() { + return latchedWrapperMissing(depth); + } + + @Benchmark + public Object unguardedPresent() { + return unguardedPresent(depth); + } + + @Benchmark + public Object latchedPresent() { + return latchedPresent(depth); + } + + @Benchmark + public Object subclassMissing() { + return subclassMissing(depth); + } + + @Benchmark + public Object subclassPresent() { + return subclassPresent(depth); + } + + // Each arm descends on its own so that a throw has a realistic amount of stack to fill in. + + private Object unguardedMissing(int remaining) { + if (remaining > 0) { + return unguardedMissing(remaining - 1); + } + try { + return invokeHandle(impl); + } catch (AbstractMethodError e) { + return null; + } + } + + private Object latchedMissing(int remaining) { + return remaining > 0 ? latchedMissing(remaining - 1) : MISSING.tryApply(impl); + } + + private Object latchedWrapperMissing(int remaining) { + return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.tryApply(wrapper); + } + + private Object unguardedPresent(int remaining) { + return remaining > 0 ? unguardedPresent(remaining - 1) : invokeHandle(full); + } + + private Object latchedPresent(int remaining) { + return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.tryApply(full); + } + + private Object subclassMissing(int remaining) { + return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.tryApply(impl); + } + + private Object subclassPresent(int remaining) { + return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.tryApply(full); + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + if (result != 0) { + throw new IllegalStateException("compiling " + name + " failed"); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java b/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java new file mode 100644 index 00000000000..9b2d2fe010c --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java @@ -0,0 +1,10 @@ +package datadog.trace.api.function; + +/** + * A function that may throw a checked exception, typically a method reference such as {@code + * Connection::getClientInfo}. The exception type is a parameter so that it flows through the + * caller: a function that throws nothing checked declares {@code RuntimeException}. + */ +public interface ThrowingFunction { + R apply(T t) throws E; +} diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java new file mode 100644 index 00000000000..088384ccb46 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -0,0 +1,299 @@ +package datadog.trace.util; + +import datadog.trace.api.function.Strategy; +import datadog.trace.api.function.StrategyConsumer; +import datadog.trace.api.function.ThrowingFunction; +import javax.annotation.Nullable; + +/** + * A per-class latch for an operation that, once it has failed for a class, will fail the same way + * for every instance of that class: an interface method the class does not implement, for example. + * A failure that is the same for everyone, whatever the class, needs only a single flag and no + * per-class state. + * + *

Intended as a {@code static final} anonymous subclass, one per call site and per operation: a + * class lacking one method says nothing about another, so latches must not be shared. As a constant + * of a known exact type, the receiver lets the JIT inline {@link #apply} and {@link #keyOf}. + * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #apply}, + * so checked exceptions and a tight {@code try} scope come for free, and latch through the + * protected helpers. Only the declaring subclass can change the state. + * + *

{@link #fallback} is what a latched target yields instead of the operation: {@code null} + * unless overridden. The helpers return it too when the operation fails, so the failing call and + * every skipped call after it agree. Override it when the operation has a slower alternative, such + * as an older API, rather than checking {@link #isLatched} at the call site. + * + *

{@link #keyOf} chooses the class the latch is keyed on, and is used by every operation, so the + * check and the latch cannot disagree. The default is the target's own class. Never key on a + * wrapper whose contents can differ from one instance to the next; override {@link #keyOf} to + * return the class of the object that is actually deficient. + * + *

This is a hint, not a lock. Nothing is stored for a class until it is latched, and until then + * the cost is one plain flag read. The state is deliberately not atomic. A stale read only costs + * another failure; a thread always sees its own write, so each thread pays for at most one failure + * after its own first. Other threads' writes become visible eventually, with no bound on how long + * that takes. A class is never latched unless the subclass latched it. + * + * @param the type of the value the operation is applied to + * @param the type of the result + * @param the checked exception {@link #apply} may throw + */ +public abstract class ClassLatch { + private static final String RECEIVER_PREFIX = "Receiver class "; + + /** + * Per-class state. Created eagerly so that it is safely published through a final field; it holds + * nothing for a class until {@link ClassValue#get} is called for it. The value is a JDK type so + * that nothing from the agent class loader is referenced from an application class. + */ + private final ClassValue latched = new Latches(); + + /** Whether any class has been latched; keeps the per-class lookup off the common path. */ + private boolean anyLatched; + + private static final class Latches extends ClassValue { + @Override + protected boolean[] computeValue(Class type) { + return new boolean[1]; + } + } + + /** Performs the operation. Latch through the protected helpers when it failed for the class. */ + @Nullable + protected abstract R apply(T target) throws E; + + /** + * What a latched target yields instead of the operation; the helpers also return it when the + * operation fails. {@code null} unless overridden. A latch that calls {@link #latch} directly + * should return this too, so that the failing call and later skipped calls agree. + */ + @Nullable + protected R fallback(T target) throws E { + return null; + } + + /** The class the latch is keyed on. The target's own class unless overridden. */ + protected Class keyOf(T target) { + return target.getClass(); + } + + /** + * Performs the operation, returning {@code null} for a {@code null} target and {@link #fallback} + * for a latched one. A {@code null} result means nothing is available: the target was {@code + * null}, or neither the operation nor the fallback produced a value. + */ + @Nullable + public final R tryApply(@Nullable T target) throws E { + if (target == null) { + return null; + } + if (isLatched(target)) { + return fallback(target); + } + try { + return apply(target); + } catch (NoSuchMethodError e) { + // a method reference passed to handleNoSuchMethod resolves where it is written, in apply's + // own frame, so a missing target method surfaces here, outside the helper's own try, as the + // invokedynamic call site's own linkage failure; HotSpot reports this directly as + // NoSuchMethodError on some JVM versions + latch(target); + return fallback(target); + } catch (BootstrapMethodError e) { + // on other JVM versions the same linkage failure is wrapped instead + if (e.getCause() instanceof NoSuchMethodError) { + latch(target); + return fallback(target); + } + throw e; + } + } + + /** + * Like {@link #tryApply}, but returns {@code defaultValue} when there is nothing available. It is + * also used when the operation or {@link #fallback} itself produced {@code null}, so a call and a + * skipped call always agree. + */ + public final R tryApplyOrDefault(@Nullable T target, R defaultValue) throws E { + final R result = tryApply(target); + return result != null ? result : defaultValue; + } + + /** Returns whether the operation is being skipped for the target. */ + public final boolean isLatched(@Nullable T target) { + return target != null && anyLatched && latched.get(keyOf(target))[0]; + } + + /** Skips the operation for the target's key from now on. */ + protected final void latch(T target) { + latched.get(keyOf(target))[0] = true; + // after the write: a reader that sees the flag can look the class up, and one that sees the + // flag but not yet the write just performs the operation once more + anyLatched = true; + } + + /** Resumes performing the operation for the target's key, for tests or a policy that retries. */ + protected final void unlatch(T target) { + if (anyLatched) { + latched.get(keyOf(target))[0] = false; + } + } + + /** + * Latches the target's key if the error's message names that class as the receiver that lacks + * {@code methodName}. Returns whether it latched. An error that does not name the key, for + * example one thrown inside a wrapper's delegate, is left alone — and so is one that names the + * key but for a different method, for example one the guarded method's own implementation calls + * internally. + */ + protected final boolean latchIfNamed(T target, String methodName, AbstractMethodError error) { + if (isNamedIn(error, keyOf(target), methodName)) { + latch(target); + return true; + } + return false; + } + + /** + * For a call to a method that some implementations may lack: returns {@link #fallback} if the + * call raises {@link AbstractMethodError} or {@link UnsupportedOperationException}, latching the + * key first in the former case if, and only if, the error names it (see {@link #latchIfNamed}). + * An error that does not name the key still yields the fallback; it just is not latched. An + * unsupported operation names no class, so it is never latched, and is caught on every call. + * Anything else, checked exceptions included, propagates unchanged. + * + * 

{@code
+   * protected Properties apply(Connection c) throws SQLException {
+   *   return handleAbstractMethod(c, "getClientInfo", Connection::getClientInfo);
+   * }
+   * }
+ * + * This covers only {@link AbstractMethodError}. A method that may also be missing from the + * classes on the classpath altogether raises {@link NoSuchMethodError}, which this does not + * handle; see {@link #handleNoSuchMethod} and {@link #handleNoSuchOrAbstractMethod}, and prefer + * the latter when unsure which a call site can see. + * + *

{@code methodName} must be {@code call}'s own method, not merely some method of {@code T}: + * the error's message can name the key class while blaming a different method that the guarded + * method's implementation happens to call internally, and only a method name check tells the two + * apart. + * + *

Pass a method reference or a non-capturing lambda, and keep this method small so it inlines: + * that is what lets the JIT see the exact function at each call site (see {@link Strategy}). + * Compose {@link #latchIfNamed} and {@link #latch} directly for anything more involved. + */ + @Nullable + @StrategyConsumer + protected final R handleAbstractMethod( + T target, String methodName, @Strategy ThrowingFunction call) throws E { + try { + return call.apply(target); + } catch (AbstractMethodError e) { + latchIfNamed(target, methodName, e); + return fallback(target); + } catch (UnsupportedOperationException e) { + // no class to attribute it to, and it may come from a delegate: never latched + return fallback(target); + } + } + + /** + * For a call to a method that is missing from the classes on the classpath altogether, for + * example because the caller was built against a newer library than the one present: returns + * {@link #fallback} if the call raises {@link NoSuchMethodError}, latching the target's key + * first. Anything else propagates unchanged; see {@link #handleNoSuchOrAbstractMethod} if the + * method may instead be present but unimplemented by some classes ({@link AbstractMethodError}). + * + *

A {@link NoSuchMethodError} is a failure of resolution, which the JVM keeps for the call + * site, but it can equally be raised by a call made inside one receiver's + * implementation. Its message names the declared type, not the receiver, so it cannot be + * attributed to a class. It therefore latches the target's key, never the whole call site: a + * site-wide failure then costs one throw per receiver class, and an inner failure stays confined + * to the class that has it. + * + * 

{@code
+   * protected Properties apply(Connection c) throws SQLException {
+   *   return handleNoSuchMethod(c, Connection::getClientInfo);
+   * }
+   * }
+ */ + @Nullable + @StrategyConsumer + protected final R handleNoSuchMethod(T target, @Strategy ThrowingFunction call) + throws E { + try { + return call.apply(target); + } catch (NoSuchMethodError e) { + latch(target); + return fallback(target); + } + } + + /** + * For a call to a method that may be missing ({@link NoSuchMethodError}) or unimplemented by some + * classes ({@link AbstractMethodError}): both yield {@link #fallback}, as does {@link + * UnsupportedOperationException}. This is {@link #handleAbstractMethod} and {@link + * #handleNoSuchMethod} together, with the same latching rules: an {@link AbstractMethodError} + * latches only if its message names the key, and a {@link NoSuchMethodError} latches the target's + * key. The two are easy to confuse, so prefer this one unless you know which a call site can see. + * + * 
{@code
+   * protected Properties apply(Connection c) throws SQLException {
+   *   return handleNoSuchOrAbstractMethod(c, "getClientInfo", Connection::getClientInfo);
+   * }
+   * }
+ */ + @Nullable + @StrategyConsumer + protected final R handleNoSuchOrAbstractMethod( + T target, String methodName, @Strategy ThrowingFunction call) throws E { + try { + return handleAbstractMethod(target, methodName, call); + } catch (NoSuchMethodError e) { + latch(target); + return fallback(target); + } + } + + /** + * Matches the receiver class named by HotSpot's message against a class. Two formats exist: + * + *
    + *
  • JDK 11+: {@code Receiver class X does not define or inherit an implementation of the + * resolved method ...} + *
  • JDK 8: {@code X.method(descriptor)} + *
+ * + * A concrete object's class is never the abstract method's declaring class or an interface, so an + * exact match can only be the receiver. The message also names the resolved method itself: that + * method's implementation can call a different, unimplemented method on the very same receiver, + * raising an error that names the key class but blames a method other than {@code methodName}, so + * the method name is checked too. An unparseable message never matches. + */ + static boolean isNamedIn(AbstractMethodError e, Class type, String methodName) { + final String message = e.getMessage(); + if (message == null) { + return false; + } + final String name = type.getName(); + if (message.startsWith(RECEIVER_PREFIX)) { + final int end = RECEIVER_PREFIX.length() + name.length(); + return message.startsWith(name, RECEIVER_PREFIX.length()) + && message.length() > end + && message.charAt(end) == ' ' + && message.indexOf(methodName + "(", end) > end; + } + if (message.startsWith(name) && message.length() > name.length() + 1) { + // JDK 8: the method name follows the class name and runs up to the descriptor, so it + // contains no '.'; that rules out a longer class name sharing this one as a prefix + final int methodStart = name.length() + 1; + final int paren = message.indexOf('(', methodStart); + return message.charAt(name.length()) == '.' + && paren > methodStart + && message.indexOf('.', methodStart) < 0 + && paren == methodStart + methodName.length() + && message.regionMatches(methodStart, methodName, 0, methodName.length()); + } + return false; + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java new file mode 100644 index 00000000000..c41c439d8ca --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java @@ -0,0 +1,322 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class ClassLatchTest { + + private static AbstractMethodError receiverError(Class type) { + return new AbstractMethodError( + "Receiver class " + + type.getName() + + " does not define or inherit an implementation of the resolved method 'abstract" + + " java.lang.String m()' of interface I."); + } + + /** Latches the target's class unconditionally on any IllegalStateException. */ + private static class Counting extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + + @Override + protected String apply(Object target) { + calls.incrementAndGet(); + try { + throw new IllegalStateException(); + } catch (IllegalStateException e) { + latch(target); + return "failed"; + } + } + } + + @Test + void latchesTheTargetsClassAndSkipsLaterCalls() { + Counting latch = new Counting(); + + assertEquals("failed", latch.tryApply("x")); + assertNull(latch.tryApply("y")); + assertNull(latch.tryApply("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffected() { + Counting latch = new Counting(); + latch.tryApply("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertEquals("failed", latch.tryApply(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + } + + @Test + void nullTargetReturnsTheDefaultWithoutCalling() { + Counting latch = new Counting(); + + assertNull(latch.tryApply(null)); + assertFalse(latch.isLatched(null)); + assertEquals(0, latch.calls.get()); + } + + @Test + void tryApplyOrDefaultReturnsTheResultWhenThereIsOne() { + ClassLatch latch = + new ClassLatch() { + @Override + protected Boolean apply(Object target) { + return false; + } + }; + + // a real false must not be replaced by the fallback + assertEquals(false, latch.tryApplyOrDefault("x", Boolean.TRUE)); + } + + @Test + void tryApplyOrDefaultReturnsTheFallbackWhenSkippedOrNull() { + Counting latch = new Counting(); + + // the first call latches and yields a value; later calls are skipped + assertEquals("failed", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault(null, "fallback")); + } + + @Test + void aCallThatYieldsNothingAndASkippedCallAgree() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + latch(target); + return null; + } + }; + + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); + } + + @Test + void aNullTargetYieldsNullNotTheFallback() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + return "called"; + } + + @Override + protected String fallback(Object target) { + return "fallback"; + } + }; + + assertNull(latch.tryApply(null)); + assertEquals("default", latch.tryApplyOrDefault(null, "default")); + } + + @Test + void tryApplyOrDefaultPrefersTheFallbackOverTheDefault() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + latch(target); + return fallback(target); + } + + @Override + protected String fallback(Object target) { + return "fallback"; + } + }; + + assertEquals("fallback", latch.tryApplyOrDefault("x", "default")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "default")); + } + + @Test + void keyOfChoosesTheClassToLatch() { + // a wrapper whose contents differ: latch on what it holds, never on the wrapper itself + final class Wrapper { + final Object delegate; + + Wrapper(Object delegate) { + this.delegate = delegate; + } + } + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Wrapper target) { + latch(target); + return "called"; + } + + @Override + protected Class keyOf(Wrapper target) { + return target.delegate.getClass(); + } + }; + + assertEquals("called", latch.tryApply(new Wrapper("x"))); + + assertTrue(latch.isLatched(new Wrapper("another string"))); + assertFalse(latch.isLatched(new Wrapper(Integer.valueOf(1)))); + assertEquals("called", latch.tryApply(new Wrapper(Integer.valueOf(1)))); + } + + /** A subclass may expose {@code unlatch}, for a policy that retries. */ + private static final class Resumable extends ClassLatch { + @Override + protected String apply(Object target) { + latch(target); + return "called"; + } + + void resume(Object target) { + unlatch(target); + } + } + + @Test + void unlatchResumesForThatKeyOnly() { + Resumable latch = new Resumable(); + latch.tryApply("x"); + latch.tryApply(Integer.valueOf(1)); + assertTrue(latch.isLatched("x")); + assertTrue(latch.isLatched(Integer.valueOf(1))); + + latch.resume("x"); + + assertFalse(latch.isLatched("x")); + assertTrue(latch.isLatched(Integer.valueOf(1))); + assertEquals("called", latch.tryApply("x")); + } + + @Test + void latchIfNamedLatchesOnlyWhenTheErrorNamesTheKey() { + final boolean[] result = new boolean[1]; + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + result[0] = latchIfNamed(target, "m", receiverError(target.getClass())); + return "named"; + } + }; + latch.tryApply("x"); + assertTrue(result[0]); + assertTrue(latch.isLatched("x")); + + ClassLatch other = + new ClassLatch() { + @Override + protected String apply(Object target) { + result[0] = latchIfNamed(target, "m", receiverError(Integer.class)); + return "other"; + } + }; + other.tryApply("x"); + assertFalse(result[0]); + assertFalse(other.isLatched("x")); + } + + @Test + void latchIfNamedDoesNotLatchWhenTheErrorNamesTheKeyButADifferentMethod() { + // the receiver's own implementation of "m" can call some other method internally; an + // AbstractMethodError raised by that other method still names the receiver class, but it is + // not evidence that "m" itself is missing + final boolean[] result = new boolean[1]; + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + result[0] = + latchIfNamed( + target, + "m", + new AbstractMethodError( + "Receiver class " + + target.getClass().getName() + + " does not define or inherit an implementation of the resolved" + + " method 'abstract java.lang.String other()' of interface I.")); + return "named"; + } + }; + latch.tryApply("x"); + assertFalse(result[0]); + assertFalse(latch.isLatched("x")); + } + + @Test + void attributesTheHotSpotMessageFormats() { + assertTrue(ClassLatch.isNamedIn(receiverError(String.class), String.class, "m")); + // the message names the receiver class, but blames a different method than "m" + assertFalse(ClassLatch.isNamedIn(receiverError(String.class), String.class, "other")); + // "java.lang.String" is a prefix of "java.lang.StringBuilder", but not the same class + assertFalse(ClassLatch.isNamedIn(receiverError(String.class), StringBuilder.class, "m")); + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError("Receiver class " + String.class.getName()), + String.class, + "m")); + + // JDK 8 reports "." + String name = String.class.getName(); + assertTrue( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".getClientInfo()Ljava/util/Properties;"), + String.class, + "getClientInfo")); + // the message names the receiver class, but blames a different method + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".otherMethod()Ljava/util/Properties;"), + String.class, + "getClientInfo")); + // a different class whose name merely starts with this one + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + "Builder.getClientInfo()Ljava/util/Properties;"), + String.class, + "getClientInfo")); + // a class in a package named like this class + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".Inner.getClientInfo()Ljava/util/Properties;"), + String.class, + "getClientInfo")); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name), String.class, "getClientInfo")); + assertFalse( + ClassLatch.isNamedIn(new AbstractMethodError(name + "."), String.class, "getClientInfo")); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), String.class, "getClientInfo")); + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError("something else"), String.class, "getClientInfo")); + } + + @Test + void checkedExceptionsPropagateWithoutLatching() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) throws SQLException { + throw new SQLException("boom"); + } + }; + + assertThrows(SQLException.class, () -> latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/CompilingClassLoaders.java b/internal-api/src/test/java/datadog/trace/util/CompilingClassLoaders.java new file mode 100644 index 00000000000..97e0281fe3e --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/CompilingClassLoaders.java @@ -0,0 +1,35 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import javax.tools.JavaCompiler; + +/** Shared support for tests that compile throwaway sources to pin real JVM error shapes. */ +final class CompilingClassLoaders { + private CompilingClassLoaders() {} + + static void compile(JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + assertEquals(0, result, "compiling " + name); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java new file mode 100644 index 00000000000..4d2befa8faf --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java @@ -0,0 +1,491 @@ +package datadog.trace.util; + +import static datadog.trace.util.CompilingClassLoaders.compile; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +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 static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class HandleAbstractMethodTest { + + /** What a call site writes: {@code apply} delegating to {@code handleAbstractMethod}. */ + private abstract static class Handling extends ClassLatch { + protected abstract R invoke(T target) throws E; + + protected String methodName() { + return "m"; + } + + @Override + protected final R apply(T target) throws E { + return handleAbstractMethod(target, methodName(), this::invoke); + } + } + + private static AbstractMethodError receiverError(Class type) { + return receiverError(type, "m"); + } + + private static AbstractMethodError receiverError(Class type, String methodName) { + return new AbstractMethodError( + "Receiver class " + + type.getName() + + " does not define or inherit an implementation of the resolved method 'abstract" + + " java.lang.String " + + methodName + + "()' of interface I."); + } + + private static final class Throwing extends Handling { + final AtomicInteger calls = new AtomicInteger(); + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String invoke(Object target) throws Exception { + calls.incrementAndGet(); + if (failure instanceof AbstractMethodError) { + // name the class of whatever was called, as the JVM does for the receiver + throw receiverError(target.getClass()); + } + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + } + } + + @Test + void returnsTheResultAndLatchesNothing() throws Exception { + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + return "ok"; + } + }; + + assertEquals("ok", latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void latchesTheReceiverClassAndStopsCalling() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("y")); + assertNull(latch.tryApply("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffectedByALatch() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + latch.tryApply("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.tryApply(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + } + + @Test + void doesNotLatchWhenTheErrorNamesAnotherClass() throws Exception { + // a wrapper whose delegate lacks the method: the error names the delegate, not the wrapper + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + throw receiverError(Integer.class); + } + }; + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("x")); + + assertEquals(2, calls.get()); + assertFalse(latch.isLatched("x")); + } + + /** Fails like {@link Throwing} with an {@link AbstractMethodError}, but has a fallback. */ + private static final class WithFallback extends Handling { + final AtomicInteger calls = new AtomicInteger(); + final AtomicInteger fallbacks = new AtomicInteger(); + final Class named; + final String result; + + WithFallback(Class named, String result) { + this.named = named; + this.result = result; + } + + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + if (named == null) { + return result; + } + throw receiverError(named == Object.class ? target.getClass() : named); + } + + @Override + protected String fallback(Object target) { + fallbacks.incrementAndGet(); + return "fallback"; + } + } + + @Test + void theFailingCallAndLaterSkippedCallsBothYieldTheFallback() { + // Object.class stands for "name the receiver's own class" + WithFallback latch = new WithFallback(Object.class, null); + + assertEquals("fallback", latch.tryApply("x")); + assertEquals("fallback", latch.tryApply("y")); + + assertEquals(1, latch.calls.get()); + assertEquals(2, latch.fallbacks.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void anUnnamedErrorYieldsTheFallbackWithoutLatching() { + // the fallback is correct whether or not the error can be attributed; latching only caches it + WithFallback latch = new WithFallback(Integer.class, null); + + assertEquals("fallback", latch.tryApply("x")); + assertEquals("fallback", latch.tryApply("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void aRealNullIsNotReplacedByTheFallback() { + // an operation that succeeds with null is not a failure: the fallback is never consulted + WithFallback latch = new WithFallback(null, null); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("x")); + + assertEquals(2, latch.calls.get()); + assertEquals(0, latch.fallbacks.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void anUnsupportedOperationYieldsTheFallbackWithoutLatching() { + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + throw new UnsupportedOperationException(); + } + + @Override + protected String fallback(Object target) { + return "fallback"; + } + }; + + assertEquals("fallback", latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void doesNotLatchWhenTheErrorNamesTheKeyButADifferentMethod() throws Exception { + // "m"'s own implementation calls a different method internally; that method's + // AbstractMethodError still names the receiver class, but is not evidence "m" is missing + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + throw receiverError(target.getClass(), "other"); + } + }; + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("x")); + + assertEquals(2, calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void doesNotLatchWhenTheMessageCannotBeAttributed() throws Exception { + AtomicInteger calls = new AtomicInteger(); + for (AbstractMethodError error : + new AbstractMethodError[] { + new AbstractMethodError(), new AbstractMethodError("something else entirely") + }) { + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + throw error; + } + }; + + assertNull(latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } + assertEquals(2, calls.get()); + } + + @Test + void unsupportedOperationYieldsTheDefaultOnEveryCallAndIsNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void checkedExceptionsPropagateAndDoNotLatch() { + SQLException failure = new SQLException("boom"); + Throwing latch = new Throwing(failure); + + SQLException thrown = assertThrows(SQLException.class, () -> latch.tryApply("x")); + + assertSame(failure, thrown); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherUncheckedExceptionsPropagateAndDoNotLatch() { + Throwing latch = new Throwing(new IllegalStateException()); + + assertThrows(IllegalStateException.class, () -> latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } + + /** + * Pins the HotSpot message format that attribution depends on, using a real error: a class built + * against an old interface, called through code built against a newer one. + */ + @Test + void latchesARealAbstractMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + String vm = System.getProperty("java.vm.name", ""); + assumeTrue(vm.contains("HotSpot") || vm.contains("OpenJDK"), "message format is HotSpot's"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + HandleAbstractMethodTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected String methodName() { + return "b"; + } + + @Override + protected Object invoke(Object target) throws Exception { + calls.incrementAndGet(); + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof AbstractMethodError) { + throw (AbstractMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.tryApply(impl)); + assertNull(latch.tryApply(impl)); + + assertEquals(1, calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } + + /** + * Pins the real nested failure behind the method-identity check: a default implementation that + * calls a different, unimplemented method on the same receiver raises an error naming the + * receiver class, but for that other method, not the one being guarded. + */ + @Test + void doesNotLatchARealNestedAbstractMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + String vm = System.getProperty("java.vm.name", ""); + assumeTrue(vm.contains("HotSpot") || vm.contains("OpenJDK"), "message format is HotSpot's"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile( + compiler, oldDir, null, "I", "public interface I { default String a() { return \"a\"; } }"); + compile(compiler, oldDir, oldDir, "Impl", "public class Impl implements I { }"); + compile( + compiler, + newDir, + null, + "I", + "public interface I { String b(); default String a() { return b(); } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + HandleAbstractMethodTest.class.getClassLoader())) { + loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method a = loader.loadClass("I").getMethod("a"); + + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected Object invoke(Object target) throws Exception { + calls.incrementAndGet(); + try { + return a.invoke(target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof AbstractMethodError) { + throw (AbstractMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.tryApply(impl)); + assertNull(latch.tryApply(impl)); + + assertEquals( + 2, calls.get(), "an error naming the key but not the guarded method must not latch"); + assertFalse(latch.isLatched(impl)); + } + } + + /** + * Pins the real failure behind {@code handleNoSuchMethod}'s method-reference argument: it + * resolves where it is written, in {@code apply}'s own frame, before the guarded call is made, so + * a target method missing altogether surfaces there, bypassing the helper's own {@code try/catch} + * entirely. HotSpot reports it as a {@link BootstrapMethodError} wrapping {@link + * NoSuchMethodError} on some JVM versions and as a bare {@link NoSuchMethodError} on others; + * {@link #tryApply} must catch both. + */ + @Test + void latchesARealBootstrapMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "import java.util.function.Function;\n" + + "public class Caller {\n" + + " public static String call(I i) {\n" + + " Function fn = I::b;\n" + + " return fn.apply(i);\n" + + " }\n" + + "}\n"); + + try (URLClassLoader loader = + new URLClassLoader( + // old first: "I" (lacking b()) must win over Caller's compile-time "I" so that + // linking Caller's I::b reference fails, instead of linking fine and only failing + // later when b() is actually invoked on a receiver that doesn't implement it + new URL[] {oldDir.toUri().toURL(), newDir.toUri().toURL()}, + HandleAbstractMethodTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + AtomicInteger calls = new AtomicInteger(); + ClassLatch latch = + new ClassLatch() { + @Override + protected Object apply(Object target) throws Exception { + calls.incrementAndGet(); + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + // reflection always wraps the failure in InvocationTargetException; which error it + // wraps is what differs by JVM version + if (e.getCause() instanceof BootstrapMethodError) { + throw (BootstrapMethodError) e.getCause(); + } + if (e.getCause() instanceof NoSuchMethodError) { + throw (NoSuchMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.tryApply(impl)); + assertNull(latch.tryApply(impl)); + + assertEquals(1, calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java new file mode 100644 index 00000000000..1dcc6d063e4 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java @@ -0,0 +1,108 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class HandleNoSuchMethodTest { + + /** What a call site writes: {@code apply} delegating to {@code handleNoSuchMethod}. */ + private static final class Throwing extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String apply(Object target) throws Exception { + return handleNoSuchMethod( + target, + t -> { + calls.incrementAndGet(); + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + }); + } + } + + @Test + void returnsTheResultAndLatchesNothing() throws Exception { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + return handleNoSuchMethod(target, t -> "ok"); + } + }; + + assertEquals("ok", latch.tryApply("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void noSuchMethodYieldsTheFallbackNowAndOnceLatched() { + AtomicInteger calls = new AtomicInteger(); + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + return handleNoSuchMethod( + target, + t -> { + calls.incrementAndGet(); + throw new NoSuchMethodError(); + }); + } + + @Override + protected String fallback(Object target) { + return "fallback"; + } + }; + + assertEquals("fallback", latch.tryApply("x")); + assertEquals("fallback", latch.tryApply("x")); + assertEquals(1, calls.get()); + } + + @Test + void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("y")); + + assertEquals(1, latch.calls.get(), "later calls should be skipped"); + assertTrue(latch.isLatched("x")); + // the message names the declared type, not the receiver: another class gets its own attempt + assertFalse(latch.isLatched(Integer.valueOf(1))); + } + + @Test + void doesNotHandleAbstractMethodErrorOrUnsupportedOperation() { + Throwing abstractMethod = new Throwing(new AbstractMethodError("Impl.b()V")); + assertThrows(AbstractMethodError.class, () -> abstractMethod.tryApply("x")); + assertFalse(abstractMethod.isLatched("x")); + + Throwing unsupported = new Throwing(new UnsupportedOperationException()); + assertThrows(UnsupportedOperationException.class, () -> unsupported.tryApply("x")); + assertFalse(unsupported.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryApply("x")); + assertFalse(checked.isLatched("x")); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java new file mode 100644 index 00000000000..52abc86bb5e --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java @@ -0,0 +1,182 @@ +package datadog.trace.util; + +import static datadog.trace.util.CompilingClassLoaders.compile; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class HandleNoSuchOrAbstractMethodTest { + + /** What a call site writes: {@code apply} delegating to {@code handleNoSuchOrAbstractMethod}. */ + private abstract static class Handling extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + + protected abstract R invoke(T target) throws E; + + @Override + protected final R apply(T target) throws E { + return handleNoSuchOrAbstractMethod( + target, + "m", + t -> { + calls.incrementAndGet(); + return invoke(t); + }); + } + } + + private static final class Throwing extends Handling { + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String invoke(Object target) throws Exception { + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + } + } + + @Test + void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("y")); + assertNull(latch.tryApply("z")); + + assertEquals(1, latch.calls.get(), "later calls should be skipped"); + assertTrue(latch.isLatched("x")); + } + + @Test + void noSuchMethodNeverLatchesTheWholeSite() throws Exception { + // the message names the declared type, not the receiver, so it cannot be attributed to a class; + // latching only the target's key means another class still gets its own attempt + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + latch.tryApply("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.tryApply(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + assertTrue(latch.isLatched(Integer.valueOf(1))); + } + + @Test + void abstractMethodStillLatchesOnlyWhenTheErrorNamesTheKey() throws Exception { + Throwing named = + new Throwing( + new AbstractMethodError( + "Receiver class " + + String.class.getName() + + " does not define or inherit an implementation of the resolved method" + + " 'abstract java.lang.String m()' of interface I.")); + assertNull(named.tryApply("x")); + assertTrue(named.isLatched("x")); + + Throwing unnamed = new Throwing(new AbstractMethodError("something else entirely")); + assertNull(unnamed.tryApply("x")); + assertNull(unnamed.tryApply("x")); + assertFalse(unnamed.isLatched("x")); + assertEquals(2, unnamed.calls.get()); + } + + @Test + void unsupportedOperationIsSwallowedAndNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.tryApply("x")); + assertNull(latch.tryApply("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryApply("x")); + assertFalse(checked.isLatched("x")); + + Throwing unchecked = new Throwing(new IllegalStateException()); + assertThrows(IllegalStateException.class, () -> unchecked.tryApply("x")); + assertFalse(unchecked.isLatched("x")); + } + + /** + * A real {@link NoSuchMethodError}: a caller built against an interface that has {@code b()}, run + * against one that does not. + */ + @Test + void latchesARealNoSuchMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + oldDir, + oldDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + newDir, + newDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + HandleNoSuchOrAbstractMethodTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + Handling latch = + new Handling() { + @Override + protected Object invoke(Object target) throws Exception { + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof NoSuchMethodError) { + throw (NoSuchMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.tryApply(impl)); + assertNull(latch.tryApply(impl)); + + assertEquals(1, latch.calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } +}