|
From: <jue...@we...> - 2004-05-30 15:10:03
|
Guillaume,
=20
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.
=20
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.
=20
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.
=20
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).
=20
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...
=20
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?=20
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.
=20
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.
=20
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.
=20
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.
=20
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.
=20
Many thanks for pointing this out! :-)
=20
Juergen
=20
________________________________
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=FCrgen h=F6ller [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=3D4291376
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=3D4730581
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=3D4809008
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=3D3149&alloc_id=3D8166&op=3Dclick
_______________________________________________
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_id=8166&op=3Dick
_______________________________________________
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=3D3149&alloc_id=3D8166&op=3Dclick
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|