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..ce32bf98b1e 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, 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.tryApplyOrNull(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..951e957e93d --- /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, ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch WRAPPER = + new ClassLatch() { + @Override + protected Object apply(Object target) { + return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch PRESENT = + new ClassLatch() { + @Override + protected Object apply(Object target) { + return handleAbstractMethod(target, 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, 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.tryApplyOrNull(impl); + if (!MISSING.isLatched(impl)) { + throw new IllegalStateException("expected the latch to latch " + impl.getClass()); + } + WRAPPER.tryApplyOrNull(wrapper); + if (WRAPPER.isLatched(wrapper) || WRAPPER.isLatched(impl)) { + throw new IllegalStateException("a wrapper must never be latched"); + } + SUBCLASS_MISSING.tryApplyOrNull(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.tryApplyOrNull(impl); + } + + private Object latchedWrapperMissing(int remaining) { + return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.tryApplyOrNull(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.tryApplyOrNull(full); + } + + private Object subclassMissing(int remaining) { + return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.tryApplyOrNull(impl); + } + + private Object subclassPresent(int remaining) { + return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.tryApplyOrNull(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..ce20efd869b --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -0,0 +1,248 @@ +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 #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; + + /** 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 unless the target is {@code null} or latched, in which case returns + * {@code null}. A {@code null} result means nothing is available: the operation was skipped, or + * it produced no value. + */ + @Nullable + public final R tryApplyOrNull(@Nullable T target) throws E { + return target == null || isLatched(target) ? null : apply(target); + } + + /** + * Like {@link #tryApplyOrNull}, but returns {@code fallback} when there is nothing available. The + * fallback is also used when the operation itself produced {@code null}, so a call and a skipped + * call always agree. + */ + public final R tryApplyOrDefault(@Nullable T target, R fallback) throws E { + final R result = tryApplyOrNull(target); + return result != null ? result : fallback; + } + + /** 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 the + * method. Returns whether it latched. An error that does not name the key, for example one thrown + * inside a wrapper's delegate, is left alone. + */ + protected final boolean latchIfNamed(T target, AbstractMethodError error) { + if (isNamedIn(error, keyOf(target))) { + latch(target); + return true; + } + return false; + } + + /** + * For a call to a method that some implementations may lack: returns {@code null} 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 + * 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, 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. + * + *

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, @Strategy ThrowingFunction call) + throws E { + try { + return call.apply(target); + } catch (AbstractMethodError e) { + latchIfNamed(target, e); + return null; + } catch (UnsupportedOperationException e) { + // no class to attribute it to, and it may come from a delegate: never latched + return null; + } + } + + /** + * 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 + * {@code null} 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 null; + } + } + + /** + * For a call to a method that may be missing ({@link NoSuchMethodError}) or unimplemented by some + * classes ({@link AbstractMethodError}): both yield {@code null}, 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, Connection::getClientInfo);
+   * }
+   * }
+ */ + @Nullable + @StrategyConsumer + protected final R handleNoSuchOrAbstractMethod(T target, @Strategy ThrowingFunction call) + throws E { + try { + return handleAbstractMethod(target, call); + } catch (NoSuchMethodError e) { + latch(target); + return null; + } + } + + /** + * 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. An unparseable message never matches. + */ + static boolean isNamedIn(AbstractMethodError e, Class type) { + 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) == ' '; + } + 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; + } + 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..2058e450293 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java @@ -0,0 +1,240 @@ +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.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); + assertNull(latch.tryApplyOrNull("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffected() { + Counting latch = new Counting(); + latch.tryApplyOrNull("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertEquals("failed", latch.tryApplyOrNull(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + } + + @Test + void nullTargetReturnsTheDefaultWithoutCalling() { + Counting latch = new Counting(); + + assertNull(latch.tryApplyOrNull(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 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.tryApplyOrNull(new Wrapper("x"))); + + assertTrue(latch.isLatched(new Wrapper("another string"))); + assertFalse(latch.isLatched(new Wrapper(Integer.valueOf(1)))); + assertEquals("called", latch.tryApplyOrNull(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.tryApplyOrNull("x"); + latch.tryApplyOrNull(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.tryApplyOrNull("x")); + } + + @Test + void latchIfNamedLatchesOnlyWhenTheErrorNamesTheKey() { + final boolean[] result = new boolean[1]; + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) { + result[0] = latchIfNamed(target, receiverError(target.getClass())); + return "named"; + } + }; + latch.tryApplyOrNull("x"); + assertTrue(result[0]); + assertTrue(latch.isLatched("x")); + + ClassLatch other = + new ClassLatch() { + @Override + protected String apply(Object target) { + result[0] = latchIfNamed(target, receiverError(Integer.class)); + return "other"; + } + }; + other.tryApplyOrNull("x"); + assertFalse(result[0]); + assertFalse(other.isLatched("x")); + } + + @Test + void attributesTheHotSpotMessageFormats() { + assertTrue(ClassLatch.isNamedIn(receiverError(String.class), String.class)); + // "java.lang.String" is a prefix of "java.lang.StringBuilder", but not the same class + assertFalse(ClassLatch.isNamedIn(receiverError(String.class), StringBuilder.class)); + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError("Receiver class " + String.class.getName()), String.class)); + + // JDK 8 reports "." + String name = String.class.getName(); + assertTrue( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".getClientInfo()Ljava/util/Properties;"), + String.class)); + // a different class whose name merely starts with this one + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + "Builder.getClientInfo()Ljava/util/Properties;"), + String.class)); + // a class in a package named like this class + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".Inner.getClientInfo()Ljava/util/Properties;"), + String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name + "."), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError("something else"), String.class)); + } + + @Test + void checkedExceptionsPropagateWithoutLatching() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String apply(Object target) throws SQLException { + throw new SQLException("boom"); + } + }; + + assertThrows(SQLException.class, () -> latch.tryApplyOrNull("x")); + assertFalse(latch.isLatched("x")); + } +} 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..d394f254aa6 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java @@ -0,0 +1,258 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +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.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +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; + + @Override + protected final R apply(T target) throws E { + return handleAbstractMethod(target, this::invoke); + } + } + + 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."); + } + + 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.tryApplyOrNull("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void latchesTheReceiverClassAndStopsCalling() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); + assertNull(latch.tryApplyOrNull("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffectedByALatch() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + latch.tryApplyOrNull("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.tryApplyOrNull(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.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("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.tryApplyOrNull("x")); + assertFalse(latch.isLatched("x")); + } + assertEquals(2, calls.get()); + } + + @Test + void unsupportedOperationYieldsTheDefaultOnEveryCallAndIsNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("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.tryApplyOrNull("x")); + + assertSame(failure, thrown); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherUncheckedExceptionsPropagateAndDoNotLatch() { + Throwing latch = new Throwing(new IllegalStateException()); + + assertThrows(IllegalStateException.class, () -> latch.tryApplyOrNull("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 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.tryApplyOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); + + assertEquals(1, calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } + + 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()); + assertEquals(0, result, "compiling " + name); + } +} 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..5ade6d31012 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java @@ -0,0 +1,82 @@ +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.tryApplyOrNull("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("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.tryApplyOrNull("x")); + assertFalse(abstractMethod.isLatched("x")); + + Throwing unsupported = new Throwing(new UnsupportedOperationException()); + assertThrows(UnsupportedOperationException.class, () -> unsupported.tryApplyOrNull("x")); + assertFalse(unsupported.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryApplyOrNull("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..743e1e796bf --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java @@ -0,0 +1,203 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +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.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +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, + 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.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); + assertNull(latch.tryApplyOrNull("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.tryApplyOrNull("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.tryApplyOrNull(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")); + assertNull(named.tryApplyOrNull("x")); + assertTrue(named.isLatched("x")); + + Throwing unnamed = new Throwing(new AbstractMethodError("something else entirely")); + assertNull(unnamed.tryApplyOrNull("x")); + assertNull(unnamed.tryApplyOrNull("x")); + assertFalse(unnamed.isLatched("x")); + assertEquals(2, unnamed.calls.get()); + } + + @Test + void unsupportedOperationIsSwallowedAndNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryApplyOrNull("x")); + assertFalse(checked.isLatched("x")); + + Throwing unchecked = new Throwing(new IllegalStateException()); + assertThrows(IllegalStateException.class, () -> unchecked.tryApplyOrNull("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.tryApplyOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); + + assertEquals(1, latch.calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } + + 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()); + assertEquals(0, result, "compiling " + name); + } +}