Re: memory leak in org.mortbay.jetty.servlet.ServletHandler?

[Kevan Miller <ke...@gmail.com>]

Hi Greg,
Thanks for the info. The LogFactory's that I saw being leaked were for "
org.apache.geronimo.jetty.JettyClassLoader". I assume it's Geronimo's 
responsibility to clean these up... Geronimo wasn't and 
o.a.g.jetty.JettyWebAppContext was only recently updated to do this (I 
assume that this is the LogFactory.release() that you are referring to...).

Finally, if you are referring to the bug in 
o.m.util.Container.removeComponent(Object) (a bug that I found and David 
Jenks reported), then I can confirm that the problem has been fixed.

On 8/31/05, Greg Wilkins <gr...@mortbay.com> wrote:
> 
> 
> 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
> > <ma...@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()
> >
> >
> 
>