Repository: mockito/mockito
Issue: 'this' is not available - when enabling mock-maker-inline

When we enable `mock-maker-inline` in our project and debug a class with `@Spy` annotated on it IntelliJ reports that `'this' is not available`:
![image](https://user-images.githubusercontent.com/325602/97474278-dad7a180-1943-11eb-837a-f6f28a4917a4.png)

When we unset `mock-maker-inline` then things work as expected and we can inspect the class variables:
![image](https://user-images.githubusercontent.com/325602/97474389-f80c7000-1943-11eb-8fcd-32f360ae63ce.png)

It appears to be the same as this report: https://stackoverflow.com/questions/62661996/unable-to-debug-junit-test-anywhere

Is this expected Mockito behaviour?

Issue comments:
Thanks for filing the issue. Could you please provide us a minimal reproduction case that we can clone/download to debug the issue? Thanks in advance!
Facing a similar issue. It can be reproduced by following these steps: https://www.baeldung.com/mockito-final
and then debugging through the code. For any variable, in the Variables window, we can see the following
![image](https://user-images.githubusercontent.com/13188944/101822456-f3181000-3aee-11eb-9761-4e57679018d0.png)

I am able to recreate this in Eclipse with the following example
Eclipse: 2020-09 (4.17.0)
Java: jdk1.8.0_201
JUnit: 5.4.2
mockito: 3.6.0 (core buildtime inline testruntime)

When I try to step into the call at
assertEquals(1, myClass.doAThing());

I get: com.sun.jdi.InternalException: Got error code in reply:35 occurred retrieving 'this' from stack frame.		

[MockitoInlineTest.txt](https://github.com/mockito/mockito/files/5850429/MockitoInlineTest.txt)
[MyClass.txt](https://github.com/mockito/mockito/files/5850430/MyClass.txt)

Let me know if there is any additional data I can add to help get to the bottom of this.

Thanks!
John
Same issue with Mockito-inline 3.8.0 and java 15. This makes tests very difficult to debug, because it makes impossible to debug the code step by step 
Our team is having this issue as well with 3.9.0. It's a major show-stopper. Would be great if it'd be fixed <3
Rafael, does this mean we need to generate some more bytecode to make sure that `this` references are working as intended with the inline mockmaker?
I am afraid we are the wrong place to fix this. Byte Buddy needs to shift the local variable array to add Mockito's logic. For reasons (tm), this requires to move the *this* variable further to the left. It is however still there:

      LocalVariableTable:
        Start  Length  Slot  Name   Signature
           76       4     2  this   Lfoo/Sample;

IntelliJ could certainly pick this up and make the variable available in the debugger. Normally this is put at slot 0 but the JVM does not require this at all. I think IntelliJ falsely assumes this. If I rename the variable to `this2`, it is even displayed but that would be inconsistent for other parsing processors. This needs to be fixed in IntelliJ.
Thank you for the explanation! Does anybody want to help us out filing this bug for IntelliJ?
Would something similar need to be done to get this fixed in eclipse?

thank you for looking at it!
Possibly, I have not used Eclipse in a while. This might even relate to the debugger API. 
As per twitter conversation:

The JVM spec says that for instance methods, the "this" reference is sorted in slot 0 on a stack frame:

https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-2.html#jvms-2.6.1

"On instance method invocation, local variable 0 is always used to pass a reference to the object on which the instance method is being invoked (this in the Java programming language)."

This is hard coded into the Java debug protocol which is why IDEs like IntelliJ and Eclipse report specific issues with "this" but not other variables; there's a specific "thisObject" call in the wire protocol (as opposed to a local variable which happens to have the name "this"

https://github.com/openjdk/jdk/blob/master/src/jdk.jdwp.agent/share/native/libjdwp/StackFrameImpl.c#L378

https://docs.oracle.com/en/java/javase/11/docs/api/jdk.jdi/com/sun/jdi/StackFrame.html#thisObject
Why do IDEs not process local variables named 'this' though? It's the very same value, would not make a difference for the user and the verifier certainly considers the byte code legal.

It is certainly true that variables are passed on their respective index but they are commonly reassigned within the method what is what happens here. 
Because in Java, at least, you can't create a variable called "this" and in fact it's a synthetic construct generated by the java compiler as a shorthand for local[0].

Secondly the local variable table is optional which can be skipped when generating code using -g:none. In this circumstance it is necessary for the debugger to be able to resolve what the object instance is instead of looking up by name.

Thirdly it's what is defined in the debug api which is what IDEs use and which is baked into various assumptions and places within the JVM, including various JIT related operations. This may not be an issue in this case (particularly if it's not called frequently) but for performance reasons the calls to other instance methods will passthrough local[0] implicitly, regardless of what the local variable table has in it.

TL;DR "this" isn't a variable in the normal sense and changing the LVT isn't sufficient to make IDEa believe that it is.
'this' only exists as a Java keyword and you are feee to call it whatever in the language, you like, also in the LVT. The zero slot is in no way privileged. The verifier already tracks if the slot is reassigned to avoid super calls on other identities then the currently dispatched instance. At the same time, it allows such calls if 'this' is reassigned to another slot later from that targeted slot. 

If I am reading the spec correctly, this is also perfectly legal. It only defines on what slot 'this' is passed to a method. I do neither think such reassignment is affecting the JIT where 'this' is no longer a special variable once the verifier is satisfied. 

Furthermore, Byte Buddy retains the 'this' variable at slot 0. As a matter of fact, that's the point of it. It makes a defensive copy of the original array to function when obfuscators or alternative JVM languages are used which sometimes reassign slots that are no longer used in the code. Therefore, I am wondering what both IDEs are doing if this is both available at slot 0 and on the copied slot that is referrenced in the LVT. It would be available from both locations. 
I added an exemption for remapping the *this* variable's index. As it is assumed immutable, that exemption works for the *this* variable only but it seems to yield the correct result in the debugger. All other variables are still remapped what is necessary since they might be mutated but the debugger seems to accept this.

In this context I noticed another unfortunate break point confusion. Since we prepend the line number to the very beginning of any method in order to add that line number to exceptions we throw from mocked methods, the debugger will stop if a break point is placed in the very first line of a method, even if the mocked path of the inline-mocked method is dispatched. In this case, no variables (including *this*) are displayed since the *real code* is never invoked but the call gets redirected into Mockito's own infrastructure. @TimvdLippe I think we rather have the Mockito exceptions show the right line number rather then not break in the first line of a method in case that a break point is set there, but maybe we should document this somewhere?
@raphw Yes I think that makes the most sense. We should probably add that caveat to https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#39
Is there any way to hot fix this issue locally? 
@iamkarlson We first need a new release of ByteBuddy. Then we can roll that into Mockito and release Mockito as well. We can do that relatively quickly, so it's mostly blocked on the ByteBuddy release.
@TimvdLippe  would that fix the eclipse as well as IntelliJ ?
It should fix both. A new release is underway in this minute.
We will need to wait until 1.11.1 is on Maven Central before we can update our dependency in Mockito.
It seems like the ByteBuddy release is experiencing some issues: https://github.com/raphw/byte-buddy/runs/2713960314
On it, javadoc does not like something, not reproducable locally but I'll find out what it is. 
Syncing to Central now and should be available throughout the night. We'll update Mockito soon but in the mean time it should be sufficient to simply bump the Byte Buddy dependency to 1.11.1.
I am waiting for #2312 to get merged (after review) and then we can upload the new version of Mockito to Maven Central.
> 
> 
> Syncing to Central now and should be available throughout the night. We'll update Mockito soon but in the mean time it should be sufficient to simply bump the Byte Buddy dependency to 1.11.1.

I tried upping Byte Buddy to 1.11.1 and still recreated the issue. Will try when the never version of Mokito is uploaded.

thanks again!
Mockito 3.11.0 is being pushed to Maven Central atm: https://github.com/mockito/mockito/actions/runs/903060692
I just took 3.11.0 and reran the test and unfortunately I am still seeing the same issue. Let me know if there is anything else that I can add.
@jwb441 Could you double-check that you aren't accidentally using an older version of ByteBuddy via a different transitive dependency? I can retest this issue tomorrow.
I just tested it myself in IntelliJ, and it does now work as expected. There is one caveat, however. If you set the breakpoint in the first line of the method, it will:

a) not contain the *this* reference if stopped in the first line.
b) trigger the breakpoint for mocked method calls.

This is something we can hardly avoid, unfortunately. But we should certainly document it in the `Mockito` class.
@TimvdLippe  and @raphw  - I missed that part in the discussion so went back and added a few more lines to the method in the spied class and I noticed a few things. If I put the break point in the method that I'm calling it never gets tripped. When I put it in the test and step into it I'm getting the error on line one (expected) and then my next step always jumps to the end of the method - I do get 'this' at that point though.
still got the same error. I am using mockito-core 4.0.0
Hi, any progress on that? We have the same issue with mockito-inline 5.2.0. 
I just tested and I cannot observe this problem. Are you sure that the method is non-static?
Hi @raphw 

We are using mockito-core 4.11 and mockito-inline 4.11 in Eclipse IDE. Our  tests do not enter the invoked method, and if we force the entry with step into (F5), we get the error reported above:

com.sun.jdi.InternalException: Got error code in reply:35 occurred retrieving 'this' from stack frame.

Surprisingly, if we remove the mockito-inline dependency, these problems disappear and debugging works correctly.

Is there a bug in mockito-inline? Does it have debugging support?

Thank in advance.
We do not support older versions. Have you tried with the latest version of Mockito?

To me the exception looks like a bug in a different tool, so you might try updating your JVM, too.
Hello again and thanks for your attention,

I confirm that with mockito-core 5.12 and modifying the MockerMock to subclass, my code is correctly debugged. But with the default MockMaker (inline) I can't debug, my internal breakpoints are ignored.

Can you please confirm if MockMaker inline is compatible with debug mode?
Where are you setting the breakpoint? You mean when invoking the actual method?
> Where are you setting the breakpoint? You mean when invoking the actual method?

The breakpoints that are causing me problems with MockMaker inline (I mean, the execution doesn't stop) are the ones that are in the real methods, invoked by the spied object. 
If the method has multiple lines: does it also happen in a line further down?
Update: I just tried the same thing with IntelliJ IDE instead of Eclipse and with IntelliJ all my breakpoints are working correctly.

Maybe it's easier for everyone to show you my code. I think it's very easy to reproduce. I have a test class SaludoTest.java, which creates a SaludoImpl spy object and then executes the method SaludoImpl.sayHello(). I want the code to stop in the sayHello() method itself.

I hope this helps. Thank you very much.

![mockito-inline](https://github.com/mockito/mockito/assets/50365161/2de3fee3-696e-4319-af52-a64fa804999b)


I also assume that is not related to the JVM but to the IDE not accepting instrumented code.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java
index edb9640afb..70abf95fb9 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java
@@ -136,7 +136,7 @@ public <T> Class<? extends T> mockClass(MockFeatures<T> features) {
         if (shouldIncludeContextLoader) {
             loaderBuilder = loaderBuilder.appendMostSpecific(contextLoader);
         }
-        ClassLoader classLoader = loaderBuilder.build(MockMethodInterceptor.class.getClassLoader());
+        ClassLoader classLoader = loaderBuilder.build();
 
         // If Mockito does not need to create a new class loader and if a mock is not based on a JDK
         // type, we attempt


Test patch:
diff --git a/src/test/java/org/mockitousage/bugs/creation/PackagePrivateWithContextClassLoaderTest.java b/src/test/java/org/mockitousage/bugs/creation/PackagePrivateWithContextClassLoaderTest.java
index e01365ba99..4f6ffbee19 100644
--- a/src/test/java/org/mockitousage/bugs/creation/PackagePrivateWithContextClassLoaderTest.java
+++ b/src/test/java/org/mockitousage/bugs/creation/PackagePrivateWithContextClassLoaderTest.java
@@ -9,6 +9,11 @@
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.withSettings;
 
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.Method;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -18,11 +23,12 @@ public class PackagePrivateWithContextClassLoaderTest {
 
     private ClassLoader oldContextClassloader;
 
-    public static class PublicClass {
-
+    public abstract static class PublicClass {
         int packagePrivateMethod() {
             return 0;
         }
+
+        abstract void packagePrivateAbstractMethod();
     }
 
     public interface PublicInterface {}
@@ -67,4 +73,102 @@ public void should_be_able_to_mock_package_private_extra_interface() throws Exce
                                 PublicInterface.class,
                                 withSettings().extraInterfaces(PackagePrivateInterface.class));
     }
+
+    /**
+     * In this test we have a class that delegates loading of mockito/JDK classes to its parent,
+     * but defines in its own for others. If mockito selects the defining classloader of the mock
+     * to the classloader of mockito, calling the abstract package-private method will fail - the
+     * defining classloader of the mocked type's package is different from the generated mock class
+     * package. Because the nonDelegatingLoader is a child of mockito's loader, it's more specific
+     * and should be preferred.
+     */
+    @Test
+    public void classloader_with_parent_but_does_not_delegate() throws Exception {
+        ClassLoader nonDelegatingLoader = new NotAlwaysDelegatingClassLoader();
+        Thread.currentThread().setContextClassLoader(nonDelegatingLoader);
+        Class<?> loaded =
+                Class.forName(LoadedByCustomLoader.class.getName(), false, nonDelegatingLoader);
+        Method attemptMock = loaded.getDeclaredMethod("attemptMock");
+        attemptMock.invoke(null);
+    }
+
+    public static class LoadedByCustomLoader {
+        public static void attemptMock() {
+            PublicClass mock = mock(PublicClass.class);
+            mock.packagePrivateAbstractMethod();
+        }
+    }
+
+    /**
+     * This classloader has a parent, but doesn't always delegate to it.
+     */
+    public static final class NotAlwaysDelegatingClassLoader extends ClassLoader {
+
+        /**
+         * Initial size of buffer used to read class data.
+         */
+        /*  Note: should be enough for most classes, and is not a hard limit. */
+        private static final int BUF_SIZE = 4096;
+
+        public NotAlwaysDelegatingClassLoader() {
+            super(NotAlwaysDelegatingClassLoader.class.getClassLoader());
+        }
+
+        @Override
+        protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+            // First, check if the class has already been loaded. If not, load it
+            // ourselves or delegate to the parent.
+            Class<?> result = findLoadedClass(name);
+            if (result == null) {
+                // All classes defined in this testsuite should be loaded by this classloader,
+                // but any other class (e.g. those coming from java.* or org.mockito.* packages)
+                // will be loaded by the parent.
+                if (name.startsWith("org.mockitousage.")) {
+                    result = findClass(name);
+                } else {
+                    return super.loadClass(name, resolve);
+                }
+            }
+            if (resolve) {
+                resolveClass(result);
+            }
+            return result;
+        }
+
+        @Override
+        public Class<?> findClass(String className) throws ClassNotFoundException {
+            try {
+                // Create a package for this class, unless it's in the default package.
+                int dotpos = className.lastIndexOf('.');
+                if (dotpos != -1) {
+                    String pkgname = className.substring(0, dotpos);
+                    if (getPackage(pkgname) == null) {
+                        definePackage(pkgname, null, null, null, null, null, null, null);
+                    }
+                }
+                String resourceName = className.replace('.', File.separatorChar) + ".class";
+                InputStream input = getSystemResourceAsStream(resourceName);
+                if (input == null) {
+                    throw new ClassNotFoundException("Couldn't find resource " + resourceName);
+                }
+                byte[] classData = loadClassData(input);
+                return defineClass(className, classData, 0, classData.length, null);
+            } catch (IOException e) {
+                throw new ClassNotFoundException("Cannot load " + className, e);
+            }
+        }
+
+        /**
+         * Load class data from a given input stream.
+         */
+        private byte[] loadClassData(InputStream input) throws IOException {
+            ByteArrayOutputStream output = new ByteArrayOutputStream(BUF_SIZE);
+            byte[] buffer = new byte[BUF_SIZE];
+            int readCount;
+            while ((readCount = input.read(buffer, 0, BUF_SIZE)) >= 0) {
+                output.write(buffer, 0, readCount);
+            }
+            return output.toByteArray();
+        }
+    }
 }