Ohhh... this isn't actually a leak. The ThreadLocal itself is no longer strongly referenced (the only reference was from the Injector instance).
The problem is that the ThreadLocal class doesn't clean up the underlying map entry until you call set() or remove() on another ThreadLocal instance in the same thread. Once you call set() or remove() a couple times, the ClassLoader should get garbage collected with no problem. The right thing to do here would be for me to use Object[] instead of InternalContext[] ( http://crazybob.org/2006/07/hard-core-java-threadlocal.html). I've actually been working a new implementation of ThreadLocal that cleans up immediately from a background thread (w/o locks): http://code.google.com/p/google-threadlocal/source/browse/trunk/main/java/lang/ThreadLocal.java This is a great use case for justifying my new implementation. Thanks for pointing it out. Bob On Wed, Oct 29, 2008 at 12:51 AM, Stuart McCulloch <[EMAIL PROTECTED]>wrote: > 2008/10/29 Bob Lee <[EMAIL PROTECTED]> > >> Also, to my knowledge, Guice does not have a ThreadLocal leak, despite >> accusations to the contrary in this thread and the Glassfish thread. >> > > I'd like to clarify this if I may... there was a small ThreadLocal leak in > the Guice1 injector: > > > http://fisheye2.atlassian.com/browse/google-guice/trunk/src/com/google/inject/InjectorImpl.java?r=253#l738 > > see where it nulls out the reference... but this still leaves the > ThreadLocal in place which > has a strong reference to Guice internals, this meant that Guice1 couldn't > be completely > unloaded from OSGi containers > > to show this, I've hacked up a basic testcase consisting of two classes > that loads Guice > and a testclass in their own custom classloader, so they can be unloaded > when they are > no longer being used (similar to OSGi and many web-app containers) > > results: > > javac -classpath guice-1.0.jar *.java > > # you can see Guice classes being loaded, but never unloaded > java -classpath "guice-1.0.jar;." -verbose:class Main > > # you can see Guice classes being loaded *and* unloaded > java -classpath "guice-snapshot.jar;." -verbose:class Main > > I believe the GlassFish team were originally testing with Guice1, hence > their statement > about the ThreadLocal (which would show up during heap analysis) however > this issue > is definitely fixed in the current Guice codebase / snapshots > > HTH > > //================================================== > > import com.google.inject.Guice; > import com.google.inject.Injector; > > public class Test { > static class Foo {} > > public static void doInject() { > Injector injector = Guice.createInjector(); > injector.getInstance(Foo.class); > } > } > > //================================================== > > import java.io.File; > import java.lang.reflect.Method; > import java.net.MalformedURLException; > import java.net.URL; > import java.net.URLClassLoader; > > public class Main { > > public static void main(String[] args) throws Exception { > > ClassLoader customLoader = new UnloadingClassLoader(); > Class clazz = customLoader.loadClass("Test"); > Method test = clazz.getDeclaredMethod("doInject", new Class[0]); > > test.invoke(null, new Object[0]); > > test = null; > clazz = null; > customLoader = null; > > while (true) { > try { > System.runFinalization(); > System.gc(); > Thread.sleep(3000); > } catch (InterruptedException ie) {} > } > } > > static class UnloadingClassLoader extends URLClassLoader { > > public UnloadingClassLoader() { > super(new URL[0]); > > String systemPath = System.getProperty("java.class.path"); > String[] classpath = systemPath.split(File.pathSeparator); > for (int i = 0; i < classpath.length; i++) { > try { > addURL(new URL(classpath[i])); > } catch (MalformedURLException e1) { > try { > addURL(new File(classpath[i]).toURI().toURL()); > } catch (MalformedURLException e2) { > throw new RuntimeException(e1); > } > } > } > } > > protected Class loadClass(String name, boolean resolve) > throws ClassNotFoundException { > > synchronized (this) { > Class clazz = findLoadedClass(name); > if (clazz != null) { > return clazz; > } > } > > if (name.startsWith("java.")) { > return super.loadClass(name, resolve); > } else { > Class clazz = findClass(name); > if (resolve) { > resolveClass(clazz); > } > return clazz; > } > } > } > } > > //================================================== > > -- > 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 -~----------~----~----~----~------~----~------~--~---
