diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java index b93ad101f3e..b9c6e41a3bb 100644 --- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -3,6 +3,9 @@ import datadog.trace.api.function.Strategy; import datadog.trace.api.function.StrategyConsumer; import datadog.trace.api.function.ThrowingFunction; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; @@ -143,10 +146,11 @@ protected final void unlatch(T target) { /** * 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. + * {@code methodName}, or, when the JVM gave no message, if that class still has {@code + * methodName} as an abstract 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 — 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)) { @@ -159,10 +163,10 @@ protected final boolean latchIfNamed(T target, String methodName, AbstractMethod /** * 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. + * key first in the former case if, and only if, the error is attributed to it (see {@link + * #latchIfNamed}). An error that is not attributed to 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 {
@@ -236,8 +240,9 @@ protected final R handleNoSuchMethod(T target, @Strategy ThrowingFunction{@code
* protected Properties apply(Connection c) throws SQLException {
@@ -257,6 +262,51 @@ protected final R handleNoSuchOrAbstractMethod(
}
}
+ /**
+ * Whether a public method named {@code methodName} is still abstract on {@code type}, as an
+ * interface method that a concrete class never implemented is. This attributes the error to the
+ * class just as a message naming it would: a wrapper that delegates the method has a concrete
+ * implementation, so an error raised by its delegate is not attributed to it. Overloads are not
+ * told apart. Each class is scanned once, whatever the answer, so a class that keeps failing
+ * without being latched does not repeat the reflection.
+ */
+ static boolean lacksImplementation(Class> type, String methodName) {
+ for (final String abstractMethod : ABSTRACT_METHOD_NAMES.get(type)) {
+ if (abstractMethod.equals(methodName)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static final String[] NO_NAMES = new String[0];
+
+ /**
+ * Per class, the names of its public methods that are still abstract. Only computed for classes
+ * that failed without a message. The value is a JDK type, like {@link #latched}'s.
+ */
+ private static final ClassValue ABSTRACT_METHOD_NAMES =
+ new ClassValue() {
+ @Override
+ protected String[] computeValue(Class> type) {
+ try {
+ int count = 0;
+ final Method[] methods = type.getMethods();
+ final String[] names = new String[methods.length];
+ for (final Method method : methods) {
+ if (Modifier.isAbstract(method.getModifiers())) {
+ names[count++] = method.getName();
+ }
+ }
+ return count == 0 ? NO_NAMES : Arrays.copyOf(names, count);
+ } catch (final SecurityException | LinkageError ignored) {
+ // cannot tell, so never latch: getMethods() resolves every public signature, and one
+ // that references a class missing from the class path raises NoClassDefFoundError
+ return NO_NAMES;
+ }
+ }
+ };
+
/**
* Matches the receiver class named by HotSpot's message against a class. Two formats exist:
*
@@ -271,11 +321,15 @@ protected final R handleNoSuchOrAbstractMethod(
* 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.
+ *
+ * JDK 8 omits the message once the call site has dispatched to a class that does implement the
+ * method, which in production is nearly always. Without a message the class itself is checked
+ * instead: see {@link #lacksImplementation}.
*/
static boolean isNamedIn(AbstractMethodError e, Class> type, String methodName) {
final String message = e.getMessage();
if (message == null) {
- return false;
+ return lacksImplementation(type, methodName);
}
final String name = type.getName();
if (message.startsWith(RECEIVER_PREFIX)) {
diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
index c41c439d8ca..6dd220f95cc 100644
--- a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
+++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
@@ -6,6 +6,9 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.sql.SQLException;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.Test;
@@ -300,12 +303,126 @@ void attributesTheHotSpotMessageFormats() {
assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name), String.class, "getClientInfo"));
assertFalse(
ClassLatch.isNamedIn(new AbstractMethodError(name + "."), String.class, "getClientInfo"));
+ // no message: String has no abstract getClientInfo, so the error is not attributed to it
assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), String.class, "getClientInfo"));
assertFalse(
ClassLatch.isNamedIn(
new AbstractMethodError("something else"), String.class, "getClientInfo"));
}
+ /** Stands in for a class that never implemented an interface method it declares. */
+ abstract static class Unimplemented {
+ public abstract String m();
+
+ public String implemented() {
+ return "ok";
+ }
+ }
+
+ /** Stands in for a wrapper that implements the method by delegating it. */
+ static class Delegating extends Unimplemented {
+ @Override
+ public String m() {
+ return "delegated";
+ }
+ }
+
+ @Test
+ void attributesAnErrorWithoutAMessageByTheClassItself() {
+ // JDK 8 gives no message once the call site has seen a class that implements the method
+ AbstractMethodError noMessage = new AbstractMethodError();
+ assertTrue(ClassLatch.isNamedIn(noMessage, Unimplemented.class, "m"));
+ // the class implements this method, so an error from it must come from elsewhere
+ assertFalse(ClassLatch.isNamedIn(noMessage, Unimplemented.class, "implemented"));
+ // a delegating wrapper is not blamed for its delegate's error
+ assertFalse(ClassLatch.isNamedIn(noMessage, Delegating.class, "m"));
+ assertFalse(ClassLatch.isNamedIn(noMessage, Unimplemented.class, "missing"));
+ }
+
+ /** Stands in for a type that is missing from the class path at run time. */
+ public static class Missing {}
+
+ /** A class with a public signature that references {@link Missing}. */
+ public static class ReferencesMissing {
+ public Missing m() {
+ return null;
+ }
+ }
+
+ @Test
+ void anUnresolvableSignatureMeansCannotTellNotAnEscapingError() throws Exception {
+ Class> type = withoutMissing().loadClass(ReferencesMissing.class.getName());
+ // getMethods() resolves every public signature, so the missing return type fails it
+ assertThrows(NoClassDefFoundError.class, type::getMethods);
+
+ assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), type, "m"));
+ // the answer is cached, so a second failure neither rescans nor throws
+ assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), type, "m"));
+ }
+
+ /** Defines {@link ReferencesMissing} itself, and refuses to load {@link Missing}. */
+ private static ClassLoader withoutMissing() {
+ return new ClassLoader(ClassLatchTest.class.getClassLoader()) {
+ @Override
+ protected Class> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (name.equals(Missing.class.getName())) {
+ throw new ClassNotFoundException(name);
+ }
+ if (!name.equals(ReferencesMissing.class.getName())) {
+ return super.loadClass(name, resolve);
+ }
+ synchronized (getClassLoadingLock(name)) {
+ Class> loaded = findLoadedClass(name);
+ if (loaded == null) {
+ byte[] bytes = classBytes(name);
+ loaded = defineClass(name, bytes, 0, bytes.length);
+ }
+ return loaded;
+ }
+ }
+ };
+ }
+
+ private static byte[] classBytes(String name) throws ClassNotFoundException {
+ try (InputStream in =
+ ClassLatchTest.class
+ .getClassLoader()
+ .getResourceAsStream(name.replace('.', '/') + ".class")) {
+ if (in == null) {
+ throw new ClassNotFoundException(name);
+ }
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ byte[] buffer = new byte[4096];
+ for (int read; (read = in.read(buffer)) != -1; ) {
+ out.write(buffer, 0, read);
+ }
+ return out.toByteArray();
+ } catch (IOException e) {
+ throw new ClassNotFoundException(name, e);
+ }
+ }
+
+ @Test
+ void latchIfNamedLatchesOnAnErrorWithoutAMessage() {
+ final boolean[] result = new boolean[1];
+ ClassLatch