commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 35263] - [beanutils] Memory leak on webapp undeploy in WrapDynaClass
Date Sun, 09 Oct 2005 06:42:08 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=35263>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35263





------- Additional Comments From skitching@apache.org  2005-10-09 08:42 -------
Sorry but I don't believe this patch will resolve the issue.

Just to recap: the problem here is when BeanUtils is deployed in a j2ee
container's classpath, and not the webapp classpath. In this case, when a
WrapDynaClass is created which wraps a class in the webapp classpath, a
reference is stored in a static map on the WrapDynaClass. When the webapp is
undeployed the webapp classloader can't be garbage-collected because there is a
strong reference from a class in the container's classloader to a class in the
webapp's classloader.

Even with the proposed patch, the map will have a strong reference to the
WrapDynaClass object (the "value" in the map, not the key). And the
WrapDynaClass holds a strong reference to the beanClass. So using a string as
the key doesn't break all the strong references from the static
WrapDynaClass.dynaClasses map to the target beanClass. Result: memory leak not
fixed.

I think that if the WrapDynaClass were also updated to use a WeakReference to
point to the beanClass class object that would fix the leak. It wouldn't be a
100% fix, as the WrapDynaClass instances stored in the dynaClass map would never
be deleted, but that leak is *much* less significant than preventing the unload
of the webapp classloader.

There is a theoretical problem with this: if the WrapDynaClass has only a weak
reference to the target class, then potentially the target class could get
garbage collected if there are no other references to it. Given that the
reference is to a Class object, though, I expect that there would always be some
other strong reference to that Class object elsewhere. For example, any instance
of that class, or any Class that subclasses it, or any Class that has a member
of that type will cause a strong reference to be held to the target beanClass.

If you do decide to use WeakReference for WrapDynaClass.beanClass, then rather
than compute a String key to this object, why not just make
WrapDynaClass.dynaClasses a WeakHashMap? This should be a more elegant solution
than the string one.

Problems with all this, though, are that 
 * member beanClass is protected. So changing its type is a binary incompatible
   change to the class, breaking all user subclasses of WrapDynaClass.
 * static member dynaClasses is also protected, so any change to the values
   used as keys for this map is really also a binary incompatible change,
   though "semantic" rather than syntactic.

As is too often the case, protected access to member variables has been used far
too liberally. Providing protected getter/setter methods for these and making
the members private would have allowed this change to be made :-(.

Regards,

Simon

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message