Re: [Springframework-developer] Cleanup of context resources on webapp reload

[<jue...@we...>]

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