diff --git a/src/main/java/org/mockito/exceptions/stacktrace/StackTraceCleaner.java b/src/main/java/org/mockito/exceptions/stacktrace/StackTraceCleaner.java index a3229a8c80..467809aa01 100644 --- a/src/main/java/org/mockito/exceptions/stacktrace/StackTraceCleaner.java +++ b/src/main/java/org/mockito/exceptions/stacktrace/StackTraceCleaner.java @@ -25,4 +25,29 @@ public interface StackTraceCleaner { * @return whether the element should be excluded from cleaned stack trace. */ boolean isIn(StackTraceElement candidate); + + /** + * It's recommended to override this method in subclasses to avoid potentially costly re-boxing operations. + */ + default boolean isIn(StackFrameMetadata candidate) { + return isIn( + new StackTraceElement( + candidate.getClassName(), + candidate.getMethodName(), + candidate.getFileName(), + candidate.getLineNumber())); + } + + /** + * Very similar to the StackFrame class declared on the StackWalker api. + */ + interface StackFrameMetadata { + String getClassName(); + + String getMethodName(); + + String getFileName(); + + int getLineNumber(); + } } diff --git a/src/main/java/org/mockito/internal/MockedConstructionImpl.java b/src/main/java/org/mockito/internal/MockedConstructionImpl.java index 47bd8089c6..5541ba07cd 100644 --- a/src/main/java/org/mockito/internal/MockedConstructionImpl.java +++ b/src/main/java/org/mockito/internal/MockedConstructionImpl.java @@ -11,7 +11,7 @@ import org.mockito.MockedConstruction; import org.mockito.exceptions.base.MockitoException; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.invocation.Location; import org.mockito.plugins.MockMaker; @@ -21,7 +21,7 @@ public final class MockedConstructionImpl implements MockedConstruction { private boolean closed; - private final Location location = new LocationImpl(); + private final Location location = LocationFactory.create(); protected MockedConstructionImpl(MockMaker.ConstructionMockControl control) { this.control = control; diff --git a/src/main/java/org/mockito/internal/MockedStaticImpl.java b/src/main/java/org/mockito/internal/MockedStaticImpl.java index fbfb54b004..f6705fb81a 100644 --- a/src/main/java/org/mockito/internal/MockedStaticImpl.java +++ b/src/main/java/org/mockito/internal/MockedStaticImpl.java @@ -17,7 +17,7 @@ import org.mockito.Mockito; import org.mockito.exceptions.base.MockitoAssertionError; import org.mockito.exceptions.base.MockitoException; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.listeners.VerificationStartedNotifier; import org.mockito.internal.progress.MockingProgress; import org.mockito.internal.stubbing.InvocationContainerImpl; @@ -35,7 +35,7 @@ public final class MockedStaticImpl implements MockedStatic { private boolean closed; - private final Location location = new LocationImpl(); + private final Location location = LocationFactory.create(); protected MockedStaticImpl(MockMaker.StaticMockControl control) { this.control = control; diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java index fc92e49acf..1dd744171f 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java @@ -51,7 +51,7 @@ import org.mockito.exceptions.base.MockitoException; import org.mockito.internal.configuration.plugins.Plugins; import org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.exceptions.stacktrace.ConditionalStackTraceFilter; import org.mockito.internal.invocation.RealMethod; import org.mockito.internal.invocation.SerializableMethod; @@ -132,11 +132,7 @@ public Callable handle(Object instance, Method origin, Object[] arguments) th } return new ReturnValueWrapper( interceptor.doIntercept( - instance, - origin, - arguments, - realMethod, - new LocationImpl(new Throwable(), true))); + instance, origin, arguments, realMethod, LocationFactory.create(true))); } @Override @@ -154,7 +150,7 @@ public Callable handleStatic(Class type, Method origin, Object[] arguments origin, arguments, new StaticMethodCall(selfCallInfo, type, origin, arguments), - new LocationImpl(new Throwable(), true))); + LocationFactory.create(true))); } @Override diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java index 83908cace6..406dea39a5 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java @@ -22,7 +22,7 @@ import net.bytebuddy.implementation.bind.annotation.StubValue; import net.bytebuddy.implementation.bind.annotation.SuperCall; import net.bytebuddy.implementation.bind.annotation.This; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.RealMethod; import org.mockito.invocation.Location; import org.mockito.invocation.MockHandler; @@ -53,7 +53,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo Object doIntercept(Object mock, Method invokedMethod, Object[] arguments, RealMethod realMethod) throws Throwable { - return doIntercept(mock, invokedMethod, arguments, realMethod, new LocationImpl()); + return doIntercept(mock, invokedMethod, arguments, realMethod, LocationFactory.create()); } Object doIntercept( diff --git a/src/main/java/org/mockito/internal/creation/proxy/ProxyMockMaker.java b/src/main/java/org/mockito/internal/creation/proxy/ProxyMockMaker.java index faa97e849b..88e6886111 100644 --- a/src/main/java/org/mockito/internal/creation/proxy/ProxyMockMaker.java +++ b/src/main/java/org/mockito/internal/creation/proxy/ProxyMockMaker.java @@ -5,7 +5,7 @@ package org.mockito.internal.creation.proxy; import org.mockito.exceptions.base.MockitoException; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.RealMethod; import org.mockito.internal.util.Platform; import org.mockito.invocation.MockHandler; @@ -153,7 +153,12 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl return handler.get() .handle( createInvocation( - proxy, method, args, realMethod, settings, new LocationImpl())); + proxy, + method, + args, + realMethod, + settings, + LocationFactory.create())); } } diff --git a/src/main/java/org/mockito/internal/debugging/LocationImpl.java b/src/main/java/org/mockito/internal/debugging/Java8LocationImpl.java similarity index 84% rename from src/main/java/org/mockito/internal/debugging/LocationImpl.java rename to src/main/java/org/mockito/internal/debugging/Java8LocationImpl.java index cf255013e2..e8ee387c0a 100644 --- a/src/main/java/org/mockito/internal/debugging/LocationImpl.java +++ b/src/main/java/org/mockito/internal/debugging/Java8LocationImpl.java @@ -9,7 +9,7 @@ import org.mockito.internal.exceptions.stacktrace.StackTraceFilter; import org.mockito.invocation.Location; -public class LocationImpl implements Location, Serializable { +class Java8LocationImpl implements Location, Serializable { private static final long serialVersionUID = -9054861157390980624L; // Limit the amount of objects being created, as this class is heavily instantiated: @@ -18,19 +18,11 @@ public class LocationImpl implements Location, Serializable { private String stackTraceLine; private String sourceFile; - public LocationImpl() { - this(new Throwable(), false); - } - - public LocationImpl(Throwable stackTraceHolder, boolean isInline) { + public Java8LocationImpl(Throwable stackTraceHolder, boolean isInline) { this(stackTraceFilter, stackTraceHolder, isInline); } - public LocationImpl(StackTraceFilter stackTraceFilter) { - this(stackTraceFilter, new Throwable(), false); - } - - private LocationImpl( + private Java8LocationImpl( StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) { computeStackTraceInformation(stackTraceFilter, stackTraceHolder, isInline); } diff --git a/src/main/java/org/mockito/internal/debugging/Java9PlusLocationImpl.java b/src/main/java/org/mockito/internal/debugging/Java9PlusLocationImpl.java new file mode 100644 index 0000000000..219835513e --- /dev/null +++ b/src/main/java/org/mockito/internal/debugging/Java9PlusLocationImpl.java @@ -0,0 +1,304 @@ +/* + * Copyright (c) 2007 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.internal.debugging; + +import org.mockito.exceptions.base.MockitoException; +import org.mockito.exceptions.stacktrace.StackTraceCleaner; +import org.mockito.exceptions.stacktrace.StackTraceCleaner.StackFrameMetadata; +import org.mockito.internal.configuration.plugins.Plugins; +import org.mockito.internal.exceptions.stacktrace.DefaultStackTraceCleaner; +import org.mockito.invocation.Location; + +import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +class Java9PlusLocationImpl implements Location, Serializable { + private static final long serialVersionUID = 2954388321980069195L; + + private static final String UNEXPECTED_ERROR_SUFFIX = + "\nThis is unexpected and is likely due to a change in either Java's StackWalker or Reflection APIs." + + "\nIt's worth trying to upgrade to a newer version of Mockito, or otherwise to file a bug report."; + + private static final String STACK_WALKER = "java.lang.StackWalker"; + private static final String STACK_FRAME = STACK_WALKER + "$StackFrame"; + private static final String OPTION = STACK_WALKER + "$Option"; + private static final String SHOW_REFLECT_FRAMES = "SHOW_REFLECT_FRAMES"; + + /** + * This is an unfortunate buffer. Inside StackWalker, a buffer is created, which is resized by + * doubling. The resizing also allocates a tonne of StackFrame elements. If we traverse more than + * BUFFER_SIZE elements, the resulting resize can significantly affect the overall cost of the operation. + * If we traverse fewer than this number, we are inefficient. Empirically, 16 is enough stack frames + * for a simple stub+call operation to succeed without resizing, as measured on Java 11. + */ + private static final int BUFFER_SIZE = 16; + + private static final Class stackWalkerClazz = clazz(STACK_WALKER); + private static final Class stackFrameClazz = clazz(STACK_FRAME); + private static final Class optionClazz = clazz(OPTION); + + private static final Object stackWalker = stackWalker(); + private static final Method walk = walk(); + + private static final String PREFIX = "-> at "; + + private static final StackTraceCleaner CLEANER = + Plugins.getStackTraceCleanerProvider() + .getStackTraceCleaner(new DefaultStackTraceCleaner()); + + /** + * In Java, allocating lambdas is cheap, but not free. stream.map(this::doSomething) + * will allocate a Function object each time the function is called (although not + * per element). By assigning these Functions and Predicates to variables, we can + * avoid the memory allocation. + */ + private static final Function toStackFrameMetadata = + MetadataShim::new; + + private static final Predicate cleanerIsIn = CLEANER::isIn; + + private static final int FRAMES_TO_SKIP = framesToSkip(); + + private final StackFrameMetadata sfm; + private volatile String stackTraceLine; + + Java9PlusLocationImpl(boolean isInline) { + this.sfm = getStackFrame(isInline); + } + + @Override + public String getSourceFile() { + return sfm.getFileName(); + } + + @Override + public String toString() { + return stackTraceLine(); + } + + private String stackTraceLine() { + if (stackTraceLine == null) { + synchronized (this) { + if (stackTraceLine == null) { + stackTraceLine = PREFIX + sfm.toString(); + } + } + } + return stackTraceLine; + } + + private static StackFrameMetadata getStackFrame(boolean isInline) { + return stackWalk( + stream -> + stream.map(toStackFrameMetadata) + .skip(FRAMES_TO_SKIP) + .filter(cleanerIsIn) + .skip(isInline ? 1 : 0) + .findFirst() + .orElseThrow( + () -> new MockitoException(noStackTraceFailureMessage()))); + } + + private static boolean usingDefaultStackTraceCleaner() { + return CLEANER instanceof DefaultStackTraceCleaner; + } + + private static String noStackTraceFailureMessage() { + if (usingDefaultStackTraceCleaner()) { + return "Mockito could not find the first non-Mockito stack frame." + + UNEXPECTED_ERROR_SUFFIX; + } else { + String cleanerType = CLEANER.getClass().getName(); + String fmt = + "Mockito could not find the first non-Mockito stack frame. A custom stack frame cleaner \n" + + "(type %s) is in use and this has mostly likely filtered out all the relevant stack frames."; + return String.format(fmt, cleanerType); + } + } + + /** + * In order to trigger the stack walker, we create some reflective frames. These need to be skipped so as to + * ensure there are no non-Mockito frames at the top of the stack trace. + */ + private static int framesToSkip() { + return stackWalk( + stream -> { + List metadata = + stream.map(toStackFrameMetadata) + .map(StackFrameMetadata::getClassName) + .collect(Collectors.toList()); + return metadata.indexOf(Java9PlusLocationImpl.class.getName()); + }); + } + + @SuppressWarnings("unchecked") + private static T stackWalk(Function, T> function) { + try { + return (T) walk.invoke(stackWalker, function); + } catch (IllegalAccessException e) { + throw new MockitoException( + "Unexpected access exception while stack walking." + UNEXPECTED_ERROR_SUFFIX, + e); + } catch (InvocationTargetException e) { + throw new MockitoException(stackWalkFailureMessage()); + } + } + + private static String stackWalkFailureMessage() { + if (usingDefaultStackTraceCleaner()) { + return "Caught an unexpected exception while stack walking." + UNEXPECTED_ERROR_SUFFIX; + } else { + String className = CLEANER.getClass().getName(); + String fmt = + "Caught an unexpected exception while stack walking." + + "\nThis is likely caused by the custom stack trace cleaner in use (class %s)."; + return String.format(fmt, className); + } + } + + private static Method walk() { + try { + return stackWalkerClazz.getMethod("walk", Function.class); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + + private static Class clazz(String name) { + try { + return Class.forName(name); + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private static Object stackWalker() { + try { + Set options = + Collections.singleton(Enum.valueOf((Class) optionClazz, SHOW_REFLECT_FRAMES)); + Method getInstance = + stackWalkerClazz.getDeclaredMethod("getInstance", Set.class, int.class); + return getInstance.invoke(null, options, BUFFER_SIZE); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { + throw new MockitoException( + "Mockito received an exception while trying to acquire a StackWalker." + + UNEXPECTED_ERROR_SUFFIX); + } + } + + private static final class MetadataShim implements StackFrameMetadata, Serializable { + private static final long serialVersionUID = 8491903719411428648L; + private static final Method getClassName = getter("getClassName"); + private static final Method getMethodName = getter("getMethodName"); + private static final Method getFileName = getter("getFileName"); + private static final Method getLineNumber = getter("getLineNumber"); + private static final Method toString = getter(Object.class, "toString"); + + private final Object stackFrame; + + private MetadataShim(Object stackFrame) { + this.stackFrame = stackFrame; + } + + @Override + public String getClassName() { + return (String) get(getClassName); + } + + @Override + public String getMethodName() { + return (String) get(getMethodName); + } + + @Override + public String getFileName() { + return (String) get(getFileName); + } + + @Override + public int getLineNumber() { + return (int) get(getLineNumber); + } + + @Override + public String toString() { + return (String) get(toString); + } + + /** + * Ensure that this type remains serializable. + */ + private Object writeReplace() { + return new SerializableShim(toStackTraceElement()); + } + + private StackTraceElement toStackTraceElement() { + try { + Method method = stackFrameClazz.getMethod("toStackTraceElement"); + return (StackTraceElement) method.invoke(stackFrame); + } catch (InvocationTargetException | NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private Object get(Method handle) { + try { + return handle.invoke(stackFrame); + } catch (InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static Method getter(String name) { + return getter(stackFrameClazz, name); + } + + private static Method getter(Class clazz, String name) { + try { + return clazz.getDeclaredMethod(name); + } catch (Throwable t) { + throw new RuntimeException(t); + } + } + } + + private static final class SerializableShim implements StackFrameMetadata, Serializable { + private static final long serialVersionUID = 7908320459080898690L; + private final StackTraceElement ste; + + private SerializableShim(StackTraceElement ste) { + this.ste = ste; + } + + @Override + public String getClassName() { + return ste.getClassName(); + } + + @Override + public String getMethodName() { + return ste.getMethodName(); + } + + @Override + public String getFileName() { + return ste.getFileName(); + } + + @Override + public int getLineNumber() { + return ste.getLineNumber(); + } + } +} diff --git a/src/main/java/org/mockito/internal/debugging/Localized.java b/src/main/java/org/mockito/internal/debugging/Localized.java index d1d7912dc4..3abcf29555 100644 --- a/src/main/java/org/mockito/internal/debugging/Localized.java +++ b/src/main/java/org/mockito/internal/debugging/Localized.java @@ -13,7 +13,7 @@ public class Localized { public Localized(T object) { this.object = object; - location = new LocationImpl(); + location = LocationFactory.create(); } public T getObject() { diff --git a/src/main/java/org/mockito/internal/debugging/LocationFactory.java b/src/main/java/org/mockito/internal/debugging/LocationFactory.java new file mode 100644 index 0000000000..daafddedaf --- /dev/null +++ b/src/main/java/org/mockito/internal/debugging/LocationFactory.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2007 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.internal.debugging; + +import org.mockito.invocation.Location; + +public final class LocationFactory { + private static final Factory factory = createLocationFactory(); + + private LocationFactory() {} + + public static Location create() { + return create(false); + } + + public static Location create(boolean inline) { + return factory.create(inline); + } + + private interface Factory { + Location create(boolean inline); + } + + private static Factory createLocationFactory() { + try { + Class.forName("java.lang.StackWalker"); + return new Java9PlusLocationFactory(); + } catch (ClassNotFoundException e) { + return new Java8LocationFactory(); + } + } + + private static final class Java8LocationFactory implements Factory { + @Override + public Location create(boolean inline) { + return new Java8LocationImpl(new Throwable(), inline); + } + } + + private static final class Java9PlusLocationFactory implements Factory { + + @Override + public Location create(boolean inline) { + return new Java9PlusLocationImpl(inline); + } + } +} diff --git a/src/main/java/org/mockito/internal/exceptions/Reporter.java b/src/main/java/org/mockito/internal/exceptions/Reporter.java index ea670407e7..9a5ea90d21 100644 --- a/src/main/java/org/mockito/internal/exceptions/Reporter.java +++ b/src/main/java/org/mockito/internal/exceptions/Reporter.java @@ -23,7 +23,7 @@ import org.mockito.exceptions.verification.TooManyActualInvocations; import org.mockito.exceptions.verification.VerificationInOrderFailure; import org.mockito.exceptions.verification.WantedButNotInvoked; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.exceptions.util.ScenarioPrinter; import org.mockito.internal.junit.ExceptionFactory; import org.mockito.internal.matchers.LocalizedMatcher; @@ -84,7 +84,7 @@ public static MockitoException incorrectUseOfApi() { return new MockitoException( join( "Incorrect use of API detected here:", - new LocationImpl(), + LocationFactory.create(), "", "You probably stored a reference to OngoingStubbing returned by when() and called stubbing methods like thenReturn() on this reference more than once.", "Examples of correct usage:", @@ -262,7 +262,7 @@ public static MockitoException incorrectUseOfAdditionalMatchers( "Invalid use of argument matchers inside additional matcher " + additionalMatcherName + " !", - new LocationImpl(), + LocationFactory.create(), "", expectedSubMatchersCount + " sub matchers expected, " @@ -295,7 +295,7 @@ public static MockitoException reportNoSubMatchersFound(String additionalMatcher return new InvalidUseOfMatchersException( join( "No matchers found for additional matcher " + additionalMatcherName, - new LocationImpl(), + LocationFactory.create(), "")); } @@ -322,7 +322,7 @@ public static AssertionError argumentsAreDifferent( .append("Argument(s) are different! Wanted:\n") .append(wanted) .append("\n") - .append(new LocationImpl()) + .append(LocationFactory.create()) .append("\n") .append("Actual invocations have different arguments:\n"); @@ -365,7 +365,7 @@ public static MockitoAssertionError wantedButNotInvoked( } private static String createWantedButNotInvokedMessage(DescribedInvocation wanted) { - return join("Wanted but not invoked:", wanted.toString(), new LocationImpl(), ""); + return join("Wanted but not invoked:", wanted.toString(), LocationFactory.create(), ""); } public static MockitoAssertionError wantedButNotInvokedInOrder( @@ -375,7 +375,7 @@ public static MockitoAssertionError wantedButNotInvokedInOrder( "Verification in order failure", "Wanted but not invoked:", wanted.toString(), - new LocationImpl(), + LocationFactory.create(), "Wanted anywhere AFTER following interaction:", previous.toString(), previous.getLocation(), @@ -400,7 +400,7 @@ private static String createTooManyInvocationsMessage( return join( wanted.toString(), "Wanted " + pluralize(wantedCount) + ":", - new LocationImpl(), + LocationFactory.create(), "But was " + pluralize(actualCount) + ":", createAllLocationsMessage(invocations), ""); @@ -413,7 +413,7 @@ public static MockitoAssertionError neverWantedButInvoked( join( wanted.toString(), "Never wanted here:", - new LocationImpl(), + LocationFactory.create(), "But invoked here:", createAllLocationsArgsMessage(invocations))); } @@ -463,7 +463,7 @@ private static String createTooFewInvocationsMessage( "Wanted " + discrepancy.getPluralizedWantedCount() + (discrepancy.getWantedCount() == 0 ? "." : ":"), - new LocationImpl(), + LocationFactory.create(), "But was " + discrepancy.getPluralizedActualCount() + (discrepancy.getActualCount() == 0 ? "." : ":"), @@ -496,7 +496,7 @@ public static MockitoAssertionError noMoreInteractionsWanted( return new NoInteractionsWanted( join( "No interactions wanted here:", - new LocationImpl(), + LocationFactory.create(), "But found this interaction on mock '" + MockUtil.getMockName(undesired.getMock()) + "':", @@ -508,7 +508,7 @@ public static MockitoAssertionError noMoreInteractionsWantedInOrder(Invocation u return new VerificationInOrderFailure( join( "No interactions wanted here:", - new LocationImpl(), + LocationFactory.create(), "But found this interaction on mock '" + MockUtil.getMockName(undesired.getMock()) + "':", @@ -527,7 +527,7 @@ public static MockitoAssertionError noInteractionsWanted( return new NoInteractionsWanted( join( "No interactions wanted here:", - new LocationImpl(), + LocationFactory.create(), "But found these interactions on mock '" + MockUtil.getMockName(mock) + "':", @@ -645,7 +645,7 @@ public static MockitoException smartNullPointerException(String invocation, Loca return new SmartNullPointerException( join( "You have a NullPointerException here:", - new LocationImpl(), + LocationFactory.create(), "because this method call was *not* stubbed correctly:", location, invocation, diff --git a/src/main/java/org/mockito/internal/exceptions/stacktrace/DefaultStackTraceCleaner.java b/src/main/java/org/mockito/internal/exceptions/stacktrace/DefaultStackTraceCleaner.java index 6b0575273e..6437ae4aa4 100644 --- a/src/main/java/org/mockito/internal/exceptions/stacktrace/DefaultStackTraceCleaner.java +++ b/src/main/java/org/mockito/internal/exceptions/stacktrace/DefaultStackTraceCleaner.java @@ -13,9 +13,18 @@ public class DefaultStackTraceCleaner implements StackTraceCleaner { @Override public boolean isIn(StackTraceElement e) { - if (isFromMockitoRunner(e.getClassName()) || isFromMockitoRule(e.getClassName())) { + return isIn(e.getClassName()); + } + + @Override + public boolean isIn(StackFrameMetadata e) { + return isIn(e.getClassName()); + } + + private boolean isIn(String className) { + if (isFromMockitoRunner(className) || isFromMockitoRule(className)) { return true; - } else if (isMockDispatcher(e.getClassName()) || isFromMockito(e.getClassName())) { + } else if (isMockDispatcher(className) || isFromMockito(className)) { return false; } else { return true; diff --git a/src/main/java/org/mockito/internal/invocation/DefaultInvocationFactory.java b/src/main/java/org/mockito/internal/invocation/DefaultInvocationFactory.java index 81f801518f..4921f4006d 100644 --- a/src/main/java/org/mockito/internal/invocation/DefaultInvocationFactory.java +++ b/src/main/java/org/mockito/internal/invocation/DefaultInvocationFactory.java @@ -8,7 +8,7 @@ import java.util.concurrent.Callable; import org.mockito.internal.creation.DelegatingMethod; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.mockref.MockWeakReference; import org.mockito.internal.progress.SequenceNumber; import org.mockito.invocation.Invocation; @@ -71,7 +71,7 @@ private static InterceptedInvocation createInvocation( RealMethod realMethod, MockCreationSettings settings) { return createInvocation( - mock, invokedMethod, arguments, realMethod, settings, new LocationImpl()); + mock, invokedMethod, arguments, realMethod, settings, LocationFactory.create()); } private static MockitoMethod createMockitoMethod(Method method, MockCreationSettings settings) { diff --git a/src/main/java/org/mockito/internal/matchers/LocalizedMatcher.java b/src/main/java/org/mockito/internal/matchers/LocalizedMatcher.java index 00b47de7a9..a54eb6d79e 100644 --- a/src/main/java/org/mockito/internal/matchers/LocalizedMatcher.java +++ b/src/main/java/org/mockito/internal/matchers/LocalizedMatcher.java @@ -5,7 +5,7 @@ package org.mockito.internal.matchers; import org.mockito.ArgumentMatcher; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.invocation.Location; @SuppressWarnings("unchecked") @@ -16,7 +16,7 @@ public class LocalizedMatcher { public LocalizedMatcher(ArgumentMatcher matcher) { this.matcher = matcher; - this.location = new LocationImpl(); + this.location = LocationFactory.create(); } public Location getLocation() { diff --git a/src/main/java/org/mockito/internal/progress/MockingProgressImpl.java b/src/main/java/org/mockito/internal/progress/MockingProgressImpl.java index 991b5e4734..2585d32cfc 100644 --- a/src/main/java/org/mockito/internal/progress/MockingProgressImpl.java +++ b/src/main/java/org/mockito/internal/progress/MockingProgressImpl.java @@ -14,7 +14,7 @@ import org.mockito.internal.configuration.GlobalConfiguration; import org.mockito.internal.debugging.Localized; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.exceptions.Reporter; import org.mockito.internal.listeners.AutoCleanableListener; import org.mockito.invocation.Location; @@ -105,7 +105,7 @@ public VerificationMode pullVerificationMode() { @Override public void stubbingStarted() { validateState(); - stubbingInProgress = new LocationImpl(); + stubbingInProgress = LocationFactory.create(); } @Override diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java index 2402f364a6..bcca2ef52a 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java @@ -10,7 +10,7 @@ import java.io.Serializable; import org.mockito.Mockito; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.invocation.InvocationOnMock; import org.mockito.invocation.Location; import org.mockito.stubbing.Answer; @@ -57,7 +57,8 @@ public Object apply(Class type) { } return Mockito.mock( - type, new ThrowsSmartNullPointer(invocation, new LocationImpl())); + type, + new ThrowsSmartNullPointer(invocation, LocationFactory.create())); } }); } diff --git a/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java b/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java index 9900db9d98..d5cf3e5577 100644 --- a/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java +++ b/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java @@ -13,7 +13,7 @@ import java.util.List; import org.mockito.Mockito; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.mockref.MockReference; import org.mockito.internal.invocation.mockref.MockStrongReference; import org.mockito.invocation.Invocation; @@ -72,7 +72,7 @@ public Invocation toInvocation() { new SerializableMethod(method), args, NO_OP, - location == null ? new LocationImpl() : location, + location == null ? LocationFactory.create() : location, 1); if (verified) { i.markVerified(); diff --git a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNullsTest.java b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNullsTest.java index 984e07da2a..267c6d6966 100644 --- a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNullsTest.java +++ b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNullsTest.java @@ -20,7 +20,7 @@ import org.assertj.core.api.ThrowableAssert; import org.junit.Test; import org.mockito.exceptions.verification.SmartNullPointerException; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.InterceptedInvocation; import org.mockito.internal.invocation.SerializableMethod; import org.mockito.internal.invocation.mockref.MockStrongReference; @@ -146,7 +146,7 @@ private static InterceptedInvocation invocationMethodWithArgs(final T obj) GenericFooBar.class.getMethod("methodWithArgs", int.class, Object.class)), new Object[] {1, obj}, InterceptedInvocation.NO_OP, - new LocationImpl(), + LocationFactory.create(), 1); } @@ -269,7 +269,7 @@ private static InterceptedInvocation invocationMethodWithVarArgs(final T[] o "methodWithVarArgs", int.class, Object[].class)), new Object[] {1, obj}, InterceptedInvocation.NO_OP, - new LocationImpl(), + LocationFactory.create(), 1); } diff --git a/src/test/java/org/mockitointegration/ClassLoadabilityChecker.java b/src/test/java/org/mockitointegration/ClassLoadabilityChecker.java new file mode 100644 index 0000000000..d19c51fb74 --- /dev/null +++ b/src/test/java/org/mockitointegration/ClassLoadabilityChecker.java @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2017 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockitointegration; + +import java.util.HashSet; +import java.util.Set; + +/** + * Check that classes can be loaded and initialized on a provided classloader. Used + * for checking that Mockito has no dependency on libraries like JUnit. + *

+ * Some classes are excluded from this checking - namely, classes that fail due to + * the absence of Java classes. It's assumed that this is due to a specific optional + * dependency on APIs available in certain Java versions and so other elements of the + * test Matrix will check that those classes do not depend on JUnit or ByteBuddy. We + * exclude based on the failure of a ClassNotFoundException, or a NoClassDefFoundError + * caused by the failing to load of a failing parent class. + */ +public final class ClassLoadabilityChecker { + private static final boolean INITIALIZE_CLASSES = true; + private final Set excludedClasses = new HashSet<>(); + private final ClassLoader classLoader; + private final String purpose; + + public ClassLoadabilityChecker(ClassLoader classLoader, String purpose) { + this.classLoader = classLoader; + this.purpose = purpose; + } + + public void checkLoadability(String className) { + try { + Class.forName(className, INITIALIZE_CLASSES, classLoader); + } catch (ClassNotFoundException | LinkageError e) { + if (isFailureExcluded(className, e)) { + return; + } + e.printStackTrace(); + throw new AssertionError( + String.format("'%s' has some dependency to %s", className, purpose)); + } + } + + private boolean isFailureExcluded(String loadedClass, Throwable thrown) { + if (thrown == null) { + return false; + } + if (thrown instanceof ClassNotFoundException) { + ClassNotFoundException cnf = (ClassNotFoundException) thrown; + if (cnf.getMessage().startsWith("java.")) { + excludedClasses.add(loadedClass); + return true; + } + } else if (thrown instanceof NoClassDefFoundError) { + NoClassDefFoundError ncdf = (NoClassDefFoundError) thrown; + // if Foo fails due to depending on a Java class, Foo$Bar will fail with a NCDFE + int lastInnerClass = loadedClass.lastIndexOf('$'); + if (lastInnerClass != -1) { + String parent = loadedClass.substring(0, lastInnerClass); + if (excludedClasses.contains(parent) && ncdf.getMessage().contains(parent)) { + excludedClasses.add(loadedClass); + return true; + } + } + } + + return isFailureExcluded(loadedClass, thrown.getCause()); + } +} diff --git a/src/test/java/org/mockitointegration/NoByteCodeDependenciesTest.java b/src/test/java/org/mockitointegration/NoByteCodeDependenciesTest.java index 3a2908dc1c..3db1395066 100644 --- a/src/test/java/org/mockitointegration/NoByteCodeDependenciesTest.java +++ b/src/test/java/org/mockitointegration/NoByteCodeDependenciesTest.java @@ -43,21 +43,11 @@ public void pure_mockito_should_not_depend_bytecode_libraries() throws Exception pureMockitoAPIClasses.remove( "org.mockito.internal.util.reflection.InstrumentationMemberAccessor"); + ClassLoadabilityChecker checker = + new ClassLoadabilityChecker( + classLoader_without_bytecode_libraries, "ByteBuddy or Objenesis"); for (String pureMockitoAPIClass : pureMockitoAPIClasses) { - checkDependency(classLoader_without_bytecode_libraries, pureMockitoAPIClass); - } - } - - private void checkDependency(ClassLoader classLoader, String pureMockitoAPIClass) - throws ClassNotFoundException { - try { - Class.forName(pureMockitoAPIClass, true, classLoader); - } catch (Throwable e) { - e.printStackTrace(); - throw new AssertionError( - String.format( - "'%s' has some dependency to Byte Buddy or Objenesis", - pureMockitoAPIClass)); + checker.checkLoadability(pureMockitoAPIClass); } } } diff --git a/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java b/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java index 7b156f0aa3..503d859617 100644 --- a/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java +++ b/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java @@ -42,27 +42,18 @@ public void pure_mockito_should_not_depend_JUnit___ByteBuddy() throws Exception .omit("runners", "junit", "JUnit", "opentest4j") .listOwnedClasses(); + ClassLoadabilityChecker checker = + new ClassLoadabilityChecker(classLoader_without_JUnit, "JUnit"); + // The later class is required to be initialized before any inline mock maker classes can be // loaded. - checkDependency( - classLoader_without_JUnit, + checker.checkLoadability( "org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker"); pureMockitoAPIClasses.remove( "org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker"); for (String pureMockitoAPIClass : pureMockitoAPIClasses) { - checkDependency(classLoader_without_JUnit, pureMockitoAPIClass); - } - } - - private void checkDependency(ClassLoader classLoader_without_JUnit, String pureMockitoAPIClass) - throws ClassNotFoundException { - try { - Class.forName(pureMockitoAPIClass, true, classLoader_without_JUnit); - } catch (Throwable e) { - e.printStackTrace(); - throw new AssertionError( - String.format("'%s' has some dependency to JUnit", pureMockitoAPIClass)); + checker.checkLoadability(pureMockitoAPIClass); } } } diff --git a/src/test/java/org/mockitousage/internal/debugging/LocationFactoryTest.java b/src/test/java/org/mockitousage/internal/debugging/LocationFactoryTest.java new file mode 100644 index 0000000000..072c13fc02 --- /dev/null +++ b/src/test/java/org/mockitousage/internal/debugging/LocationFactoryTest.java @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2007 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockitousage.internal.debugging; + +import org.junit.Test; +import org.mockito.internal.debugging.LocationFactory; +import org.mockitoutil.TestBase; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; + +public class LocationFactoryTest extends TestBase { + + @Test + public void shouldLocationNotContainGetStackTraceMethod() { + assertThat(LocationFactory.create().toString()) + .contains("shouldLocationNotContainGetStackTraceMethod"); + } + + @Test + public void provides_location_class() { + // when + final List files = new ArrayList(); + new Runnable() { // anonymous inner class adds stress to the check + public void run() { + files.add(LocationFactory.create().getSourceFile()); + } + }.run(); + + // then + assertEquals("LocationFactoryTest.java", files.get(0)); + } +} diff --git a/src/test/java/org/mockitousage/internal/debugging/LocationImplTest.java b/src/test/java/org/mockitousage/internal/debugging/LocationImplTest.java deleted file mode 100644 index d20bfce02a..0000000000 --- a/src/test/java/org/mockitousage/internal/debugging/LocationImplTest.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright (c) 2007 Mockito contributors - * This program is made available under the terms of the MIT License. - */ -package org.mockitousage.internal.debugging; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertEquals; - -import java.util.ArrayList; -import java.util.List; - -import org.junit.Test; -import org.mockito.internal.debugging.LocationImpl; -import org.mockito.internal.exceptions.stacktrace.StackTraceFilter; -import org.mockitoutil.TestBase; - -@SuppressWarnings("serial") -public class LocationImplTest extends TestBase { - - @Test - public void shouldLocationNotContainGetStackTraceMethod() { - assertThat(new LocationImpl().toString()) - .contains("shouldLocationNotContainGetStackTraceMethod"); - } - - @Test - public void shouldBeSafeInCaseForSomeReasonFilteredStackTraceIsEmpty() { - // given - StackTraceFilter filterReturningEmptyArray = - new StackTraceFilter() { - @Override - public StackTraceElement[] filter(StackTraceElement[] target, boolean keepTop) { - return new StackTraceElement[0]; - } - - @Override - public StackTraceElement filterFirst(Throwable target, boolean isInline) { - return null; - } - }; - - // when - String loc = new LocationImpl(filterReturningEmptyArray).toString(); - - // then - assertEquals("-> at <>", loc); - } - - @Test - public void provides_location_class() { - // when - final List files = new ArrayList(); - new Runnable() { // anonymous inner class adds stress to the check - public void run() { - files.add(new LocationImpl().getSourceFile()); - } - }.run(); - - // then - assertEquals("LocationImplTest.java", files.get(0)); - } -} diff --git a/src/test/java/org/mockitoutil/TestBase.java b/src/test/java/org/mockitoutil/TestBase.java index 2fe89504e4..3f772d020b 100644 --- a/src/test/java/org/mockitoutil/TestBase.java +++ b/src/test/java/org/mockitoutil/TestBase.java @@ -17,7 +17,7 @@ import org.mockito.StateMaster; import org.mockito.internal.MockitoCore; import org.mockito.internal.configuration.ConfigurationAccess; -import org.mockito.internal.debugging.LocationImpl; +import org.mockito.internal.debugging.LocationFactory; import org.mockito.internal.invocation.InterceptedInvocation; import org.mockito.internal.invocation.InvocationBuilder; import org.mockito.internal.invocation.InvocationMatcher; @@ -84,7 +84,7 @@ protected static Invocation invocationOf(Class type, String methodName, Objec new SerializableMethod(type.getMethod(methodName, types)), args, InterceptedInvocation.NO_OP, - new LocationImpl(), + LocationFactory.create(), 1); } diff --git a/subprojects/memory-test/src/test/java/org/mockito/memorytest/LocationFactoryAllocationRateTest.java b/subprojects/memory-test/src/test/java/org/mockito/memorytest/LocationFactoryAllocationRateTest.java new file mode 100644 index 0000000000..26b36ca9af --- /dev/null +++ b/subprojects/memory-test/src/test/java/org/mockito/memorytest/LocationFactoryAllocationRateTest.java @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2022 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.memorytest; + +import com.sun.management.ThreadMXBean; +import org.junit.Test; +import org.mockito.internal.debugging.LocationFactory; + +import java.lang.management.ManagementFactory; +import java.util.stream.IntStream; + +import static org.assertj.core.api.Assertions.assertThat; + +public class LocationFactoryAllocationRateTest { + private static final int REPEAT = 1000; + private static final int RECURSION_LIMIT = 1000; + private static final double EXPECTED_IMPROVEMENT = expectedImprovement(); + + private static final ThreadMXBean memoryBean = + (ThreadMXBean) ManagementFactory.getThreadMXBean(); + + + @Test + public void shouldAllocateMuchLessMemoryThanThrowable() { + // On Java 8, this will use the internal approach. On Java 9, the StackWalker approach will + // be used. + new Throwable().fillInStackTrace(); + LocationFactory.create(); + long baseline = + countMemoryAllocations( + () -> + recurseAndThen( + RECURSION_LIMIT, + repeat(() -> new Throwable().fillInStackTrace()))); + long actual = + countMemoryAllocations( + () -> + recurseAndThen( + RECURSION_LIMIT, + repeat(() -> LocationFactory.create(false)))); + assertThat(actual * EXPECTED_IMPROVEMENT) + .as( + "stack walker approach (%d) expected to be at least %fx better than exception approach (%d)", + actual, EXPECTED_IMPROVEMENT, baseline) + .isLessThan(baseline); + } + + private static long countMemoryAllocations(Runnable someTask) { + long threadId = Thread.currentThread().getId(); + long atStart = memoryBean.getThreadAllocatedBytes(threadId); + someTask.run(); + return memoryBean.getThreadAllocatedBytes(threadId) - atStart; + } + + private static void recurseAndThen(int count, Runnable runnable) { + if (count <= 0) { + runnable.run(); + } else { + recurseAndThen(count - 1, runnable); + } + } + + private static Runnable repeat(Runnable task) { + return () -> IntStream.range(0, REPEAT).forEach(index -> task.run()); + } + + private static double expectedImprovement() { + try { + Class.forName("java.lang.StackWalker"); + return 20; + } catch (ClassNotFoundException e) { + return 1.5; + } + } +} diff --git a/subprojects/memory-test/src/test/java/org/mockito/memorytest/ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest.java b/subprojects/memory-test/src/test/java/org/mockito/memorytest/ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest.java index d3a6903e21..5c8bac2630 100644 --- a/subprojects/memory-test/src/test/java/org/mockito/memorytest/ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest.java +++ b/subprojects/memory-test/src/test/java/org/mockito/memorytest/ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest.java @@ -10,10 +10,8 @@ import static org.mockito.Mockito.when; import org.junit.Assume; -import org.junit.Ignore; import org.junit.Test; -@Ignore("https://github.com/mockito/mockito/issues/2478") public class ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest { private static final int STACK_TRACE_DEPTH = 1000; @@ -23,12 +21,7 @@ public class ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest { static { try { - Class.forName("sun.misc.SharedSecrets") - .getMethod("getJavaLangAccess") - .invoke(null); - Class.forName("sun.misc.JavaLangAccess") - .getMethod("getStackTraceElement", Throwable.class, int.class); - + Class.forName("java.lang.StackWalker"); supported = true; } catch (Exception ignored) { }