Class define support skips classes that are injected in system class loader

[lazar-mitrovic]

At the moment our experimental-class-define-support agent option skips classes that are loaded in one of built-in class loaders.
This poses an issue when running Scala's LambdaDeserializer as well as with any Mockito-related workload.

I attached a Mockito/ByteBuddy reproducer based on one demo from aws-samples/serverless-graalvm-demo repository:
mockito-reproducer.zip

You can run it by invoking bash run.sh. This will run tests and produce agent output as well as dump of all ByteBuddy generated classes using its internal TypeWriter. Output should look something like this:

Expected:

'java.lang.ClassLoader$ByteBuddyAccessor$MYh3dMuG.1642812941143.class'
net.bytebuddy.mirror.AccessibleObject.1642812940361.class
net.bytebuddy.mirror.AccessibleObject-original.1642812940361.class
'software.amazonaws.example.product.store.ProductStore$MockitoMock$1818293756$auxiliary$3kal1f2.1642812942578.class'
'software.amazonaws.example.product.store.ProductStore$MockitoMock$1818293756$auxiliary$6l2ker3.1642812942506.class'
'software.amazonaws.example.product.store.ProductStore$MockitoMock$1818293756.1642812942343.class'

Got:

[
  {
    "type":"agent-extracted",
    "classes":[
      { "nameInfo":"net/bytebuddy/utility/Invoker$Dispatcher", "hash":"ff8cc6ce1c2a2385c4cf869fce5810ce3b88922bae9aeacd7f343226ad8587d4" },
      { "nameInfo":"net/bytebuddy/mirror/AccessibleObject", "hash":"d1cf66e8ea66e501181ece0c63d8dea6ac1ba614693c44e7c35cfa3992507c86" }
    ]
  }
]

We can see that ProductStore$MockitoMock.* classes are missing from agent output. This is caused by Mockito trying to mock classes in same class loader as original class. Relevant code and comments:

https://github.com/mockito/mockito/blob/faa6e92aec0c5b854c11422224e0628fa787a1cb/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java#L145-L175

Those classes then get filtered out in our BreakpointInterceptor's onClassFileLoadHook:

graal/substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java

Lines 1224 to 1228 in 83c37dd

	for (JNIObjectHandle builtinLoader : builtinClassLoaders) {
	if (jniFunctions().getIsSameObject().invoke(jni, loader, builtinLoader)) {
	return;
	}
	}

Another potential issue is that AFAIK agent isn't additive nor thread safe, so if tests are being run in parallel, agent might overwrite old output. I'm not sure if this should be handled in agent or in native-build-tools though.

cc: @peter-hofer @vjovanov

[peter-hofer]

We can see that ProductStore$MockitoMock.* classes are missing from agent output. This is caused by Mockito trying to mock classes in same class loader as original class.

The problem at hand is, what criteria do you use to select classes? A class is ultimately defined for a specific loader from a byte array with no explicit well-defined source. By excluding the known built-in loaders, we exclude the JDK and anything else on the class or module path which can reasonably be expected to be available on the class/module path of native-image as well, and assume that everything else can be of interest for "dynamic" class loading. When classes are injected into the built-in loaders, this logic no longer applies. We currently already use heuristics to detect when that is the case for generated java.lang.reflect.Proxy classes. Perhaps we need to do the same for LambdaDeserializer and Mockito/ByteBuddy, but I assume that it's possible to generate classes with ByteBuddy (and other libraries like that) that do not follow some consistent pattern. Perhaps we can come up with a heuristic that uses the stack trace leading up to defineClass.

Another potential issue is that AFAIK agent isn't additive nor thread safe, so if tests are being run in parallel, agent might overwrite old output. I'm not sure if this should be handled in agent or in native-build-tools though.

The agent is thread-safe and can extend an existing configuration and if you observe anything different, it is a bug which we need to fix, but I suspect you mean something else. If you use agents in multiple processes in parallel and have them write to the same directory, the agents will overwrite each other's files. This is not something we can or should attempt to fix reliably on the agent level, instead whatever is using the agent this way needs to specify different directories and use native-image-configure to merge the configuration files eventually.

[vjovanov]

To find a good algorithm we first need good examples. @lazar-mitrovic we need to collect different traces that lead to defineClass from regular applications and see how can we distinguish them.

If there it is not decidable (or overly complicated) to see if a class is truly dynamically loaded, we can propose a modification to the JDK.

[mosche]

I was just investigating a case where experimental-class-define-support isn't extracting classes created using ByteBuddy's class loading strategy ClassLoadingStrategy.UsingLookup.of(...) and found this issue.

When a class is defined using a lookup they are injected into the corresponding classloader, and typically that's the system classloader :/

A minimal example:
predefinedClassesUsingLookup.zip

Class<?> clazz = (Class) MethodHandles.lookup().defineClass(CLASS_BYTES);
Object d = clazz.getDeclaredConstructor().newInstance();

[dreamlike-ocean]

I was just investigating a case where experimental-class-define-support isn't extracting classes created using ByteBuddy's class loading strategy ClassLoadingStrategy.UsingLookup.of(...) and found this issue.

When a class is defined using a lookup they are injected into the corresponding classloader, and typically that's the system classloader :/

A minimal example: predefinedClassesUsingLookup.zip

Class<?> clazz = (Class) MethodHandles.lookup().defineClass(CLASS_BYTES);
Object d = clazz.getDeclaredConstructor().newInstance();

I also encountered this problem. How can I get around it?

Class<?> aClass = unloaded.load(nativeInterface.getClassLoader(), ClassLoadingStrategy.UsingLookup.of(lookup))
                    .getLoaded();

[vjovanov]

The only way to go around this is to perform this loading at image build time. The class predefinition is experimental and doesn't work in all the cases.