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 latch = + new ClassLatch() { + @Override + protected Class keyOf(Object target) { + return Unimplemented.class; + } + + @Override + protected String apply(Object target) { + result[0] = latchIfNamed(target, "m", new AbstractMethodError()); + return "unnamed"; + } + }; + latch.tryApply("x"); + assertTrue(result[0]); + assertTrue(latch.isLatched("x")); + } + @Test void checkedExceptionsPropagateWithoutLatching() { ClassLatch latch =