Chris Beams opened SPR-8190 and commented
Tracking issue for CGLIB problems and possible solutions when moving to Javassist or, as recently suggested, Byte Buddy.
Attachments: - permgen-leak-spring.tar.gz (492.76 kB)
Issue Links:
- #8831 Latest Spring AOP + CGLib caches classes from the same classloader causing CCE in Grails ("depends on")
- #10325 Consider using javassist for proxy generation ("depends on")
- #12142 @Configuration with AspectJ instead of CGLIB ("depends on")
- #7964 Look at phasing out cglib with javaassist ("is duplicated by")
44 votes, 45 watchers
Comment From: spring-projects-issues
Kyrill Alyoshin commented
Is it possible to give this issue a bit for weight. CGLIB is one big disaster. It comes with an old ASM version, unmaintained, and buggy. I think Spring is the only project left that is actually using it.
Comment From: spring-projects-issues
Chris Beams commented
@Kyrill,
It's definitely on the radar. It would be helpful to hear about exactly what pain this causes you... is it actually ASM version conflicts, for example? What else?
Comment From: spring-projects-issues
Kyrill Alyoshin commented
ASM is huge. I have no firm evidence for it, but I also think CGLIB still leaks memory in some scenarios. Fundamentally, CGLIB prevents us from using what you, Chris, have been working so hard on: configuring Spring in Java. :-)
Comment From: spring-projects-issues
Jason Day commented
I have issues with CGLIB leaking memory in OSGi environments where a bundle that uses CGLIB to create proxies will not let them go, even after removing the bundle. I suspect it's related to the root of #8831. The result is that the permgen space eventually runs out after multiple re-deploys of bundles that use CGLIB. Currently, it's merely an inconvenience in our development environments that requires us to occasionally restart our instances since we plan to restart production instances on every application deployment.
To reproduce the issue, create a Spring bundle with a CGLIB enhanced, for example an empty @Configuration file such as the following:
@Configuration
public class MyConfig {
// class is enhanced by CGLIB
}
Then deploy the bundle to an OSGi environment. Each time the bundle is stopped and started again a new instance of the MyConfig class is re-created, each time enhanced by CGLIB, which gets cached. Take a thread dump and search for "MyConfig" in the heap and you'll see each CGLIB enhanced instance hanging around. This was produced with Spring 3.1.1 and CGLIB 2.2.
Comment From: spring-projects-issues
Phil Zoio commented
Get a similar issue when using module redeploys using Impala. Any module which uses the @Transactional where the proxying is class rather than interfaces based is affected. Each time module reloading recurrs for these modules, some more perm gen space is used up, until it eventually runs out.
Ultimately, this issue strongly discourages making any hot redeploy of modules available in production environments, so it would be a real bonus to get it sorted.
Have been using Spring 3.0.6 and Cglib 2.2. Is this problem likely to have been fixed by an upgrade to Spring 3.2, and if not, how much longer should we expect for a resolution?
Comment From: spring-projects-issues
Chris Beams commented
Phil, I'd like to make sure this is pinned down once and for all. Can you provide me with reproduction steps to get up and running with the simplest possible impala-based app that demonstrates this issue? While you're at it, please double check against 3.2. We did make some tweaks to how CGLIB caching is done there, but I cannot be sure whether it will solve your problem without looking more closely.
Comment From: spring-projects-issues
Tim Jones commented
I am not sure how far this issue has progressed but if it helps I have created a few reproducible tests that can be run in Karaf that I think highlight the issues discussed here and in https://jira.springsource.org/browse/SPR-5654
Attached is a zip file that contains the tests and hopefully reasonable instructions on how to set up the tests. I have also added a few observations of my own.
In summary I have set up 2 bundles, with one bundle exporting a transaction manager and the other bundle referencing the transaction manager with a class that has been transactionally advised via a @Transactional annotation.
The tests are further divided with one group dependent on Spring 3.1.4 and the second group dependent on Spring 3.2.2. Within these two groups there are two referencing bundles, one that is coded against interfaces (=> JDK proxy) and the other without interfaces (=> CGLIB).
Both the Spring 3.1.4 and Spring 3.2.2 test bundles that use interfaces seem to be fine, i.e. the referencing bundle can be loaded and unloaded many times without resulting in a OutOfMemoryError: PermGen space error. I was able to verify using JVisualVM that the classes are collected although there seems to be a difference as to when they are collected (details of which are in the readme of the zip).
Both the Spring 3.1.4 and Spring 3.2.2 test bundles that do not use interfaces and hence CGLIB comes into play ultimately fail with a OutOfMemoryError: PermGen space error. I was able to verify using JVisualVM that the classes are not collected and that after uninstalling the referencing bundle that instances of the test classes still remain.
It is possible that the tests could be simplified further, perhaps one bundle rather than two would suffice but I think separating the transaction manager is more realistic.
I realise that class loader permgen leaks are an old well trodden issue that came to the forefront when undeploying web apps but because OSGI permits breaking an application down into fine grained modules I think this issue will become even more apparent as the promise of undeploying/stopping/starting parts of an application becomes a significant driver for the adoption of OSGI.
I hope this helps.
Regards,
Tim
Comment From: spring-projects-issues
Tim Jones commented
Tests highlighting perm gen leaks using Karaf
Comment From: spring-projects-issues
Chris Beams commented
Tim, thank you for the detailed report. An actual move from CGLIB to Javassist is not high on our priority list right now. Could you create a new Bug, copy your comment over as the description and attach the zip there? That is more likely to get attention in the near term.
Comment From: spring-projects-issues
Tim Jones commented
I have created a new bug as requested https://jira.springsource.org/browse/SPR-10573
Comment From: spring-projects-issues
Rafael Winterhalter commented
Did you ever consider moving to something else then javassist? I want to suggest Byte Buddy (http://bytebuddy.net) as an option which is a rather new library. For full disclosure, note that I am the author of this library but I still hope this posting is not conceived as too self-promotional. I wrote the library after working with cglib and javassist for quite a while and I believe that I took the best ideas out of both frameworks but replaced the bad ideas with something better. I think Spring could profit especially by Byte Buddy's retention of primitive types, the smaller-sized proxy classes and the improved support for implementing around-advice. At last, with Byte Buddy repackaging ASM correctly from day one, users would not have to deal with cglib version conflicts anymore.
Finally, note that the makers of cglib moved their project to GitHub where they however stated that the library is officially not longer under active development. (https://github.com/cglib/cglib/issues/3)
Comment From: spring-projects-issues
Juergen Hoeller commented
We intend to stick with CGLIB for the time being, picking up CGLIB 3.2 whenever it gets released (which still seems to be planned).
Juergen
Comment From: spring-projects-issues
cemo koc commented
Spock also has just started use ByteBuddy. Mockito is using as well. It might be a good idea to revisit ByteBuddy to replace CGLIB for Spring 5.
Comment From: spring-projects-issues
Juergen Hoeller commented
From my perspective, we're happy enough with our repackaged version of CGLIB 3.2.4 against our custom ASM fork at this point, so this isn't a high priority for us in 5.0. I'm sure ByteBuddy is a fine piece but I'm also sure we're going to have some trouble and some regressions, beyond the migration effort to begin with. Keep in mind that we are not just using CGLIB for AOP proxies but also for runtime-generated configuration subclasses and generated lookup methods, for example.
In any case, I've brought this ticket back to life, leaving it in the 5.x backlog for the time being.
Comment From: spring-projects-issues
Rafael Winterhalter commented
I am currently helping Hibernate to migrate from Javassist to Byte Buddy after they had some issues and Spring is a big enough framework that I can offer you to try building a prototype.
Since Byte Buddy is just a class generator, it should be easy enough to switch library. In the end, only the generated code is exposed. If you wanted to, you could even mimic the "PoweredByCglib" part of the class name even though I would suggest something more meaningful such as SpringProxy or the like. Let me know if you are interested in my help, porting something from the cglib Enhancer is usually very straight forward!
Also, FYI, I still use some time maintaining cglib but the development has ended. We are only integrating critical bug fixes but we do not plan to apply any bigger extensions for future Java releases.
Comment From: spring-projects-issues
Oliver Drotbohm commented
One thing that came up in a discussion recently is that the pure weight of ByteBuddy is quite an obstacle. 2.6 MB is pretty hefty considering that current Spring Core is 1.2 MB and already including inlined CGLib, ASM and Objenesis. Even considering the proxying subsystem as a whole (AOP, Beans, Core) we're still at ~2MB on our own so that ByteBuddy would more than double the package. Did you consider to splitting the library up so that the footprint could be reduced?
What would definitely be interesting to see is — in contrast to strive to replace CGLib right away — whether there's a way we could provide users the option to switch to ByteBuddy if they explicitly opt into it.
Comment From: spring-projects-issues
Juergen Hoeller commented
Frankly, from a size perspective, I'd rather handcraft minimal ASM-based class generation code for our own purposes. The end result would probably be in the 200-300 KB ballpark, since we literally just need dynamic subclass generation for non-interface-based AOP proxies, for configuration classes and for lookup method replacements, always just overriding a few methods to intercept straight calls to them. As far as I'm aware, Weld and OpenWebBeans use custom code there as well, primarily for performance reasons (they need to optimize more aggressively due to the overuse of proxies in CDI) but possibly also for jar size reasons.
Then again, we simply don't have any issues with CGLIB at this point, not even on JDK 9 or OSGi. Our approach of generating proxy classes into the application class loader, with references to application code as well as framework code, doesn't mind references to our embedded variant of CGLIB (since such generated code depends on other parts of Spring core anyway). We do not make assumptions about the presence of annotations either: For introspection purposes, we always look at the user-declared class - if necessary, traversing up to the superclass of a CGLIB proxy class - which is quite an established pattern already.
FWIW, we always bind our variant of CGLIB to our embedded variant of ASM. Back in 2013, we made CGLIB 3.0 work against our opened-up version of ASM 4.2 for Java 8 support, so we didn't even have to wait for CGLIB 3.1 in Spring Framework 4.0 milestones. The recent CGLIB 3.2 upgrade was largely a bonus, and our opened-up ASM 5.1 variant works fine on JDK 9 already... Once ASM 6.0 is out, we'll apply the corresponding upgrade to our fork, but we don't have to wait for it, and we in particular don't have to wait for an ASM-6.0-based CGLIB version. It all works fine with our custom variants for our purposes on JDK 9 already.
I'm sure Byte Buddy is a fine piece of engineering, and its swiss army knife capabilities for creating classes from scratch are impressive. However, for our purposes, it looks like overkill: >2 MB extra mandatory jar size for no particular benefit with our use cases, not even for JDK 9 compatibility. Finally, there may be new side effects and maintenance issues: While CGLIB has several stakeholders and is small enough to be patched by ourselves, Byte Buddy is not even GA for a year yet and a huge single-maintainer codebase. It may be more actively developed at this point, but even with the best of intentions, there are no guarantees for the years to come.
Comment From: spring-projects-issues
Juergen Hoeller commented
The above said, I certainly don't mind a ByteBuddyAopProxy variant as an alternative to our CglibAopProxy. The question is just when we'd choose it: The plain presence of Byte Buddy on the classpath doesn't seem like a clear enough indication, in particular when brought in through Hibernate or the like.
I wouldn't bother with replacements for configuration class and lookup method processing since we'd have to introduce a specific class processing SPI there for a start. Runtime configuration classes don't get into the hands of user code anyway, and lookup methods are a niche feature.
Comment From: spring-projects-issues
Oliver Drotbohm commented
I wonder: are issues like #18894 something tied to CGLib? I.e. would those become easily solvable with something like ByteBuddy in place?
Comment From: spring-projects-issues
Juergen Hoeller commented
Well, I guess a custom proxy class could artifically redeclare annotations from interfaces, even in contrast to Java's usual inheritance rules... But effectively, this is simply us not checking corresponding interface methods for annotations in such a scenario. The easiest solution is an explicit lookup on our end there, in a particular with our rather sophisticated merged annotation support.
Comment From: spring-projects-issues
Rafael Winterhalter commented
I did consider splitting up Byte Buddy in the past and even did a prototype. In the end, the core dependency would have been something around 1.7 MB with dozens of extension modules which would have made the library much more difficult to use, Another issue with this modularization, with Byte Buddy being a convention over configuration framework components need to know about default implementations and not only some interface and I did not think that class path discovery would be the right approach to overcome this either. Therefore, I decided to retain the single jar file, also because I do not think we would have a different discussion over a 1.7 MB dependency. Many users of Byte Buddy that create Java agents shade and minimize the jar anyways what then creates the minimal sized-jar for the specific use case.
That said, I fully understand that you do not want to bring in a dependency of this size when it even overweights your core dependencies. From my own experience, I would still not recommend you to spin your own proxy mechanism. As a matter of fact, this is where I started out; I needed to write a small replacement for cglib that generated simple proxy classes. This "small library" is now Byte Buddy. Creating proxy code that is performant is not a trivial task and it took me years of fine-tuning to reduce the overhead of the proxy code to a minimum. For example, cglib's MethodProxy class quickly adds so many branches to its invoke method that its creates an inlining barrier for the JIT. On average, cglib proxied-calls bring 10 times the overhead on each invocation compared to Byte Buddy. In the end, this is the reason I suggested you to use the library in the first place; I can very much understand why you chose cglib as it perfectly covers your use-case.
Comment From: spring-projects-issues
Juergen Hoeller commented
Rafael, good point about proxy dispatch paths and inlining. An optimized dispatch code path would certainly be worth an option at least, i.e. back to the optional ByteBuddyAopProxy variant suggested above. It'd be great to experiment with that ahead of our 5.0 GA still.
Also, point taken, designing custom class generation code is non-trivial... but not unusual out there, we're even doing it ourselves for our SpEL compilation mode already. It is probably the only way to go for a default footprint as minimal as we have it with CGLIB now.
Ollie, with respect to annotation retaining, here's an interesting one from the Bean Validation world: https://hibernate.atlassian.net/projects/BVAL/issues/BVAL-491 ... If a library/framework is unaware of dynamic subclassing, it may evaluate redeclared annotations as user-specified on subclasses and apply certain constraints to that, assuming an explicitly conceived hierarchy on the user's end.
Comment From: spring-projects-issues
Rafael Winterhalter commented
I finally found the time to prototype a solution: https://github.com/raphw/spring-framework/commit/ba838f689012d5cb49671ca6d766192212ba730b - In the end, it was not difficult at all.
I am not entirely sure about the caching part, but all in all it should work out. I have not yet tested much of the solution but I would love to talk it through with someone. Is there a way to get in touch?
Comment From: spring-projects-issues
Rafael Winterhalter commented
I updated my PR with a hacked property-based switch in order to be able to run the unit tests. Everything seems to work just fine. Let me know what you guys think.
Comment From: spring-projects-issues
Juergen Hoeller commented
Turning this into a dedicated ByteBuddy ticket, and putting it into our 5.1 backlog for the time being since we're rather time-bound for 5.0 already: 5.0 RC1 is scheduled for end of March, and aside from completing our reactive story, we still got the mess that is JDK 9 to sort out for good.
Comment From: spring-projects-issues
Rafael Winterhalter commented
Thanks for the feedback and I fully understand. Just ping me if you need anything.
About Java 9: Byte Buddy only needs an ASM6 update and supports Java 9 from there on, all the internals are already laid out.
As for cglib, I added a patch to support Java 9 for cglib: https://github.com/cglib/cglib/commit/d6fe1d8c73508ef30883eb1f9ae965d15953e7d0 The original developer of the library is however difficult to get hold of, therefore, there is no new release yet but since you import, you might want to use the head version anyways.
Comment From: spring-projects-issues
Rob Winch commented
I'm not very familiar with ByteBuddy, but this comment makes me worried about passivity and ensuring that Spring Framework's ByteBuddy works with our dependenies' version of ByteBuddy.
As an anecdote, I want to talk about what I had to do just today when updating dependencies on a project using both Mockito and ByteBuddy.
I had Mockito on a recent enough 2.5.4 and a separate internal library had ByteBuddy on 1.6.0. When I upgrade Mockito to its latest as of today, I end up with Mockito on 2.7.5. ByteBuddy dependency from Mockito is at 1.6.5, however. Additionally, ByteBuddy 1.6.5 breaks public API with 1.6.0 by renaming at least one interface and one method (lest you think this is unusual, ByteBuddy has always broken public API in its patch versions; arguably, that's the root cause). Now suddenly, I have two compile-incompatible versions of the same library when I upgrade from Mockito 2.5.4 to 2.7.5.
I'm not bringing this particular example up to assign blame (I love ByteBuddy to pieces, even with its painful unpromise of public API compatibility; I have accepted that it's a high-maintenance library to keep up with) or to suggest changes to Mockito (though, in retrospect, Mockito can shade ByteBuddy as a private dependency to avoid passing down the pain of keeping up with it). I only wanted to point out that maintaining dependencies can be work, even hard work. Predictable, compatible releases are valuable because they make this work easier.
I'm not sure if passivity has already been considered, but I wanted to bring this up to ensure we think about all angles.
Comment From: spring-projects-issues
Rafael Winterhalter commented
I understand the concern and it is true that I have changed some APIs in Byte Buddy before the library became so popular. With users like Hibernate and Mockito I am more cautious with changing APIs. Sometimes, it is unfortunately not avoidable as Java byte code - unless Java - is not always backwards compatible itself but I think the APIs are isolated appropriately.
As the user with the problems referred to breaking changes in the recent versions, I can only assume that the mentioned internal library relied on Byte Buddy internal APIs or very bleeding-edge features like module redefinition which some people use to make their software run on Java 9 where the APIs change simply because Java 9 is not yet final.
I do not think this should be a big problem for an optional binding like that one planned for Spring.
Comment From: spring-projects-issues
Rafael Winterhalter commented
Just a quick follow-up: if it was possible to refactor the proxy supplier in general, one could consider to generate proxy classes during build time. Byte Buddy's proxy classes are stateless as they are currently implemented such that the runtime could simply locate the classes by their name. These names could for example be added as an annotation value to the proxied classes as a part of the same build operation.
This had the obvious advantage of avoiding class generation during runtime what is expensive. Furthermore, the Byte Buddy dependency would no longer be required during runtime what reduces the footprint of the using application. Finally, this would allow the usage of the proxy mechanism with full AOP capabilities even for native-image generation using Graal or other AOT compilation.
If this was a desired feature, I would happily provide some help creating this implementation. The dynamic proxy generation would however still be helpful, especially during testing where on-demand generation still makes perfect sense.
I created a small POC for this idea: https://github.com/raphw/proxy-example
Comment From: ma-sharifi
Is there any update on this?
Comment From: jhoeller
For the time being, we are going to hang on to our CGLIB fork in the 6.x generation. It does everything we need it do for AOT purposes and for JDK 17+ compatibility, with a pretty lean codebase that we can patch wherever and whenever we need. Supporting a different bytecode generation strategy is a huge maintenance burden that we're currently not ready to take on.
Comment From: Sineaggi
@jhoeller perhaps the Class-File API JEP might be interesting when it stabilizes?