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 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]> wrote: > 2009/2/7 Gili <[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 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
