ReflectUtils.defineClass() ignores the ClassLoader argument in Java 11

[dvega]

Affects: 5.1.5

---

In Java 11 ReflectUtils.defineClass() uses the new MethodHandles.Lookup.defineClass(), this method uses the contextClass's classloader, instead of the loader argument.

This difference in behaviour in Java 8 and Java 11 sometimes produces unexpected errors (eg. java.lang.LinkageError: attempted duplicate class definition)

Related issue #22310

[jhoeller]

Are there specific cases where your target ClassLoader has to differ from the one that loaded the context class? Could we potentially identify them through a few standard rules, not having to reconfigure the entire mechanism for common upgrade scenarios (which requires understand of the underlying mechanisms to begin with)?

In any case, all we can do is to choose ClassLoader.defineClass even on JDK 9+ through a few further checks and assumptions, accepting the warnings that the JVM will log then. It's a fine line though; after all, ClassLoader.defineClass remains effectively deprecated for external calls on JDK 9+, with no equally powerful replacement API.

[tpucal]

We also face this issue in our application.

We deploy multiple web application that share common libraries. In shared libraries we have a class with @Cacheable annotation.
Each web application is using this class in its separate spring context. However, after upgrade to Java 11 and Spring 5 we get java.lang.LinkageError.

Class generated via cglib by a web app ends up in the shared ClassLoader. When next web app tries to generate a class with the same name, it falls back to ClassLoader.defineClass. It correctly defines it in a web ClassLoader.
Unfortunately, if we follow a certain sequence we end up with Spring always failing to defineClass. MethodProxy.c2 will refer to the shared ClassLoader that already has a class with the same name.

I have created an example project that demonstrates this behaviour:
https://github.com/tpucal/spring5-cglib-linkage-error

[jhoeller]

After a bit of back and forth, I went with a revised standard algorithm in ReflectUtils.defineClass:

We only prefer the JDK 9+ Lookup.defineClass API if the specified ClassLoader matches the context class now, otherwise we try a classic reflective ClassLoader.defineClass call first... and unconditionally fall back to the JDK 9+ Lookup.defineClass API as a last resort (if ClassLoader.defineClass is unavailable, e.g. on the Jigsaw module path, where we try that approach even if the ClassLoader does not match).

This should cover custom class loader architectures like OSGi or shared Servlet container setups now, without the need for special CGLIB setup. There may be new JVM illegal access warnings in certain scenarios now which can be worked around through streamlining the ClassLoader layout or through setting --illegal-access=deny on JVM bootstrap (which enforces our unconditional fallback above).

[jhoeller]

This will be available in the upcoming 5.1.6.BUILD-SNAPSHOT (from repo.spring.io). If you have the opportunity, please give it a try as soon as possible so that we can fine-tune it before the release still.

[tpucal]

I've checked 5.1.6.BUILD-SNAPSHOT and it works fine. Thank you!