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 28291] - [logging] ContextClassLoader may load Log impl from wrong context in JDK 1.4
Date Fri, 18 Mar 2005 04:51:07 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=28291>.
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=28291





------- Additional Comments From skitching@apache.org  2005-03-18 05:51 -------
(In reply to comment #11)
> When running on Tomcat, the container should call release 
> on the instance loaded in the shared library after each
> classloader is deployed. This is why I made this suggestion.

Ah. So Tomcat has an ugly hack to specifically work around this issue with
commons-logging. Yep, I see it in 
  org.apache.catalina.loader.WebappClassLoader#stop:

  public void stop() throws LifecycleException {
        ....
        org.apache.commons.logging.LogFactory.release(this);


Because this hack exists, the bug I described doesn't occur in Tomcat, ie when
commons-logging.jar is deployed via the shared classloader, memory is correctly
released on undeploy.

Note that the *real* bug exists in commons-logging. A container shouldn't need
to call o.a.c.l.LogFactory.release (and most won't). Tomcat is trying to be nice
to webapps that use commons-logging, and in one sense the call doesn't do any
*harm*. However doing this does make porting to another servlet container
interesting, when a leak suddenly appears..) - and confuses people like me who
know commons-logging and are confused when it *doesn't* leak as expected :-).

A completely portable solution (I think) would be for the deployed app to
register for a callback on undeploy and have the app call LogFactory.release
itself. Class ServletContextListener provides a method  contextDestroyed that
should be able to do this (http://java.sun.com/j2ee/sdk_1.3/techdocs/api/) for
any servlet engine that supports java servlet 2.3. Note that I haven't tried
this myself though.

Just as a matter of style, I wouldn't recommend deploying libs in a "shared"
manner (ie visible to all webapps). This may provide a minor performance
benefit, or reduce memory consumption, but in my view it is just plain ugly, on
the same order as making class member variables public instead of providing
getter and setter methods, just to improve performance. But that's only my
personal opinion, and I know many people disagree.

If LogFactory.release is going to be called on unload, then this should work
fine regardless of whether the LogFactory class was loaded via the shared
classloader (shared/lib) or via the component-specific classloader (WEB-INF/lib).

However Catalin *was* getting a leak when commons-logging.jar was deployed in
WEB-INF/lib. I wonder why (my previous argument no longer applies, as Tomcat
*is* calling LogFactory.release on undeploy).

Catalin, how exactly are you undeploying your webapp?

-- 
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