|
[
Permalink
| « Hide
]
Max Rydahl Andersen added a comment - 09/Mar/07 12:08 PM
testcase + patch ?
Here is a diff of the fix for this issue as well as a test case.
Just to give an idea of how potentially bad this leak was, making this change had a dramatic impact on the the memory use and performance of our servlet based web application. Before, we had to max out the Xmx of our servlets at 2gb, and even still we'd get OOM errors after about 6 hours under heavy stress testing. Now they run fine at 500mb. This has also translated to some astonishing performance gains in the overall performance of our app. I'd be very interested to see if other people have a similar experience. For those who want to fix this issue without waiting for this fix, or making a custom build of hibernate, here is a workaround you can implement in your own code. This code should go after you create a Configuration object and before you create the SessionFactory:
{code:title=Bar.java|borderStyle=solid} //assumes an instantiated Configuration variable called cfg cfg.setListeners("load", new LoadEventListener [] {new DefaultLoadEventListener(), new LoadEventListener() { public void onLoad(LoadEvent event, LoadType loadType) throws HibernateException { Object obj = event.getResult(); if (obj instanceof HibernateProxy) { Enhancer.registerCallbacks(obj.getClass(),null); } } }}); {code} thanks for the patch and related testcase; but a more interesting testcase would be a test that actually showed the OOME. Are you saying that simply creating and closing a SessionFactory in multiple threads will do it ?
And i'm a bit troubled on how setting the callback will actually affect future instantiation. Scott, you have had fun with this before - could you verify/fix it ?
The test case demonstraits that during the course of normal operation, a fully hydrated object managed by hibernate is stuck in static memory. Creating any proxy object on any given thread will cause this behavior. How much memory this leaks, and weather or not you get an OOM is dependent on:
1) How many proxy types your application has. 2) How many threads you create proxies on. 3) Most importantly, how large the object graph is that is attached to these proxy objects. To create a test case that shows the OOM is very easy, but coming up with a test case that actually asserts if there is an imminent OOM is next to impossible in Java. But the OOM is just a symptom of the real issue which is hydrated objects stuck in memory. This alone should be a giant red flag. As far as future instantiations, there should be no worry here. Any time a proxy is ever instantiated, it is always preceded by the Enhancer.registerCallbacks() method. In fact, the callback object is directly tied to the instance of the proxy being created so if this callback object was ever used by future instantiations, that would be a very bad thing. Paul,
Thanks for submitting the problem, fix and test. Scott My next step is to evaluate whether we could fix the root issue differently (or work around it), that being
In your example above, is the potential of the leak really that high? I wouldn't expect 15,000 objects leaked. Correct me if I'm wrong, but I would expect one TLS slot per thread to hold a reference. If you have 100 threads, that is a max of 100 objects leaked. Unless a collection is stored in each TLS slot and then the potential is higher. Either way, we should fix the leak. http://forum.hibernate.org/viewtopic.php?t=947902 seems to discuss this issue as well. Yep, that thread discusses this issue exactly.
I'm afraid the leak is as large as I stated. The static thread local is created on *each* proxy type. So if my application has a Customer class and a Order class, each type has their own static thread local, and the last proxy object created of each type on each thread will be leaked. RE: fixing the issue differently. That occurred to me too. I though you might just be able to call one of the Enhancer.create() methods which let you pass in the callback directly, but I figured there must have been a reason why that wasn't chosen (sounds like I was right) so I decided to respect the original design. But one little extra call to clear the thread local should be a pretty minimal impact. Fix checked into 3_2 branch and trunk.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Resolved
Critical
Big memory leak in the use of CGLIB
