JMockit on multiple class loaders does not work nicely

[Vampire]

I'm not 100% sure how it is intended with the mocking bridge fields that are stored in NegativeArraySizeException or similar, but it doesn't work too nicely if you use JMockit from multiple classloaders.

To reproduce the error, just comment out the MockingBridge.setMockingBridgeFields(); line from my PR #309. Tests that use mocking bridge fields will fail thereafter, e. g. changeContextCLDuringReplay() will fail with a NullPointerException.

The problem here is, if you initialize JMockit on some class loader B where it was not initialized yet, the mocking bridge fields in NegativeArraySizeException or similar are updated to the ones from that class loader B and the ones from class loaders (A) where JMockit was initialized before are lost at this place.

If you then use JMockit on the class loader A where it was initilized before B, the classes from A are used. But the transformed-in calls that use the mocking bridge fields in NegativeArraySizeException or similar use the mocking bridges from class loader B which does not harmonize of course, as then various static fields etc. are not like expected, like e. g. mockit.internal.state.MockClasses#mockupClassesToMockupInstances which misses the instances in the B version that are present in the A version.

I'd like to report this either as bug if JMockit should work fine with multiple classloaders, or as improvement request if this is expected behaviour.

Also if this is expected behaviour for now, please at least document it somewhere (I didn't find it mentioned) and maybe add some check that recognizes that the mocking bridge fields class loader does not match the other JMockit classes class loader and raise some meaningful error that explains that you can only use JMockit on one class loader at a time and if you switched to a new class loader that you cannot easily go back to the old one.

[Vampire]

Btw. what I tried already:

I was able to add Maps to the host class instead of the InvocationHandlers directly and I was able to fill them with the correct entries. (ClassLoader -> MockingBridge from that ClassLoader)
But I don't get them extracted from the rewritten code of a system class.
e. g. in the mockit.CustomClassLoadingTest#changeContextCLDuringReplay test when ic.lookup(anyString) is called and the line I mentioned above was commented out.

There I would need to get the right ClassLoader to get the right MockingBridge instance.
For this I tried to inject the code NegativeArraySizeException.$MMB.get(MockingBridge.class.getClassLoader());, but I get a NoClassDefFoundError presumably because the InitialContext class is a JRE class and so the mockit classes cannot be found for loading.

So at time the rewritten code is called, I guess there is no chance to find out which classloader should currently be used, is there? Maybe this can be solved in any other way?

[Vampire]

Btw. what I tried already:

I was able to add Maps to the host class instead of the InvocationHandlers directly and I was able to fill them with the correct entries. (ClassLoader -> MockingBridge from that ClassLoader)
But I don't get them extracted from the rewritten code of a system class.
e. g. in the mockit.CustomClassLoadingTest#changeContextCLDuringReplay test when ic.lookup(anyString) is called and the line I mentioned above was commented out.

There I would need to get the right ClassLoader to get the right MockingBridge instance.
For this I tried to inject the code NegativeArraySizeException.$MMB.get(MockingBridge.class.getClassLoader());, but I get a NoClassDefFoundError presumably because the InitialContext class is a JRE class and so the mockit classes cannot be found for loading.

So at time the rewritten code is called, I guess there is no chance to find out which classloader should currently be used, is there? Maybe this can be solved in any other way?

[rliesenfeld]

To avoid we both losing time working on different directions:

1. I am currently removing the old code (from 2014) that deals with custom class loaders, which was added for Maven's IsolatedClassLoader (as Maven itself has essentially dropped this class loader).
2. The only real-world situations I am aware of that would need custom classloading support are: a) using JMockit in Robolectric (Android) tests; and b) using JMockit in Arquillian tests. There is little to no demand for Robolectric support, and so far I don't think it would be a good idea to add said support. The same applies to Arquillian: little user demand, but mostly it looks like a bad idea for several reasons.
3. Finally, if support for one of these cases which involve custom classloading were to be added, it would be imperative to have a separate Maven module in the JMockit project, which contain a few representative tests. I would need this to really understand the problem and to figure out what changes are actually needed in classes like mockit.internal.startup.Startup. Isolated PRs, where the "why do it at all" and "why do it this way" are not clear, are not going to work for me.

