geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Wilkins <gr...@mortbay.com>
Subject Re: memory leak in org.mortbay.jetty.servlet.ServletHandler?
Date Wed, 31 Aug 2005 18:58:41 GMT

Kevan, 

The 5.1.5rc1 release of Jetty has the LogFactory.release() call for it's
own ClassLoader, so it will definitely help with the jcl memory leaks.

It also contains a patch suggested by David Jencks for another object
leak when context are stopped and Jetty did not release it's own components.

As for the context specific jcl created by Jetty, that will be referenced
by the classloader that loaded jetty, so it is geronimos job to release
the jcl factories for the Jetty if/when it removes Jetty from a running 
geronimo server.   I'm a bit out of touch with geronimo at the moment, 
but I don't think that code will be in o.a.g.jetty.JettyWebAppContext
as the classloader it deals with there will not be the loader that
actaully loaded the Jetty classes themselves.   However the loader
in o.a.g.jetty.JettyWebAppContext should also release jcl factories (and it
does appear to do so).

cheers



Kevan Miller wrote:
> Hi Greg,
> Apologies for the slow response. I'm not sure I'm in sync with you as to
> who should be releasing LogFactory's. I'm not sure what your RC1 update
> will do and what you'd like me to test.
> 
> Seems like Geronimo should be calling LogFactory.release() for the case
> I'm describing. I think Geronimo owns the context/classloader for which
> the LogFactory is being allocated. And thus Geronimo should be calling
> LogFactory.release. Do you agree? I've suggested a change to Geronimo
> (o.a.g.jetty.JettyWebAppContext) that addresses the problem (I may not
> yet have the appropriate scope for the release, but will be resolving this).
> 
> --kevan
> 
> On 8/23/05, *Greg Wilkins* <gregw@mortbay.com
> <mailto:gregw@mortbay.com>> wrote:
> 
> 
>     Kevan,
> 
>     I'm just checking in a version that uses commons logging 1.0.4 and
>     we are now calling release on the context classloader when the context
>     is stopped.
> 
>     HOWEVER
> 
>     I don't think the example you give below is not a leak, at least not
>     from Jetty.  There is no mechanism to release a single log, only the log
>     factories for a classloader.
> 
>     The context log is loaded from the LogFactory of the classloader that
>     loaded Jetty.   So it will be reused over a stop/start of a given
>     context.
>     If Jetty it self is unloaded, then the container that loaded Jetty
>     should release
>     the commons log factory that jetty used and thus release that logger.
> 
>     I'll do an RC1 release shortly and you can test if this is true or not.
> 
>     cheers
> 
> 
>     Kevan Miller wrote:
>     > I've been chasing some Geronimo leaks (described in
>     > http://issues.apache.org/jira/browse/GERONIMO-484)
>     <http://issues.apache.org/jira/browse/GERONIMO-484)>
>     >
>     > I've run across the following problem that seems to be a Jetty issue.
>     > Comments?
>     >
>     > (B) org.apache.commons.logging.LogFactory
>     >   * static field "factories" is holding onto LogFactory objects
>     >   * org.mortbay.jetty.servlet.ServletHandler.doStart() creates new
>     > GeronimoLog instances with the following call:
>     > _contextLog =
>     LogFactory.getLog(getHttpContext().getHttpContextName());
>     >   * however, there is no corresponding LogFactory.release(_contextLog)
>     > in ServletHandler.doStop()
> 
> 


Mime
View raw message