Improve diagnostics for CGLIB ClassLoader issues with shared classes in parent ClassLoader

[iherasymenko]

Reproducible on JDK 11, 15 (presumably 9+) and the latest Spring Framework (5.2.9).

I put together a simple reproducer, which mimics the Tomcat setup where this issue can be observed.

The deployment configuration is as follows.

-- lib
     - spring libraries
     - shared lib
-- webapp1
     - Configuration1.class that references a class with @Transactional annotation from the shared lib
-- webapp2
     - Configuration2.class that references a class with @Transactional annotation from the shared lib

• If the JVM is run with --illegal-access=deny and BOTH apps are deployed, the second app fails with the error.
• If the JVM is run with --illegal-access=deny and just ONE app is deployed, everything works fine.
• If the JVM is run with --illegal-access=warn (or simply without this argument) and either one app or both apps are deployed, everything works fine.
• If the JVM is run without specifying --illegal-access, but the libraries are placed on the module path, the second app fails with the error.

The reproducer is here. Run gradlew runWithIllegalAccessDeny, gradlew runWithIllegalAccessWarn, and gradlew runOnModulePath accordingly.

The error:

java.lang.LinkageError-->loader 'app' attempted duplicate class definition for com.example.SimpleService$$EnhancerBySpringCGLIB$$df04dc18. (com.example.SimpleService$$EnhancerBySpringCGLIB$$df04dc18 is in unnamed module of loader 'app')

	at org.springframework.cglib.core.ReflectUtils.defineClass(ReflectUtils.java:558)
	at org.springframework.cglib.core.AbstractClassGenerator.generate(AbstractClassGenerator.java:363)
	at org.springframework.cglib.proxy.Enhancer.generate(Enhancer.java:585)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:110)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:108)
	at org.springframework.cglib.core.internal.LoadingCache$2.call(LoadingCache.java:54)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at org.springframework.cglib.core.internal.LoadingCache.createEntry(LoadingCache.java:61)
	at org.springframework.cglib.core.internal.LoadingCache.get(LoadingCache.java:34)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData.get(AbstractClassGenerator.java:134)
	at org.springframework.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:319)
	at org.springframework.cglib.proxy.Enhancer.createHelper(Enhancer.java:572)
	at org.springframework.cglib.proxy.Enhancer.createClass(Enhancer.java:419)
	at org.springframework.aop.framework.ObjenesisCglibAopProxy.createProxyClassAndInstance(ObjenesisCglibAopProxy.java:57)
	at org.springframework.aop.framework.CglibAopProxy.getProxy(CglibAopProxy.java:205)
	... 17 more

[jhoeller]

I'm afraid this is expected since the official JDK 9 Lookup.defineClass API only allows for defining a new class in the same ClassLoader as the "context class" (the original user class in this case), whereas our old ClassLoader.defineClass approach allows us to define the new class in the specific application ClassLoader (but unfortunately isn't a public API and therefore unavailable if you're applying --illegal-access=deny).

From that perspective, your current deployment layout is generally not supported with --illegal-access=deny. You'll have to co-locate your classes in each local application class loader where we are allowed to define isolated proxy classes for them.

That said, we could at least provide some hints for such scenarios in our documentation... since this isn't obvious at all.

[iherasymenko]

@jhoeller thanks for such a quick reply. Using --illegal-access=warn is not a problem for us (yet), so we will stick with this option.

Let me ask a question, just out of my curiosity. What if proxy classes had unique names per application context? Wouldn't that solve the issue?

[jhoeller]

It looks like we can address this along the lines of #28747, throwing a corresponding exception from ReflectUtils.defineClass where we add a specific exception message for how to solve this. We don't really have a good place in the general documentation where this could be easily found, so it seems necessary to catch and explain this on the spot.

As for proxy classes with unique names, that would only solve part of the problem since there would still be leaks from the higher ClassLoader to the shared ClassLoader in the runtime-generated classes. It's definitely preferable to isolate this into each specific application ClassLoader.