Hi Adrian, Thanks very much for your response.
On Wed, 2005-07-27 at 11:46 -0400, Adrian Brock wrote: > Static collections are the root of all memory leak brain deaths. Agreed. And if java provided the ability to attach arbitrary data to a classloader then LogFactory would use this instead of having a static map keyed by classloader and the problem would go away. But it doesn't [yet]. Or if j2ee provided some mechanism for an arbitrary library deployed in the container classpath to associate data with a component then that would also be a viable solution and LogFactory wouldn't have to use a static Map. But neither of these features exist as far as I know. In other words, we're aware of the problem but there doesn't seem to be any way around using that ugly static map when people have deployed commons-logging in the container's classpath instead of the component's. Suggestions for alternative solutions are *very* welcome. > > JBoss now uses a patched version of commons-logging > http://anoncvs.forge.jboss.com:8080/changelog/JBoss?cs=MAIN:adrian:20050609001010 > to fix the memory leaks. The same change was made to commons-logging's trunk in Nov 2004, ie to change the LogFactory.factories map to use weak references for its keys. [NB: this is only activated when commons-logging-optional.jar is in the classpath as commons-logging supports java 1.1 which doesn't have weakrefs]. Unfortunately the use of weak references to the map keys doesn't fix the issue completely. If the logging adapter class is retrieved from the TCCL then there is a chain: * strong ref from LogFactory.class to its factories map * strong ref from a map entry to the LogFactory subclass * strong ref from the LogFactory to the adapter class * strong ref from the adapter class to the classloader which loaded it In this case, using weak refs to the *keys* in the LogFactory.factories doesn't prevent the leak. > > JBoss also has its own trivial logging abstraction (factory + wrapper) > that does not have these problems and allows other logging targets, > e.g. JDK logging see the bottom of > http://wiki.jboss.org/wiki/Wiki.jsp?page=Logging Interesting approach. I personally don't like this for several reasons: * The logging configuration is stored in a container-level file. That means that someone needs container administration access in order to change the logging configuration for a component, yes? I think a component's logging config should be modifiable by the component's administrator. * As the page indicates, there is a problem when multiple components happen to use the same logging categories. Deploying separate test and uat instances must be interesting (or having multiple developers trying to deploy their personal versions to the same container). The workaround is to define separate Appender objects for each alternate deployment URL, and filters to then ensure that the component only logs to its associated appender(s). This would seem to work, but it isn't very elegant or efficient is it? * the logging configuration can't reference any custom Level classes or Appender classes or Layout classes defined in the component. If this were to be allowed, then you would have bidirectional references from the *static* logging info in log4j back into the components which would block undeploy. However if components were just to include log4j.jar in their WEB-INF/lib directory you wouldn't need to go to all these complicated lengths to work around the problem of having a shared logging installation; proper per-component logging would just work. Wouldn't that be easier? > > Your whole argument is just wrong. If commons-logging is really > ready for the big time, it should be useable from infrastructure > software that has to deal with mulitple application contexts > and talk amongst themselves in a dynamic environment. Is "application context" the same as the term "component" that I use? Are you talking here about problems that occur when one component calls directly into another component? If so, I agree that there is currently a problem with this in commons-logging: it can produce class cast exceptions. However this is a different issue. In fact, I was not aware until recently that components could obtain references to objects in other components, and then call them without having the TCCL reset. Possibly earlier commons-logging maintainers knew this but if they did they didn't document it ;-(. Now that this is known it will be addressed. By "infrastructure software" are you talking about situations where the container is effectively extended by putting libraries into the container's classpath? The wiki page addresses this; that is entirely allowable but in this case the container-extension is expected to act as a container-extension, not just a random library, and actively manage commons-logging by ensuring LogFactory.release is called on undeploy. Is there a specific problem with this? > Saying you can fix all the problems by isolating the classloading > or using deployment lifecycle is just stupid. What if every > thirdparty library required this? It would be an unmaintainable mess. Well, my whole point is that in general thirdparty libraries should *not* be dropped into the container's classpath. Only "container extensions" should go into the container's classpath. And container extensions should be pretty rare. I feel that it is simply unacceptable to drop thirdparty libraries that have never expected to be "container extensions" into the container's classpath? Any use of static variables by that thirdparty library is likely to be highly problematic as such statics will be shared across components. And if the thirdparty library is not threadsafe then again there can be *very* nasty issues when it is deployed into the container classpath. So it seems to me that only libs which are explicitly designed to be used as a "container extension" should be deployed in that way - all others should be deployed at the component level. > > You are correct, java.sql.DriverManager is also a brain death. > In fact, it is even worse because of the crazy classloading > rules it uses to avoid security problems in applets. :-(. > > On Wed, 2005-07-27 at 07:16, Simon Kitching wrote: > > Hi, > > > > I was recently informed of this thread about memory leaks occurring on > > undeploy with hibernate and saw a fair bit of criticism on > > commons-logging. > > > > I have written a counter-argument which is available here: > > http://wiki.apache.org/jakarta-commons/Logging/UndeployMemoryLeak > > > > I hope you find this information useful. All feedback on this is very > > welcome. > > > > Regards, > > > > Simon > > > > > > > > > > ------------------------------------------------------- > > SF.Net email is sponsored by: Discover Easy Linux Migration Strategies > > from IBM. Find simple to follow Roadmaps, straightforward articles, > > informative Webcasts and more! Get everything you need to get up to > > speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click > > _______________________________________________ > > hibernate-devel mailing list > > hibernate-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/hibernate-devel ------------------------------------------------------- SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf _______________________________________________ hibernate-devel mailing list hibernate-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/hibernate-devel
