Repository: mockito/mockito
Issue: Rename MockMethodDispatcher.class to MockMethodDispatcher.raw on build
Labels: android
final-class-or-methods

The `org.mockito.internal.creation.bytebuddy.MockMethodDispatcher.class` file should be renamed to `org.mockito.internal.creation.bytebuddy.MockMethodDispatcher.raw` on building Mockito. This way, class loaders are no longer capable of loading the class directly what should never happen. Doing so, the bootstrap injection mechanism cannot be superseeded by child-first class loaders which break mockability due to the dispatcher class being loaded twice by two different class loaders.

This problem was observed with the Robolectrics class loader.

Additionally, the following change is required in the `InlineByteBuddyMockMaker` when creating the boot jar

```java
String source = "org/mockito/internal/creation/bytebuddy/MockMethodDispatcher";
outputStream.putNextEntry(new JarEntry(source + ".class"));
InputStream inputStream = InlineByteBuddyMockMaker.class.getClassLoader().getResourceAsStream(source + ".raw");
if (inputStream == null) {
  throw new MockitoInitializationException(join(
    "The MockMethodDispatcher class file could not be located " + source,
    "",
    "The class loader responsible: " + InlineByteBuddyMockMaker.class.getClassLoader()
  ));
}
try {
  int length;
  byte[] buffer = new byte[1024];
  while ((length = inputStream.read(buffer)) != -1) {
    outputStream.write(buffer, 0, length);
  }
} finally {
  inputStream.close();
}
outputStream.closeEntry();
```

Issue comments:
If the renaming is only needed when creating a distribution jar, you can customize the jar task with something like this:

`jar {
  rename "org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.class", "org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.raw"
}`

If you need to rename this file after compilation, you could use the `doLast` section of the `classes` task:

`doLast {
  file("${buildDir}/main/org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.class").renameTo("${buildDir}/main/org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.raw")
}`
I discovered the Robolectric ClassLoader issue as well while updating the Mockito mockmaker to ByteBuddy internally. My fix now has the correct class hierarchy, but it now fails to properly resolve the `MockMethodDispatcher`. Should we revert this change (and figure out a way to work with both the broken ClassLoader and the fixed version)?
For now, I have patched the Robolectric ClassLoader to special case the class:

```java
String extension = className.equals("org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher") ? "raw" : "class";
    String classFilename = className.replace('.', '/') + "." + extension;
```
However, this introduces a version complexity matrix. E.g.:

broken Robolectric works with current Mockito
fixed Robolectric does not work with current Mockito
fixed Robolectric + workaround works with current Mockito

How can we remove the workaround in Robolectric while at the same time remove the workaround in Mockito while adhering to SemVer in BOTH projects?
Current thinking is (on Mockito build time) keep the `.class` and copying to `.raw`. Then we can update Mockito to just use `.class` and old Robolectric ignores `.raw`, while the new Robolectric uses the original `.class`.

Pull request review comments:
In order for this to work reliably, we need to also set one more task dependency on compileJava. Let me try to update the code in the branch...
Above change makes the compiled classes directory contain both: the 'raw' and normal class. Is this what we want?
Yes, for the time until packaging the jar. This is required to make IDEs work with Mockito which do otherwise claim that there is a class missing (experienced with IntellIJ). It does not hurt us as we know that our unit tests are run by a parent-first class loader where class loading works as expected. This is only an issue when dealing with parent-first loaders where the `.class` file must not exist.

