Afterburner MyClassLoader#loadAndResolve() is not idempotent when tryToUseParent is true #49
Description
The MyClassLoader used by the Afterburner module checks to see if it has already loaded a class with the given name and short-circuits, but it does not check whether the parent class loader has already loaded the requested class:
// First things first: just to be sure; maybe we have already loaded it?
Class<?> old = findLoadedClass(className.getDottedName());
if (old != null) {
return old;
}
ClassLoader#defineClass fails with a LinkageError when a class with the same name has already been defined, so the second time that a generated class with the same base name and bytecode is loaded by afterburner, the attempt to define it on the parent class loader will fail and silently fall through to define it on the MyClassLoader itself. This will succeed, and produce no ill effect in most circumstances.
try {
Method method = ClassLoader.class.getDeclaredMethod("defineClass",
new Class[] {String.class, byte[].class, int.class,
int.class});
method.setAccessible(true);
return (Class<?>)method.invoke(getParent(),
className.getDottedName(), byteCode, 0, byteCode.length);
} catch (Exception e) {
// Should we handle this somehow?
}
However, when the bean class being deserialized is a private inner class (as is commonly seen in bean classes generated by the Immutables framework), it is necessary that the class loader into which afterburner injects its custom instantiator class be the same as the class loader which loaded the private inner class itself, or else an IllegalAccessError will occur during deserialization.
Under typical operations, afterburner will only generate a specific custom instantiator class once, but when two threads, each using an ObjectMapper with the same ClassLoader instance both attempt to deserialize the same type at the same time (for the first time), both threads can end up trying to define the same custom instantiator bytecode at almost exactly the same time. The first thread to make the attempt will define the new class on the "parent" class loader. The second attempt will fail to define the (now already existing) class on the parent class loader, and will fall back to defining it on the MyClassLoader itself. If the class being deserialized is a private inner class, the second thread will subsequently throw an IllegalAccessError during deserialization, and all further attempts to deserialize that type will also throw the same error.
I believe that MyClassLoader#loadAndResolve should first check to see if the parent class loader has loaded the requested class, then proceed with the logic of the method as written. Something like this at the top of the method:
Class<?> old = null;
if (_cfgUseParentLoader && getParent() != null) {
try {
Method method = ClassLoader.class.getDeclaredMethod("findLoadedClass", String.class);
method.setAccessible(true);
old = (Class<?>)method.invoke(getParent(), className.getDottedName());
} catch (Exception e) {
// Fall through
}
}
if (old != null) {
return old;
}
old = findLoadedClass(className.getDottedName());
if (old != null) {
return old;
}