blackdrag commented on code in PR #2798: URL: https://github.com/apache/groovy/pull/2798#discussion_r3799723985
########## src/spec/doc/guide-integrating.adoc: ########## @@ -378,6 +378,43 @@ stub generation is done, for the joint compiler. However, overriding `CompilationUnit` is not recommended and should only be done if no other standard solution works. +== Class loader release in managed environments + +Applications that load and discard Groovy repeatedly — a web container performing (parallel) +redeployments with Groovy inside each web application, plugin systems, or any host that expects +class loaders to be garbage collected — need one extra consideration. + +Groovy associates runtime metadata (`ClassInfo`) with every class it dispatches on, including +JDK classes such as `String`. By default those associations use `java.lang.ClassValue`, which +gives the fastest possible lookup, but a long-standing JVM issue +(https://bugs.openjdk.org/browse/JDK-8136353[JDK-8136353]) prevents such associations on +long-lived classes from ever releasing their value — and with it the Groovy class loader the +value belongs to. In a container this shows up as metaspace growth on every redeployment, even +after the old application is undeployed. + Review Comment: This is fine for the integration guide, so I do not want to block this. But for us developers I think that is a bit too short for the complexity of ClassValue. First of all the bug you mentioned has been closed. There are follow-ups, but they are documentation issues, not something to be fixed. in that manner it works as designed. Think of it like this. With ClassValue we basically realize a chain: `key class - class association -> class value instance -> value` Important to note here is that the class of the ClassValue implementation itself does not prevent any unloading. It is different from a static field in the key class for example. Now if we add the value, then there is a strong association between the key class and the value. If the value is for example an empty ArrayList, then it will not prevent the unloading of the classloader for the ClassValue. But if in the Tomcat scenario with Groovy the value class is from Groovy, then the loader for Groovy will be prevented from unloading as long as the key class exists. If the key class is an immortal class like String, then even unloading Tomcat and having had Groovy loaded in an isolated class loader, would still keep that class loader around. The issue mentioned at openjdk.org is actually more complicated than this. Assume we store an ArrayList again, but the element is any instance of a class of that Groovy defining class loader. Then we avoided the class problem for the value, but again got it through one level of indirection. And the real problem in this scenario is that this can be a long chain. The sort formula for Groovy is then, key = Groovy class is fine. key == JDK class, bad. The issue itself was actually for the stored value referencing the ClassValue instance. I put emphasis on the class loader here, because that is actually the case we care about. There is a strategy against that: use a SoftReference for the value of ClassValue. Since the chain is now always broken there would be no issue anymore. Of course then the usage of the ClassValue must fulfill some constraints. Only getting the value no longer works, you have to check the reference and if it did not retain the class value, remove the association and recompute it. That means it must be legal to recompute the value in the first place and the recomputing cost is to be considered. Based on that I see some potentially problematic parts in Groovy: * ClassInfo.globalClassValue -> static + unknown keys -> problem * AwaitableAdapterRegistry.awaitableCache -> static + unknown keys -> problem * Closure.CALL_OVERRIDES -> should be ok I think we should investigate the two problems above further. So maybe to not loose this kind of context we create a followup issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
