Menu
▾
▴
springframework-developer
|
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
|
|
From: Guillaume P. <gpo...@gl...> - 2004-05-30 22:26:57
Attachments:
patch.txt
|
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
|
|
From: <jue...@we...> - 2004-05-31 11:09:20
|
Guillaume,
=20
As far as I see, the only benefit that we would gain with such a =
classloader check is that we avoid recaching introspection results =
during application runtime, for the case where the garbage collector has =
eagerly removed them. However, the latter can only happen when there are =
no other references to the given Class: This won't be the case with =
typical Spring usage, as both bean factories and web command controllers =
(i.e. the main usages of BeanWrapper) hold a reference to the bean class =
respectively command class.
=20
And if there are no other references to a specific Class, I suppose the =
garbage collector is free to remove the cached introspection results for =
it; this can be considered a desirable thing. So I guess it's better to =
leave CachedIntrospectionResults as it is, as there's no noteworthy =
difference in all other respects. Do you see any negative impact on =
performance, given the scenario from above? Your earlier point comes to =
my mind here: It's also about a simple implementation that doesn't =
obscure the purpose of the code.
=20
Juergen
=20
________________________________
Von: spr...@li... im Auftrag =
von Guillaume Poirier
Gesendet: Mo 31.05.2004 00:26
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on =
webapp reload
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=FCrgen h=F6ller [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=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
-------------------------------------------------------
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
|
|
From: <jue...@we...> - 2004-05-31 12:23:12
|
I've just found out that our JPetStore's remaining resource leak is not =
caused by iBATIS or JDOM but by Struts: Struts uses Commons BeanUtils =
which in turn uses the JavaBeans Introspector without flushing... So on =
web app shutdown, the Introspector cache holds references to Struts =
Actions from the web app classloder. This prevents garbage collection of =
JDOM's singletons because the web app classloader (which holds its =
loaded classes and thus their static references) is still around. So =
it's not JDOM's fault; it's rather like with Spring's =
SQLErrorCodesFactory and co.
=20
Which gives 2 candidates that would benefit from a general =
Introspector.flushCaches call on shutdown so far: Struts and Quartz. As =
per my mail from yesterday, Spring does not need that anymore as it =
flushes the JavaBeans Introspector for each class that it caches =
introspection results for itself. If you still see Spring classes =
hanging around after web app shutdown, that's caused by leaks of *other* =
tools that prevent the class loader (which is referencing Spring =
classes) from being disposed.
=20
So the remaining question is: Should we provide a way to call =
Introspector.flushCaches on web app shutdown? Doing this in =
ContextLoaderListener will just affect apps that actually use =
ContextLoaderListener: What about DispatcherServlet-only apps? What =
about Struts apps that load a Spring context via a plugin? What about =
Servlet 2.2 apps that can't register listeners? I guess it would be =
better to provide separate IntrospectorCleanupListener and =
IntrospectorCleanupServlet classes (analogous to Log4jConfigListener and =
Log4jConfigServlet), just doing a Introspector.flushCaches call on =
shutdown.
=20
Of course, it would be ideal if the JavaBeans Introspector used a =
WeakHashMap with WeakReference values - none of this would be necessary =
then. After all, it's a pretty obvious bug in the current JDK: What's =
the point in using a WeakHashMap with values that reference the key but =
are not held in WeakReferences? If they wouldn't care about garbage =
collection in the first place, they could use a plain HashMap... =
Currently, they seem to go just half the way.
=20
With the current JDK implementation, it would also be nice if the =
servlet container did an Introspector.flushCaches call on web app =
shutdown, or preferably a flushFromCaches call for each class loader by =
the corresponding class loader. However, it's unrealistic that all =
containers will do this any time soon - and this wouldn't be necessary =
either if that d*** JavaBeans Introspector implementation properly used =
WeakReferences.
=20
So I guess I'll add IntrospectorCleanupListener and =
IntrospectorCleanupServlet classes... thoughts?
=20
Juergen
=20
________________________________
Von: spr...@li... im Auftrag =
von j=FCrgen h=F6ller [werk3AT]
Gesendet: Mo 31.05.2004 13:08
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on =
webapp reload
Guillaume,
As far as I see, the only benefit that we would gain with such a =
classloader check is that we avoid recaching introspection results =
during application runtime, for the case where the garbage collector has =
eagerly removed them. However, the latter can only happen when there are =
no other references to the given Class: This won't be the case with =
typical Spring usage, as both bean factories and web command controllers =
(i.e. the main usages of BeanWrapper) hold a reference to the bean class =
respectively command class.
And if there are no other references to a specific Class, I suppose the =
garbage collector is free to remove the cached introspection results for =
it; this can be considered a desirable thing. So I guess it's better to =
leave CachedIntrospectionResults as it is, as there's no noteworthy =
difference in all other respects. Do you see any negative impact on =
performance, given the scenario from above? Your earlier point comes to =
my mind here: It's also about a simple implementation that doesn't =
obscure the purpose of the code.
Juergen
________________________________
Von: spr...@li... im Auftrag =
von Guillaume Poirier
Gesendet: Mo 31.05.2004 00:26
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on =
webapp reload
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=FCrgen h=F6ller [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=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
-------------------------------------------------------
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_id149&alloc_id=8166&op=3Dick
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|
|
From: Colin S. <col...@ex...> - 2004-05-31 13:53:25
|
The IntrospectorCleanupListener and IntrospectorCleanupServlet sound=20
reasonable. At the end of the day, as has been discussed, this is=20
probably going to be of most use to people in development; I'm not sure=20
how many people trust hot-deploy in production. Certainly I would never=20
do it with something like JBoss. The only option I am pretty strongly=20
opposed to is to have the default behaviour to be to call flushCaches on=20
shutdown, since it can affect so many other apps in the container; I=20
realize you don't think this is a good default either .
I should mention Juergen that your changes appear to have had an=20
interesting effect on hot-redeploy of my main app in JBoss. As of about=20
maybe 1.5 months ago, hot redeploy in JBoss stopped working for me, with=20
JBoss complaining on reload about ClassCastExceptions on a number of=20
classes. As of about a week ago, it's working again. I never really had=20
time to track down why the hot-redeploy stopped working (the JBoss=20
unified classloader is a fairly random, unpredictable beast, especially=20
with EJBs involved), but the timing with it working again now makes me=20
think that the recent changes are responsible for it working again...
Regards,
Colin
j=FCrgen h=F6ller [werk3AT] wrote:
>I've just found out that our JPetStore's remaining resource leak is not =
caused by iBATIS or JDOM but by Struts: Struts uses Commons BeanUtils whi=
ch in turn uses the JavaBeans Introspector without flushing... So on web =
app shutdown, the Introspector cache holds references to Struts Actions f=
rom the web app classloder. This prevents garbage collection of JDOM's si=
ngletons because the web app classloader (which holds its loaded classes =
and thus their static references) is still around. So it's not JDOM's fau=
lt; it's rather like with Spring's SQLErrorCodesFactory and co.
>=20
>Which gives 2 candidates that would benefit from a general Introspector.=
flushCaches call on shutdown so far: Struts and Quartz. As per my mail fr=
om yesterday, Spring does not need that anymore as it flushes the JavaBea=
ns Introspector for each class that it caches introspection results for i=
tself. If you still see Spring classes hanging around after web app shutd=
own, that's caused by leaks of *other* tools that prevent the class loade=
r (which is referencing Spring classes) from being disposed.
>=20
>So the remaining question is: Should we provide a way to call Introspect=
or.flushCaches on web app shutdown? Doing this in ContextLoaderListener w=
ill just affect apps that actually use ContextLoaderListener: What about =
DispatcherServlet-only apps? What about Struts apps that load a Spring co=
ntext via a plugin? What about Servlet 2.2 apps that can't register liste=
ners? I guess it would be better to provide separate IntrospectorCleanupL=
istener and IntrospectorCleanupServlet classes (analogous to Log4jConfigL=
istener and Log4jConfigServlet), just doing a Introspector.flushCaches ca=
ll on shutdown.
>=20
>Of course, it would be ideal if the JavaBeans Introspector used a WeakHa=
shMap with WeakReference values - none of this would be necessary then. A=
fter all, it's a pretty obvious bug in the current JDK: What's the point =
in using a WeakHashMap with values that reference the key but are not hel=
d in WeakReferences? If they wouldn't care about garbage collection in th=
e first place, they could use a plain HashMap... Currently, they seem to =
go just half the way.
>=20
>With the current JDK implementation, it would also be nice if the servle=
t container did an Introspector.flushCaches call on web app shutdown, or =
preferably a flushFromCaches call for each class loader by the correspond=
ing class loader. However, it's unrealistic that all containers will do t=
his any time soon - and this wouldn't be necessary either if that d*** Ja=
vaBeans Introspector implementation properly used WeakReferences.
>=20
>So I guess I'll add IntrospectorCleanupListener and IntrospectorCleanupS=
ervlet classes... thoughts?
>=20
>Juergen
>=20
>
>________________________________
>
>Von: spr...@li... im Auftrag vo=
n j=FCrgen h=F6ller [werk3AT]
>Gesendet: Mo 31.05.2004 13:08
>An: spr...@li...
>Betreff: Re: [Springframework-developer] Cleanup of context resources on=
webapp reload
>
>
>
>Guillaume,
>
>As far as I see, the only benefit that we would gain with such a classlo=
ader check is that we avoid recaching introspection results during applic=
ation runtime, for the case where the garbage collector has eagerly remov=
ed them. However, the latter can only happen when there are no other refe=
rences to the given Class: This won't be the case with typical Spring usa=
ge, as both bean factories and web command controllers (i.e. the main usa=
ges of BeanWrapper) hold a reference to the bean class respectively comma=
nd class.
>
>And if there are no other references to a specific Class, I suppose the =
garbage collector is free to remove the cached introspection results for =
it; this can be considered a desirable thing. So I guess it's better to l=
eave CachedIntrospectionResults as it is, as there's no noteworthy differ=
ence in all other respects. Do you see any negative impact on performance=
, given the scenario from above? Your earlier point comes to my mind here=
: It's also about a simple implementation that doesn't obscure the purpos=
e of the code.
>
>Juergen
>
>
>________________________________
>
>Von: spr...@li... im Auftrag vo=
n Guillaume Poirier
>Gesendet: Mo 31.05.2004 00:26
>An: spr...@li...
>Betreff: Re: [Springframework-developer] Cleanup of context resources on=
webapp reload
>
>
>
>Juergen,
>
>I think that Introspector.flushCaches() on a ServletContextListener it s=
till
>the lesser of evils for that memory leak, the effect on other webapps wo=
uld
>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 b=
e 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 targe=
t
>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=FCrgen h=F6ller [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 actu=
ally
>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 an=
d
>GlobalAdvisorAdapterRegistry to classic singletons again, with an
>Introspector.flushCaches call on shutdown. While this didn't have any ef=
fect
>with Petclinic, it did result in proper cleanup with Image Database.
>
>So the Introspector.flushCaches call *does* have some effect, contrary t=
o 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 get=
s
>cleaned up properly.
>
>This means that you're perfectly right: Coding singletons with
>WeakReferences only cleans up that particular singleton object but of co=
urse
>doesn't do anything about the class loader itself with its remaining lea=
ks.
>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 an=
d
>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 curr=
ent
>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 afte=
r it
>fetched the BeanInfo for it (i.e. right after it's been cached). As we c=
ache
>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 getti=
ng
>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 curr=
ent
>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 flushFromCache=
s
>calls after their Introspector usage. I've also noticed that JDOM, as us=
ed
>within iBATIS SQL Maps 1.3, has a resource leak too. I haven't researche=
d
>where the Hibernate leaks come from in detail, but the root of the probl=
em
>seems to be CGLIB there.
>
>Many thanks for pointing this out! :-)
>
>Juergen
>
>
>________________________________
>
>Von: spr...@li... im Auftrag vo=
n
>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 lea=
k,
>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 kep=
t
>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 j=
ust
>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 ca=
use
>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 real=
ly
>kept alive because Introspector has an hard reference on the class insta=
nce
>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 a=
llow
>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 t=
hat
>assertion? I realize that if there's something else than Introspector t=
hat
>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 wo=
uld
>still leak as fast. But while the WeakReference on the singleton will d=
elay
>the OutOfMemoryError, does it really help that much, since anyway all th=
e
>Class definitions and static members cannot be collected? You have to
>consider that coding defensively on this might reduce performance becaus=
e 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 tri=
ed
>various GC configuration options - always the same effect.
>
>Juergen
>
>
>________________________________
>
>Von: spr...@li... im Auftrag vo=
n
>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 nar=
row
>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 giv=
es
>info about (indirectly through BeanDescriptor and others), any time
>Introspector.getBeanInfo(Class) is used, that class and it's static memb=
ers
>will not ever be able to be garbage collected. I kind of remember someo=
ne
>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 migh=
t
>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 colle=
cted
>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 t=
he
>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 instanc=
e to
>be gargabe collected when neither is being refered to by something else.
>
>Guillaume
> =20
>
|
|
From: <jue...@we...> - 2004-06-01 01:48:39
|
I've just done several tests, and you're right. Garbage collections will =
quickly throw away the CachedIntrospectionResults objects, as it's not =
held in a strong reference anywhere else - just the class which is the =
key in the cache is, but that doesn't affect the WeakReference value... =
So I've just adopted your patch: keeping strong references if in a =
cache-safe classloader, using weak references else.
=20
I see why the JavaBeans Introspector was implemented that way... It's =
indeed not that simple to be both performant and ready for garbage =
collection. The JDK 1.5 solution sounds like a final solution for this =
issue. Up until then, we'll have to flush the Introspector cache =
accordingly. Our final solution of specific flushFromCaches calls and =
the strong/weak reference combo that results from your patch should =
represent the best compromise.
=20
Anyway, I'm too tired to upload the release now... it's already 3:45 AM =
here. Once again, we've managed to find yet another issue that delays =
for a day - at least it's already solved this time. I'll be really happy =
when that release is finally out... As I have an external workshop =
during the day, the actual release will happen tonight (in about 16 =
hours). Please, no crucial issues anymore ;-)
=20
Juergen
=20
________________________________
Von: spr...@li... im Auftrag =
von Guillaume Poirier
Gesendet: Di 01.06.2004 00:58
An: spr...@li...
Betreff: Re: [Springframework-developer] Cleanup of context resources on =
webapp reload
Juergen,
I'm not sure I'm following you on this one, as far I as can tell,
CachedIntrospectionResults.forClass(Class) is only called in
BeanWrapperImpl, and the result is only kept in the BeanWrapper was =
instance
variable. So anytime a BeanWrapperImpl instance is collected, its
CachedIntrospectionResults would get collected too. Both the factories =
the
web controllers hold references only on the class, not the
CachedIntrospectionResults for that class, don't they? Holding a strong
reference on the former won't prevent garbage collecting on the latter. =
So
that means, most of the time when the garbage collector runs, you would =
need
to create the CachedIntrospectionResults again for any binding of
command/form in a web controller, creation of prototype beans in the
factory, use of BeanUtils.copyProperties(), direct use of =
BeanWrapperImpl,
etc.. I'm mostly guessing here, but I would think the performance hit =
could
be potentially significant.
But reading your post makes me think of something that I think would be =
the
best solution for those kind of memory leaks, except it would need to be
implemented within the JDK. The best would be to have kind of an =
equivalent
of ThreadLocal but for class. Have a HashMap in java.lang.Class, and =
e.g.
ClassReference (or a better name) holding a WeakReference on the target, =
and
placing the strong reference in the java.lang.Class's HashMap. May be =
I'll take a look at sun's bug list and suggest it. It looks like =
something that would work, doesn't it?
Guillaume
---- Messages d=B4origine ----
De: j=FCrgen h=F6ller [werk3AT] <jue...@we...>
Date: lundi, mai 31, 2004 7:08 am
Objet: Re: [Springframework-developer] Cleanup of context resources on =
webapp reload
> Guillaume,
>
> As far as I see, the only benefit that we would gain with such a
> classloader check is that we avoid recaching introspection results
> during application runtime, for the case where the garbage
> collector has eagerly removed them. However, the latter can only
> happen when there are no other references to the given Class: This
> won't be the case with typical Spring usage, as both bean
> factories and web command controllers (i.e. the main usages of
> BeanWrapper) hold a reference to the bean class respectively
> command class.
>
> And if there are no other references to a specific Class, I
> suppose the garbage collector is free to remove the cached
> introspection results for it; this can be considered a desirable
> thing. So I guess it's better to leave CachedIntrospectionResults
> as it is, as there's no noteworthy difference in all other
> respects. Do you see any negative impact on performance, given the
> scenario from above? Your earlier point comes to my mind here:
> It's also about a simple implementation that doesn't obscure the
> purpose of the code.
>
> Juergen
>
>
> ________________________________
>
> Von: spr...@li... im
> Auftrag von Guillaume Poirier
> Gesendet: Mo 31.05.2004 00:26
> An: spr...@li...
> Betreff: Re: [Springframework-developer] Cleanup of context
> resources on webapp reload
>
>
>
> 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
> shouldprobably 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
> targetclass. 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=FCrgen h=F6ller [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
> furthertests, 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?
> EveryApplicationContext 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
> provideout-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
> negativeimpact 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
> gettinggarbage 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
> flushFromCachescalls 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
> researchedwhere 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'scollection. 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
> instanceof 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
> garbagecollected 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
> significanteffect 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
> someonementioning 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
> collectedwhen no more active thread use it, is if another
> ClassLoader has an hard
> reference to the class instance, or an instance of that class.=20
> 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?66&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
>
>
>
>
> -------------------------------------------------------
> 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?66&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_id149&alloc_id?66&op=D5ick
> _______________________________________________
> 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
|
|
From: Seth L. <se...@eh...> - 2004-05-24 17:57:29
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 jürgen höller [werk3AT] wrote: | So basically, all static caches cause resource leaks when restarting a Tomcat web app? I wonder why this happens... The class loader should completely dissolve all classes that it has loaded in its lifetime, including static caches. Or have I misunderstood something here? Anyone having in-detail experience with handling such a scenario? | That has been my experience. I've noticed the static init blocks from BeanWrapperImpl never leaving scope after a webapp uninstall. Again, an easy way to verify this is to just redeploy the spring-minimal.war over and over. This will generate a OOM. Seth -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.3-nr1 (Windows XP) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFAsjfxKZsFSwtW+wIRAiRTAJ9CBVRbheW6McGOLKbunWEyXeESZwCfU4sO tzdNXKEXcoYjFUuLxkjX/Ig= =zu5p -----END PGP SIGNATURE----- |