[rliesenfeld]

To avoid we both losing time working on different directions:

1. I am currently removing the old code (from 2014) that deals with custom class loaders, which was added for Maven's IsolatedClassLoader (as Maven itself has essentially dropped this class loader).
2. The only real-world situations I am aware of that would need custom classloading support are: a) using JMockit in Robolectric (Android) tests; and b) using JMockit in Arquillian tests. There is little to no demand for Robolectric support, and so far I don't think it would be a good idea to add said support. The same applies to Arquillian: little user demand, but mostly it looks like a bad idea for several reasons.
3. Finally, if support for one of these cases which involve custom classloading were to be added, it would be imperative to have a separate Maven module in the JMockit project, which contain a few representative tests. I would need this to really understand the problem and to figure out what changes are actually needed in classes like mockit.internal.startup.Startup. Isolated PRs, where the "why do it at all" and "why do it this way" are not clear, are not going to work for me.

[Vampire]

To avoid we both losing time working on different directions:

I'm not currently working on this. I just shared the things I tried while I tried to fix my other PR before I recognized the real problem.

I am currently removing the old code (from 2014) that deals with custom class loaders, which was added for Maven's IsolatedClassLoader (as Maven itself has essentially dropped this class loader).

The only real-world situations I am aware of that would need custom classloading support are: a) using JMockit in Robolectric (Android) tests; and b) using JMockit in Arquillian tests. There is little to no demand for Robolectric support, and so far I don't think it would be a good idea to add said support. The same applies to Arquillian: little user demand, but mostly it looks like a bad idea for several reasons.

Finally, if support for one of these cases which involve custom classloading were to be added, it would be imperative to have a separate Maven module in the JMockit project, which contain a few representative tests. I would need this to really understand the problem and to figure out what changes are actually needed in classes like mockit.internal.startup.Startup. Isolated PRs, where the "why do it at all" and "why do it this way" are not clear, are not going to work for me.

But why remove a feature that mostly works fine?
This feature would be needed for any situation where JMockit is used on a custom class loader, like writing an IntelliJ IDEA plugin with JMockit enabled integration tests and similar, or any tests that are run on an application server - be it with Arquillian or without doesn't make any difference.

So are you telling me that in the near future we are not able to update JMockit anymore, or we will not be able to use it within our Arquillian tests anymore? I just need to know, because then I can remove JMockit from our integration tests again and forbid the usage of JMockit. Now would be the right time to either do this or look for another mocking framework that supports custom class loaders.

[Vampire]

To avoid we both losing time working on different directions:

I'm not currently working on this. I just shared the things I tried while I tried to fix my other PR before I recognized the real problem.

I am currently removing the old code (from 2014) that deals with custom class loaders, which was added for Maven's IsolatedClassLoader (as Maven itself has essentially dropped this class loader).

The only real-world situations I am aware of that would need custom classloading support are: a) using JMockit in Robolectric (Android) tests; and b) using JMockit in Arquillian tests. There is little to no demand for Robolectric support, and so far I don't think it would be a good idea to add said support. The same applies to Arquillian: little user demand, but mostly it looks like a bad idea for several reasons.

Finally, if support for one of these cases which involve custom classloading were to be added, it would be imperative to have a separate Maven module in the JMockit project, which contain a few representative tests. I would need this to really understand the problem and to figure out what changes are actually needed in classes like mockit.internal.startup.Startup. Isolated PRs, where the "why do it at all" and "why do it this way" are not clear, are not going to work for me.