Patch that resolved the issue:
diff --git a/build.gradle b/build.gradle
index fdd11d9b4a..08d1c3d37b 100644
--- a/build.gradle
+++ b/build.gradle
@@ -23,7 +23,7 @@ ext {
     artifactName = 'mockito-core'
     bintrayAutoPublish = true
     bintrayRepo = 'maven'
-    mavenCentralSync = false
+    mavenCentralSync = true
 }
 
 allprojects {
@@ -69,6 +69,7 @@ apply from: 'gradle/root/coverage.gradle'
 apply from: 'gradle/root/gradle-fix.gradle'
 apply from: 'gradle/root/release.gradle'
 
+apply from: 'gradle/mockito-core/inline-mock.gradle'
 apply from: 'gradle/mockito-core/osgi.gradle'
 apply from: 'gradle/mockito-core/javadoc.gradle'
 apply from: 'gradle/mockito-core/license.gradle'
diff --git a/gradle/mockito-core/inline-mock.gradle b/gradle/mockito-core/inline-mock.gradle
new file mode 100644
index 0000000000..b67300a26d
--- /dev/null
+++ b/gradle/mockito-core/inline-mock.gradle
@@ -0,0 +1,14 @@
+task copyMockMethodDispatcher(type: Copy) {
+    dependsOn compileJava
+    from "${buildDir}/classes/main/org/mockito/internal/creation/bytebuddy"
+    into "${buildDir}/classes/main/org/mockito/internal/creation/bytebuddy"
+    include "MockMethodDispatcher.class"
+    rename { String fileName ->
+        fileName.replace('.class', '.raw')
+    }
+}
+classes.dependsOn(copyMockMethodDispatcher)
+
+jar {
+    exclude("org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.class")
+}
diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
index c747e94605..882d12b768 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
@@ -5,7 +5,6 @@
 package org.mockito.internal.creation.bytebuddy;
 
 import net.bytebuddy.agent.ByteBuddyAgent;
-import net.bytebuddy.dynamic.ClassFileLocator;
 import org.mockito.Incubating;
 import org.mockito.exceptions.base.MockitoException;
 import org.mockito.exceptions.base.MockitoInitializationException;
@@ -20,6 +19,7 @@
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Modifier;
 import java.util.jar.JarEntry;
@@ -90,59 +90,91 @@
 @Incubating
 public class InlineByteBuddyMockMaker implements ClassCreatingMockMaker {
 
-    private final Instrumentation instrumentation;
+    private static final Instrumentation INSTRUMENTATION;
 
-    private final BytecodeGenerator bytecodeGenerator;
-
-    private final WeakConcurrentMap<Object, MockMethodInterceptor> mocks = new WeakConcurrentMap.WithInlinedExpunction<Object, MockMethodInterceptor>();
+    private static final Throwable INITIALIZATION_ERROR;
 
-    public InlineByteBuddyMockMaker() {
+    static {
+        Instrumentation instrumentation;
+        Throwable initializationError = null;
         try {
-            instrumentation = ByteBuddyAgent.install();
-            if (!instrumentation.isRetransformClassesSupported()) {
-                throw new MockitoInitializationException(join(
-                        "Byte Buddy requires retransformation for creating inline mocks. This feature is unavailable on the current VM.",
-                        "",
-                        "You cannot use this mock maker on this VM:",
-                        Platform.describe()));
-            }
-            File boot = File.createTempFile("mockitoboot", "jar");
-            boot.deleteOnExit();
-            JarOutputStream outputStream = new JarOutputStream(new FileOutputStream(boot));
             try {
-                outputStream.putNextEntry(new JarEntry("org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.class"));
-                outputStream.write(ClassFileLocator.ForClassLoader.of(InlineByteBuddyMockMaker.class.getClassLoader())
-                        .locate("org.mockito.internal.creation.bytebuddy.MockMethodDispatcher")
-                        .resolve());
-                outputStream.closeEntry();
-            } finally {
-                outputStream.close();
-            }
-            instrumentation.appendToBootstrapClassLoaderSearch(new JarFile(boot));
-            try {
-                Class<?> dispatcher = Class.forName("org.mockito.internal.creation.bytebuddy.MockMethodDispatcher");
-                if (dispatcher.getClassLoader() != null) {
-                    throw new MockitoInitializationException(join(
-                            "The MockMethodDispatcher must not be loaded manually but must be injected into the bootstrap class loader.",
+                instrumentation = ByteBuddyAgent.install();
+                if (!instrumentation.isRetransformClassesSupported()) {
+                    throw new IllegalStateException(join(
+                            "Byte Buddy requires retransformation for creating inline mocks. This feature is unavailable on the current VM.",
+                            "",
+                            "You cannot use this mock maker on this VM"));
+                }
+                File boot = File.createTempFile("mockitoboot", ".jar");
+                boot.deleteOnExit();
+                JarOutputStream outputStream = new JarOutputStream(new FileOutputStream(boot));
+                try {
+                    String source = "org/mockito/internal/creation/bytebuddy/MockMethodDispatcher";
+                    InputStream inputStream = InlineByteBuddyMockMaker.class.getClassLoader().getResourceAsStream(source + ".raw");
+                    if (inputStream == null) {
+                        throw new IllegalStateException(join(
+                                "The MockMethodDispatcher class file is not locatable: " + source + ".raw",
+                                "",
+                                "The class loader responsible for looking up the resource: " + InlineByteBuddyMockMaker.class.getClassLoader()
+                        ));
+                    }
+                    outputStream.putNextEntry(new JarEntry(source + ".class"));
+                    try {
+                        int length;
+                        byte[] buffer = new byte[1024];
+                        while ((length = inputStream.read(buffer)) != -1) {
+                            outputStream.write(buffer, 0, length);
+                        }
+                    } finally {
+                        inputStream.close();
+                    }
+                    outputStream.closeEntry();
+                } finally {
+                    outputStream.close();
+                }
+                instrumentation.appendToBootstrapClassLoaderSearch(new JarFile(boot));
+                try {
+                    Class<?> dispatcher = Class.forName("org.mockito.internal.creation.bytebuddy.MockMethodDispatcher");
+                    if (dispatcher.getClassLoader() != null) {
+                        throw new IllegalStateException(join(
+                                "The MockMethodDispatcher must not be loaded manually but must be injected into the bootstrap class loader.",
+                                "",
+                                "The dispatcher class was already loaded by: " + dispatcher.getClassLoader()));
+                    }
+                } catch (ClassNotFoundException cnfe) {
+                    throw new IllegalStateException(join(
+                            "Mockito failed to inject the MockMethodDispatcher class into the bootstrap class loader",
                             "",
-                            "The dispatcher class was already loaded by: " + dispatcher.getClassLoader()));
+                            "It seems like your current VM does not support the instrumentation API correctly."), cnfe);
                 }
-            } catch (ClassNotFoundException cnfe) {
-                throw new MockitoInitializationException(join(
-                        "Mockito failed to inject the MockMethodDispatcher class into the bootstrap class loader",
+            } catch (IOException ioe) {
+                throw new IllegalStateException(join(
+                        "Mockito could not self-attach a Java agent to the current VM. This feature is required for inline mocking.",
+                        "This error occured due to an I/O error during the creation of this agent: " + ioe,
                         "",
-                        "It seems like your current VM does not support the instrumentation API correctly:",
-                        Platform.describe()), cnfe);
+                        "Potentially, the current VM does not support the instrumentation API correctly"), ioe);
             }
-            bytecodeGenerator = new TypeCachingBytecodeGenerator(new InlineBytecodeGenerator(instrumentation, mocks), true);
-        } catch (IOException ioe) {
+        } catch (Throwable throwable) {
+            instrumentation = null;
+            initializationError = throwable;
+        }
+        INSTRUMENTATION = instrumentation;
+        INITIALIZATION_ERROR = initializationError;
+    }
+
+    private final BytecodeGenerator bytecodeGenerator;
+
+    private final WeakConcurrentMap<Object, MockMethodInterceptor> mocks = new WeakConcurrentMap.WithInlinedExpunction<Object, MockMethodInterceptor>();
+
+    public InlineByteBuddyMockMaker() {
+        if (INITIALIZATION_ERROR != null) {
             throw new MockitoInitializationException(join(
-                    "Mockito could not self-attach a Java agent to the current VM. This feature is required for inline mocking.",
-                    "This error occured due to an I/O error during the creation of this agent: " + ioe,
+                    "Could not initialize inline Byte Buddy mock maker. (This mock maker is not supported on Android.)",
                     "",
-                    "Potentially, the current VM does not support the instrumentation API correctly:",
-                    Platform.describe()), ioe);
+                    Platform.describe()), INITIALIZATION_ERROR);
         }
+        bytecodeGenerator = new TypeCachingBytecodeGenerator(new InlineBytecodeGenerator(INSTRUMENTATION, mocks), true);
     }
 
     @Override
@@ -260,7 +292,7 @@ public TypeMockability isTypeMockable(final Class<?> type) {
         return new TypeMockability() {
             @Override
             public boolean mockable() {
-                return instrumentation.isModifiableClass(type) && !EXCLUDES.contains(type);
+                return INSTRUMENTATION.isModifiableClass(type) && !EXCLUDES.contains(type);
             }
 
             @Override
@@ -278,4 +310,24 @@ public String nonMockableReason() {
             }
         };
     }
+
+    static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
+        try {
+            StackTraceElement[] stack = throwable.getStackTrace();
+            int skip = 0;
+            StackTraceElement next;
+            do {
+                next = stack[stack.length - current - ++skip];
+            } while (!next.getClassName().equals(targetType.getName()));
+            int top = stack.length - current - skip;
+            StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
+            System.arraycopy(stack, 0, cleared, 0, top);
+            System.arraycopy(stack, top + skip, cleared, top, current);
+            throwable.setStackTrace(cleared);
+            return throwable;
+        } catch (RuntimeException ignored) {
+            // This should not happen unless someone instrumented or manipulated exception stack traces.
+            return throwable;
+        }
+    }
 }
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 7f6483fc7a..394bd2d2eb 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
@@ -23,6 +23,8 @@
 import java.lang.reflect.Modifier;
 import java.util.concurrent.Callable;
 
