I began this discussion thread because in revision 830 Bob committed
changes with a description "Replace ReferenceCache w/ MapMaker". It's
not clear what really changed between ReferenceCache and MapMaker. If
you take a look at MapMaker, FinalizableReferenceQueue and Finalizer it
looks like *some* references (ones that expire) are now cleared without
an active thread, but not all. As we've discussed in that other thread,
it is certainly possible to clear *all* references without an active
thread. The question is whether Bob plans on doing this for Guice 2.0 or
not.
I'd love to hear what Bob has in mind :)
Thanks,
Gili
Stuart McCulloch wrote:
> 2009/2/8 Gili <[email protected] <mailto:[email protected]>>
>
>
> Stuart,
>
> It was my understanding [1] that there was no need for a worker
> thread. I thought that we had established that there were concurrent
> reference map implementations out there that will do a great job
> without it and one of them might even end up in Java7 (all the better
> since we'd end up using a core Java API).
>
> [1] http://code.google.com/p/google-guice/issues/detail?id=288
>
>
> I'll have to defer to Bob, Doug and the other JSR166 experts on this ;)
> [ http://code.google.com/p/google-guice/issues/detail?id=288#c11 ]
>
> a concurrent reference map is imho an important (and useful) feature,
> so I don't mind them taking time in making sure everything works ok
>
> I personally favor any of the solutions you mentioned except for
> running Guice in the system or boot classpath.
>
> Gili
>
> On Feb 8, 7:10 am, Stuart McCulloch <[email protected]
> <mailto:[email protected]>> wrote:
> > 2009/2/7 Gili <[email protected]
> <mailto:[email protected]>>
> >
> > > Hi,
> >
> > > I noticed you guys committed some new code to tackle the memory
> leaks
> > > recently so I gave it a spin. I still see the memory leak under
> > > Glassfish v2ur2 and the GC root that is keeping the instances alive
> > > seems to be com.google.inject.internal.Finalizer.
> >
> > > Stuart, can you please look into whether Glassfish or Guice is at
> > > fault here?
> >
> > Hi Gili,
> >
> > The current reference cache in Guice relies on a background
> worker thread
> > to dequeue and finalize references. Because an active Java thread
> is a GC
> > root this thread must be stopped when it's no longer required,
> otherwise any
> > classes that it refers to (and their classloaders) cannot be
> collected.
> >
> > There are various ways to do this - some libraries expose a
> public API that
> > containers can use to tell the thread when to stop. Others use
> "declarative"
> > metadata (such as the Bundle-Activator manifest entry in OSGi) to
> refer to
> > names of helper classes that can take part in certain managed
> lifecycles.
> >
> > The approach used in Guice (and google-collections) is for the
> thread itself
> > to try and detect when it's no longer required. It does this by
> waiting to
> > see
> > when the reference queue's classloader is eligible for GC, which
> means the
> > client application no longer (strongly) refers to the queue.
> >
> > For this to work properly the worker thread must have no strong
> references
> > to the client application's classloader, otherwise it would
> indirectly keep
> > the
> > queue alive. This is why Bob's code goes to great lengths to try
> and load
> > the worker class in a separate classloader unconnected to the
> application.
> >
> > While this works in most environments, there are two potential
> references
> > that can occur when you spawn a thread from a container managed
> thread:
> >
> > 1) the new thread automatically picks up any InheritedThreadLocals
> >
> > 2) it gets the current AccessControlContext (only when security
> is on)
> >
> > These references are internal to the thread and cannot be removed
> using
> > the public API or other runtime independent methods. So if either
> of these
> > situations occur the worker thread will not stop and you will see
> a "leak".
> >
> > 1) could occur for many reasons, such as an application or
> container not
> > removing its thread locals before returning a thread to the
> common pool.
> >
> > 2) is expected in almost all Java EE environments, hence what you
> see :(
> >
> > So... where do we go from here?
> >
> > Well, you could consider putting Guice on the system or boot
> classpath, ie.
> > tied to some classloader that's above the client classloader. So
> while the
> > worker thread will still have a reference to the system-level
> classloader,
> > it
> > shouldn't stop the client classloader from being unloaded. The
> downside is
> > that you can't replace Guice without restarting the container -
> but this
> > might
> > be acceptable, given you're more likely to replace client apps than
> > libraries
> >
> > [ at the very least you'd need the
> com.google.inject.internal.Finalizer
> > class
> > available on the system / container classpath, you don't need
> all of Guice
> > ]
> >
> > Another option is to remove the need for a worker thread, but I'm
> not sure
> > if
> > this is feasible (there are alternative reference maps out there
> but YMMV :)
> > Note that you'll also have a separate worker thread for each
> application
> > that
> > jarjars the reference queue under a different package ( ie. if
> you use Guice
> > and google-collections you might end up with two worker threads! )
> >
> > Alternatively, the google team could think again about exposing
> an API to
> > shutdown this thread - or possibly provide a BundleActivator
> class that
> > could manage the thread from an OSGi perspective, then you
> wouldn't be
> > exposing a public API but would help anyone using an OSGi
> container* ;)
> >
> > [ * it would introduce a compile dep on OSGi for the library, but not
> > clients ]
> >
> > The last resort would be to patch Guice to shutdown this thread
> and use
> > this in your application rather than the official binary - but
> this may not
> > be
> > an option for everyone ;)
> >
> > Of course once the JDK provides an inbuilt reference map these issues
> > should disappear (and you'd only need one worker thread) - most
> of the
> > problems stem from doing all of this outside of the JDK core classes!
> >
> > HTH
> >
> > Thanks,
> >
> > > Gili
> >
> > --
> > Cheers, Stuart
>
>
>
>
> --
> Cheers, Stuart
>
> >
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"google-guice" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/google-guice?hl=en
-~----------~----~----~----~------~----~------~--~---