But why remove a feature that mostly works fine?
This feature would be needed for any situation where JMockit is used on a custom class loader, like writing an IntelliJ IDEA plugin with JMockit enabled integration tests and similar, or any tests that are run on an application server - be it with Arquillian or without doesn't make any difference.

So are you telling me that in the near future we are not able to update JMockit anymore, or we will not be able to use it within our Arquillian tests anymore? I just need to know, because then I can remove JMockit from our integration tests again and forbid the usage of JMockit. Now would be the right time to either do this or look for another mocking framework that supports custom class loaders.

[Vampire]

Yep, I tested with your latest commits and now JMockit is not usable anymore in WildFly or any other system with custom classloaders, no matter if -javaagent option is used or not, it just does not work anymore but throws a NullPointerException on initialization. :-(

[Vampire]

Yep, I tested with your latest commits and now JMockit is not usable anymore in WildFly or any other system with custom classloaders, no matter if -javaagent option is used or not, it just does not work anymore but throws a NullPointerException on initialization. :-(

[rliesenfeld]

The code thas was removed didn't really work; for some tests yes, but several other tests in JMockit's test suite failed for various reasons. In the end, no one was relying on the custom classloading support, which is not surprising given it was old code from 2014 and wasn't being kept up-to-date with every new release since then.

[rliesenfeld]

The code thas was removed didn't really work; for some tests yes, but several other tests in JMockit's test suite failed for various reasons. In the end, no one was relying on the custom classloading support, which is not surprising given it was old code from 2014 and wasn't being kept up-to-date with every new release since then.

[Vampire]

Well, we started to rely on it. ;-)
Now we have to consider the other mocking frameworks to see whether another one supports the use-case. :-(

[Vampire]

Well, we started to rely on it. ;-)
Now we have to consider the other mocking frameworks to see whether another one supports the use-case. :-(

[rliesenfeld]

Yes, I think that's the way to go.

[rliesenfeld]

Yes, I think that's the way to go.

[Vampire]

Which also is bad, as all our non-integ tests should be rewritten to use the new mocking framework if one is found and that will be quite some work. :-(

[Vampire]

Which also is bad, as all our non-integ tests should be rewritten to use the new mocking framework if one is found and that will be quite some work. :-(

[rliesenfeld]

I ran into a serious problem while testing JMockit initialization through the Attach API from inside a running Wildfly server (from a .war file deployed by an Arquillian test): when JMockit tries to instantiate the necessary VirtualMachine class (in AgentLoader#getVirtualMachineImplementationFromEmbeddedOnes()), it gets an UnsatisfiedLinkError: Native library jdk\jre\bin\attach.dll already loaded in another classloader.

It seems that the Wildfly server has already loaded the attach native library, probably from the regular system classloader. And a native library cannot be loaded multitple times from different class loaders. At this point, I don't know how to avoid this failure (JMockit needs the VirtualMachine instance so it can call loadAgent(pathToJar) on it). Any ideas?

[rliesenfeld]

I ran into a serious problem while testing JMockit initialization through the Attach API from inside a running Wildfly server (from a .war file deployed by an Arquillian test): when JMockit tries to instantiate the necessary VirtualMachine class (in AgentLoader#getVirtualMachineImplementationFromEmbeddedOnes()), it gets an UnsatisfiedLinkError: Native library jdk\jre\bin\attach.dll already loaded in another classloader.

It seems that the Wildfly server has already loaded the attach native library, probably from the regular system classloader. And a native library cannot be loaded multitple times from different class loaders. At this point, I don't know how to avoid this failure (JMockit needs the VirtualMachine instance so it can call loadAgent(pathToJar) on it). Any ideas?

[Vampire]

Are you sure you didn't deploy the same WAR two times? If you deploy the same WAR two times, or deploy different WAR files that both use the Attach API, you will get this error. On first try everything will work fine. But if you loaded another WAR before that tried to use the Attach API, even if it failed because it wasn't able to determine the right jmockit.jar, the VM is busted for further Attach API usages from other class loaders until restart. With a JMockit 1.25 with my "recreate JAR on disk" PR it works fine, because then the loading works on first try and on second try it sees that JMockit is already loaded in the VM. (It can just happen that it is from a different version, hence the version check PR)

[Vampire]

Are you sure you didn't deploy the same WAR two times? If you deploy the same WAR two times, or deploy different WAR files that both use the Attach API, you will get this error. On first try everything will work fine. But if you loaded another WAR before that tried to use the Attach API, even if it failed because it wasn't able to determine the right jmockit.jar, the VM is busted for further Attach API usages from other class loaders until restart. With a JMockit 1.25 with my "recreate JAR on disk" PR it works fine, because then the loading works on first try and on second try it sees that JMockit is already loaded in the VM. (It can just happen that it is from a different version, hence the version check PR)

[rliesenfeld]

Yep, you're right, the war had been deployed before, and it worked the first time. So, from the second time JMockit simply needs to read the "Startup#instrumentation" field on the Startup class loaded in the system classloader.

The recreateJarFileFromClasspath() method is too complicated; I am going to try and find a simpler solution; maybe extracting the jmockit.jar from the deployed EAR/WAR file, if that "VFS" class loader can be used.

[rliesenfeld]

Yep, you're right, the war had been deployed before, and it worked the first time. So, from the second time JMockit simply needs to read the "Startup#instrumentation" field on the Startup class loaded in the system classloader.

The recreateJarFileFromClasspath() method is too complicated; I am going to try and find a simpler solution; maybe extracting the jmockit.jar from the deployed EAR/WAR file, if that "VFS" class loader can be used.

[Vampire]

Yep, you're right, the war had been deployed before, and it worked the first time. So, from the second time JMockit simply needs to read the "Startup#instrumentation" field on the Startup class loaded in the system classloader.

And the hostClassName probably, just as it was before you removed the code. :-)
Otherwise the rewritten code will call into the wrong mock bridge fields.
And then you probably again have the case described in this issue, that JMockit can only be used from the last classloader where it was initilized from and thus the mocking bridge fields were assigned from. Except you find a better solution for this. :-)

The recreateJarFileFromClasspath() method is too complicated; I am going to try and find a simpler solution; maybe extracting the jmockit.jar from the deployed EAR/WAR file, if that "VFS" class loader can be used.

You are welcome to find an easier working solution. I just doubt you will, as you most probably cannot access the jmockit.jar from the class loader, as it contains stuff that is made available through class loading, but I think you cannot access the jmockit.jar itself via class loading. So you could probably at build time first build jmockit.jar and then again build jmockit.jar containing itself. This you can then access as resource from the class loader, but it will double the size of jmockit.jar as it then has to contain itself. That's why I chose the approach that I implemented in my PR which should work with any class loader that anyone can invent. :-)

[Vampire]

Yep, you're right, the war had been deployed before, and it worked the first time. So, from the second time JMockit simply needs to read the "Startup#instrumentation" field on the Startup class loaded in the system classloader.

And the hostClassName probably, just as it was before you removed the code. :-)
Otherwise the rewritten code will call into the wrong mock bridge fields.
And then you probably again have the case described in this issue, that JMockit can only be used from the last classloader where it was initilized from and thus the mocking bridge fields were assigned from. Except you find a better solution for this. :-)

The recreateJarFileFromClasspath() method is too complicated; I am going to try and find a simpler solution; maybe extracting the jmockit.jar from the deployed EAR/WAR file, if that "VFS" class loader can be used.

You are welcome to find an easier working solution. I just doubt you will, as you most probably cannot access the jmockit.jar from the class loader, as it contains stuff that is made available through class loading, but I think you cannot access the jmockit.jar itself via class loading. So you could probably at build time first build jmockit.jar and then again build jmockit.jar containing itself. This you can then access as resource from the class loader, but it will double the size of jmockit.jar as it then has to contain itself. That's why I chose the approach that I implemented in my PR which should work with any class loader that anyone can invent. :-)

[rliesenfeld]

I already have a (partial) working solution which creates a temp jar file containing only the META-INF/MANIFEST.MF file and one new class called "InstrumentationHolder" (which is simply read from the current class loader using "ClassFile.readFromFile(Class)"); nothing needs to be done at build time. The code changes will be made available on sunday, I expect.

[rliesenfeld]

I already have a (partial) working solution which creates a temp jar file containing only the META-INF/MANIFEST.MF file and one new class called "InstrumentationHolder" (which is simply read from the current class loader using "ClassFile.readFromFile(Class)"); nothing needs to be done at build time. The code changes will be made available on sunday, I expect.

[Vampire]

I'm looking forward to seeing your solution. How do you know which classes you pack into the temporary jar? Do you hard-code a list? I made the build-time step to not having the necessity to hard-code a list of files that needs to be in the JAR, and the MD5 step to disambiguate resources on the classpath like org.junit.Runner or META-INF/MANIFEST.MF. And the temp-creation and renaming to jmockit-<MD5>.jar to both, not trying to override a file that is potentially locked by the filesystem and not creating a new JAR file on the filesystem on each run.

[Vampire]

I'm looking forward to seeing your solution. How do you know which classes you pack into the temporary jar? Do you hard-code a list? I made the build-time step to not having the necessity to hard-code a list of files that needs to be in the JAR, and the MD5 step to disambiguate resources on the classpath like org.junit.Runner or META-INF/MANIFEST.MF. And the temp-creation and renaming to jmockit-<MD5>.jar to both, not trying to override a file that is potentially locked by the filesystem and not creating a new JAR file on the filesystem on each run.

[rliesenfeld]

There is only one class that needs to go into the temp jar file: the one pointed at by the "Agent-Class" attribute in MANIFEST.MF. This part is already working, but I ran into other issues (mainly, that ClassFileTransformers get added to the global Instrumentation object every time JMockit is initialized on a new class loader - I am addressing this by having an InstrumentationWrapper that prevents duplicate transformes).

[rliesenfeld]

There is only one class that needs to go into the temp jar file: the one pointed at by the "Agent-Class" attribute in MANIFEST.MF. This part is already working, but I ran into other issues (mainly, that ClassFileTransformers get added to the global Instrumentation object every time JMockit is initialized on a new class loader - I am addressing this by having an InstrumentationWrapper that prevents duplicate transformes).

[Vampire]

Nice, this works great so far.
I added a call to Startup.initializeIfPossible() to our startup bean so that JMockit is in any case initialized at start and not depending on the classpath ordering that cannot be influenced to prevent on-demand initialization.
Now we can use JMockit properly in the Arquillian tests, many thanks.

There are two issues remaining though, but I'll open separate issues about this.

1. Now each time a new jmockit....jar is created in the temp folder, even if it is only 4 KiB, it still fills up the temp directory
2. Each MockUp you create that mocks a class that is one of your libs (not a JRE class or something that is shipped with the app-server, but shipped with your EAR) will produce the "internal class mocked" warning, even the three JMockit-internal startup mocks are affected

[Vampire]

Nice, this works great so far.
I added a call to Startup.initializeIfPossible() to our startup bean so that JMockit is in any case initialized at start and not depending on the classpath ordering that cannot be influenced to prevent on-demand initialization.
Now we can use JMockit properly in the Arquillian tests, many thanks.

There are two issues remaining though, but I'll open separate issues about this.

1. Now each time a new jmockit....jar is created in the temp folder, even if it is only 4 KiB, it still fills up the temp directory
2. Each MockUp you create that mocks a class that is one of your libs (not a JRE class or something that is shipped with the app-server, but shipped with your EAR) will produce the "internal class mocked" warning, even the three JMockit-internal startup mocks are affected