+import static org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.hideRecursiveCall;
+
 public class MockMethodAdvice extends MockMethodDispatcher {
 
     final WeakConcurrentMap<Object, MockMethodInterceptor> interceptors;
@@ -180,26 +182,6 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen
         }
     }
 
-    static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
-        try {
-            StackTraceElement[] stack = throwable.getStackTrace();
-            int skip = 0;
-            StackTraceElement next;
-            do {
-                next = stack[stack.length - current - ++skip];
-            } while (!next.getClassName().equals(targetType.getName()));
-            int top = stack.length - current - skip;
-            StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
-            System.arraycopy(stack, 0, cleared, 0, top);
-            System.arraycopy(stack, top + skip, cleared, top, current);
-            throwable.setStackTrace(cleared);
-            return throwable;
-        } catch (RuntimeException ignored) {
-            // This should not happen unless someone instrumented or manipulated exception stack traces.
-            return throwable;
-        }
-    }
-
     private static class ReturnValueWrapper implements Callable<Object> {
 
         private final Object returned;


Test patch:
diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
index 3b5256dae4..ee36f6de77 100644
--- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
+++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
@@ -1,10 +1,5 @@
 package org.mockito.internal.creation.bytebuddy;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.Observable;
-import java.util.Observer;
-import java.util.regex.Pattern;
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.ClassFileVersion;
 import net.bytebuddy.description.modifier.Visibility;
@@ -21,6 +16,12 @@
 import org.mockito.mock.SerializableMode;
 import org.mockito.plugins.MockMaker;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Observable;
+import java.util.Observer;
+import java.util.regex.Pattern;
+
 import static net.bytebuddy.ClassFileVersion.JAVA_V8;
 import static net.bytebuddy.ClassFileVersion.JAVA_V9;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -157,7 +158,7 @@ public void should_remove_recursive_self_call_from_stack_trace() throws Exceptio
 
         Throwable throwable = new Throwable();
         throwable.setStackTrace(stack);
-        throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class);
+        throwable = InlineByteBuddyMockMaker.hideRecursiveCall(throwable, 2, SampleInterface.class);
 
         assertThat(throwable.getStackTrace()).isEqualTo(new StackTraceElement[]{
                 new StackTraceElement("foo", "", "", -1),
@@ -170,7 +171,7 @@ public void should_remove_recursive_self_call_from_stack_trace() throws Exceptio
     public void should_handle_missing_or_inconsistent_stack_trace() throws Exception {
         Throwable throwable = new Throwable();
         throwable.setStackTrace(new StackTraceElement[0]);
-        assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class)).isSameAs(throwable);
+        assertThat(InlineByteBuddyMockMaker.hideRecursiveCall(throwable, 0, SampleInterface.class)).isSameAs(throwable);
     }
 
     @Test
