|
From: Guillaume P. <gpo...@gl...> - 2004-05-30 22:26:57
|
Juergen,
I think that Introspector.flushCaches() on a ServletContextListener it still
the lesser of evils for that memory leak, the effect on other webapps would
not be that bad anyway, just slow a little bit the next few calls to
Introspector.getBeanInfo(). However, I agree with you that this should
probably be dealt with by the application rather than Spring. But may be it
could be an option with Spring's ContextLoaderListener (probably off by
default)?
Concerning your solution with CachedIntrospectionResults, I would have a
sugestion for performance. For the majority of the use cases,
CachedIntrospectionResults would be in the same ClassLoader as the target
class. So I would suggest to first check the ClassLoader to know if it's
safe to cache the class without WeakReference. I've attached a patch to the
E-Mail if you would like to include my suggestion.
Thanks,
Guillaume
----- Original Message -----
From: "jürgen höller [werk3AT]" <jue...@we...>
To: <spr...@li...>
Sent: Sunday, May 30, 2004 11:09 AM
Subject: Re: [Springframework-developer] Cleanup of context resources on
webapp reload
Guillaume,
I'm using a profiler, explicitly running garbage collection after
application shutdown and then seeing what remains on the heap. It's actually
my first really instance profiler work for about two years; haven't had any
need for it in the meantime.
Anyway, I did some further tests today, changing SQLErrorCodesFactory and
GlobalAdvisorAdapterRegistry to classic singletons again, with an
Introspector.flushCaches call on shutdown. While this didn't have any effect
with Petclinic, it did result in proper cleanup with Image Database.
So the Introspector.flushCaches call *does* have some effect, contrary to my
assumption from yesterday, but not in all scenarios. After some further
tests, I found out that Hibernate (-> Petclinic) has its own leaks, thus
keeps the web app class loader around, thus no cleanup of Spring's
singletons in that class loader. If Hibernate isn't used, everything gets
cleaned up properly.
This means that you're perfectly right: Coding singletons with
WeakReferences only cleans up that particular singleton object but of course
doesn't do anything about the class loader itself with its remaining leaks.
As the code *is* uglier with WeakReferences, I've permanently changed
SQLErrorCodesFactory and GlobalAdvisorAdapterRegistry back to classic
singletons (like they were until a week ago).
I'd like to leave CachedIntrospectionResults as-is with a WeakHashMap and
WeakReferences, as this holds references to classes and might reside in a
parent classloader of the referenced classes. That's how the JDK's
Introspector class should be coded too...
The remaining issue is: Where to invoke Introspector.flushCaches? Every
ApplicationContext shutdown? How to avoid cleaning all BeanInfos *of the
entire VM*, when all we want to do is clean up all BeanInfos of the current
app?
Sure, a general flushCaches call cleans up all BeanInfo leaks of the app
too, be it from Spring or Quartz or whatever, but all we want to provide
out-of-the-box is proper cleanup of Spring itself.
So my solution for Spring is simple: CachedIntrospectionResults flushes the
Introspector cache for the given class (and its superclasses) right after it
fetched the BeanInfo for it (i.e. right after it's been cached). As we cache
the introspection results ourselves anyway, we wouldn't benefit from the
Introspector cache in the first place. That shouldn't have any negative
impact on performance.
I did quite a lot of tests with this new strategy, and everything gets
cleaned up nicely within Spring. Just if you add Hibernate or Quartz to the
mix, you'll get leaks from them that prevent the class loader from getting
garbage collected. With Quartz, a general flushCaches call helps; with
Hibernate, even that doesn't.
So in certain scenarios, it might help to add a custom
ServletContextListener that does an Introspector.flushCaches call on
shutdown - but for Spring itself, this isn't necessary, as the beans
infrastructure does not cause any leaks anymore. I'm happy with the current
solution: classic singletons as before, just a flushFromCaches call for each
introspected class right when building the CachedIntrospectionResults
object.
We should probably tell the Quartz guys to apply specific flushFromCaches
calls after their Introspector usage. I've also noticed that JDOM, as used
within iBATIS SQL Maps 1.3, has a resource leak too. I haven't researched
where the Hibernate leaks come from in detail, but the root of the problem
seems to be CGLIB there.
Many thanks for pointing this out! :-)
Juergen
________________________________
Von: spr...@li... im Auftrag von
Guillaume Poirier
Gesendet: So 30.05.2004 07:01
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on
webapp reload
Juergen, I'm curious about how exactly you assert what is leaking, what is
not, and exactly what is it better with WeakReference? Are you using a
profiler that tells you the non-garbage collected objects in the JVM or do
you use other means?
Concerning SQLErrorCodesFactory, while the class' constructor does cause a
leak (indirectly by the use of XmlBeanFactory), it is not itself the leak,
it's the class instance of SQLErrorCodes that prevent the ClassLoader's
collection. I did a test calling
SQLErrorCodesFactory.getInstance().getErrorCodes("DB2") in a child
ClassLoader, and it caused a leak when the ClassLoader is thrown away. Then
I tried to create my own SQLErrorCodesFactory implementation, and it kept
leaking until I stopped using the XmlBeanFactory to create the instances of
SQLErrorCodes. Then I tried to just use an Introspector directly to set the
properties on the SQLErrorCodes instances by reflection, and it leaked just
as the SQLErrorCodesFactory. You have to keep in mind that if e.g.
Introspector has an hard reference on SQLErrorCodes.class, then none of the
class loaded by its ClassLoader can be collected. So it would be the cause
of SQLErrorCodesFactory singleton to be kept alive, not because
SQLFactoryErrorCodes has an hard reference on it, but because the
ClassLoader does, and SQLErrorCodes.class has an hard reference on the
ClassLoader, and the Introspector has an hard reference on
SQLErrorCodes.class. So the singleton of SQLFactoryCodesFactory is really
kept alive because Introspector has an hard reference on the class instance
of SQLErrorCodes.
I might misunderstand the situation, but as I see it, you're working on the
symptom rather than the cause. If you allow the singleton to be garbage
collected when the class itself is retained, then you might save some
memory, but it's just like adding memory to the JVM to solve a leak, it will
only delay the OutOfMemoryError, it won't prevent it. However, if you allow
the ClassLoader to be collected by removing any hard reference to its
classes, that would prevent the leak to even take place at all.
That's why I'm curious as to why you say it's better with WeakReference on
singleton and Introspector.flushCaches() does nothing, how do you make that
assertion? I realize that if there's something else than Introspector that
has an hard reference on a class of the ClassLoader being thown away,
flushing the Introspector's cache will have no effect on the leak, it would
still leak as fast. But while the WeakReference on the singleton will delay
the OutOfMemoryError, does it really help that much, since anyway all the
Class definitions and static members cannot be collected? You have to
consider that coding defensively on this might reduce performance because of
more object creation and use of synchronization, while also complicating the
code. Is it really worth it, did your tests really showed a significant
effect on a typical application?
Guillaume
----- Original Message -----
From: "jürgen höller [werk3AT]" <jue...@we...>
To: <spr...@li...>
Sent: Saturday, May 29, 2004 2:28 PM
Subject: Re: [Springframework-developer] Cleanup of context resources on
webapp reload
Guillaume,
I've prototypically added an Introspector.flushCaches call to context
shutdown: I don't see any difference in the profiler. That's not too
surprising, as the originally leaking classes are not managed by beans
facilities in the first place: for example, SQLErrorCodesFactory, which is
just used internally by SQLErrorCodeSQLExceptionTranslator.
Of course, since my changes from a week ago, those classes don't leak
anymore, as they hold their singleton instance in a WeakReference now... I
still don't understand why this is necessary, but I'm 100% sure that it does
make a difference on both Sun JDK 1.4.2 and Sun JDK 1.3.1. I've also tried
various GC configuration options - always the same effect.
Juergen
________________________________
Von: spr...@li... im Auftrag von
Guillaume Poirier
Gesendet: Sa 29.05.2004 06:12
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on
webapp reload
I experimented some more about this cleanup issue, and I was able to narrow
down the problem to the caching done by the java.beans.Introspector. It
stores the BeanInfo instances in a WeakHashMap, but in that Map
implementation, only the keys uses WeakReference, the values are stored with
hard references since BeanInfo has an hard reference on the class it gives
info about (indirectly through BeanDescriptor and others), any time
Introspector.getBeanInfo(Class) is used, that class and it's static members
will not ever be able to be garbage collected. I kind of remember someone
mentioning something related to this in the mailling list, but I cannot find
the mail. I wonder if there's other case where the java[x] classes might
have an hard reference on a class or its instances. A fix for this
particular problem is to have a ServletContextListener call
Introspector.flushCaches() when the context is destroyed.
It seems like a known issue at Sun :
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4291376
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4730581
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4809008
So, unless I'm missing something here, that means fixes like using
synchronization and WeakReference on singleton probably won't help much if
at all. The only way that I can see for a class not to be gargage collected
when no more active thread use it, is if another ClassLoader has an hard
reference to the class instance, or an instance of that class. Having the
class itself have an hard reference on a its own singleton has no effect,
it's a circular reference that will not prevent the class or the instance to
be gargabe collected when neither is being refered to by something else.
Guillaume
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id149&alloc_id66&op=ick
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id149&alloc_id66&op=ick
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|