@@ -263,6 +264,11 @@ private static <T> MockCreationSettings<T> settingsFor(Class<T> type, Class<?>..
         return mockSettings;
     }
 
+    @Test
+    public void testMockDispatcherIsRelocated() throws Exception {
+        assertThat(InlineByteBuddyMockMaker.class.getClassLoader().getResource("org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.raw")).isNotNull();
+    }
+
     private static final class FinalClass {
 
         public String foo() {
diff --git a/src/test/java/org/mockito/internal/junit/JUnitRuleTest.java b/src/test/java/org/mockito/internal/junit/JUnitRuleTest.java
index b7da8bb5f7..8ae318b576 100644
--- a/src/test/java/org/mockito/internal/junit/JUnitRuleTest.java
+++ b/src/test/java/org/mockito/internal/junit/JUnitRuleTest.java
@@ -44,4 +44,4 @@ public void does_not_check_invalid_mockito_usage_on_failure() throws Throwable {
         Mockito.when(mock.simpleMethod()); // <--- unfinished stubbing
         throw new RuntimeException("foo"); // <--- some failure
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java b/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java
index 59f766fed9..d39fa14290 100644
--- a/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java
+++ b/src/test/java/org/mockitointegration/NoJUnitDependenciesTest.java
@@ -1,6 +1,7 @@
 package org.mockitointegration;
 
 import net.bytebuddy.ByteBuddy;
+import net.bytebuddy.agent.ByteBuddyAgent;
 import org.hamcrest.Matcher;
 import org.junit.Assume;
 import org.junit.Test;
@@ -9,6 +10,7 @@
 import org.mockitoutil.ClassLoaders;
 import org.objenesis.Objenesis;
 
+import java.util.Arrays;
 import java.util.Set;
 
 import static org.mockitoutil.ClassLoaders.coverageTool;
@@ -24,6 +26,7 @@ public void pure_mockito_should_not_depend_JUnit___ByteBuddy() throws Exception
                         Mockito.class,
                         Matcher.class,
                         ByteBuddy.class,
+                        ByteBuddyAgent.class,
                         Objenesis.class
                 )
                 .withCodeSourceUrlOf(coverageTool())
@@ -32,6 +35,10 @@ public void pure_mockito_should_not_depend_JUnit___ByteBuddy() throws Exception
 
         Set<String> pureMockitoAPIClasses = ClassLoaders.in(classLoader_without_JUnit).omit("runners", "junit", "JUnit").listOwnedClasses();
 
+        // The later class is required to be initialized before any inline mock maker classes can be loaded.
+        checkDependency(classLoader_without_JUnit, "org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker");
+        pureMockitoAPIClasses.remove("org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker");
+
         for (String pureMockitoAPIClass : pureMockitoAPIClasses) {
             checkDependency(classLoader_without_JUnit, pureMockitoAPIClass);
         }