Closed GoogleCodeExporter closed 8 years ago
Thanks very much! Bob has agreed to fix this issue. He's decided not to use
your
patch, and will fix it a slightly different way, but thanks for going to the
extra
effort anyway.
Original comment by kevin...@gmail.com
on 28 Jul 2008 at 7:00
A fix is a fix is a fix... :) There is always more then one way - I just took
the
fastest and least complicated way I could think of.
Thanks.
Original comment by nairb...@gmail.com
on 28 Jul 2008 at 7:43
FYI: I just tested the new version of FinalizableReferenceQueue with Guice on
an OSGi
container, and I can confirm it now allows the classloader to be garbage
collected :)
Original comment by mccu...@gmail.com
on 26 Nov 2008 at 5:24
This isn't solved for me (under Glassfish) but I'm not 100% sure I am
interpreting
the results correctly. See
http://groups.google.com/group/google-guice/msg/ff53f95f422edabf
Original comment by gili.tza...@gmail.com
on 27 Nov 2008 at 2:31
I'd need to know more about the way you're loading Guice into the classloader
hierarchy to comment, and also if you're seeing any warnings or exceptions
relating
to the Finalizer thread. What I can say is that it does unload OK under OSGi,
which
suggests that something else is keeping the classloader alive in your case.
Note that the Finalizer thread won't shutdown until the Guice classloader is
eligible
for GC, so it will appear as a GC root until that happens - you may think it
looks
like a leak, but actually it will clear itself once other references to the
Guice
classloader are cleared. So I'd dig deeper into the heapdump for other causes.
You may also need to force finalization and GC a few times using the System API.
Original comment by mccu...@gmail.com
on 27 Nov 2008 at 3:01
I did some digging and found the problem - when running under JEE it seems
there are
several additional references from the Finalizer thread to the container
classloader
via access control contexts (presumably because of additional protection
domains).
Here's the complete list of references I found and some possible solutions:
1. Finalizer -> Thread.inheritedAccessControlContext -> ... -> Web CL
Set in Thread constructor to 'AccessController.getContext()' ie. the
context of the calling thread, this may refer to the Web classloader.
Solution: wrap creation of Finalizer thread in doPrivileged block
2. URLClassLoader -> URLClassLoader.acc -> ... -> Web CL
As above URLClassLoader constructor sets 'acc' field to current context.
Solution: doPrivileged block doesn't help here, the only way I found to
avoid this reference was to use reflection to null the 'acc' field which
is yucky, anyone know a better option? Perhaps we could grab defineClass
from the system classloader (again with reflection) and use that to load
the Finalizer into the system classloader, but this is also yucky :(
3. Finalizer -> Thread.inheritableThreadLocals -> ... -> Web CL
Each thread inherits an inheritableThreadLocals map from its parent, and
this could include a reference back to the container classloader (depends
on the app) - again no easy fix for this one, had to resort to reflection
to null out the field :(
4. Finalizer -> Thread.contextClassLoader -> Web CL
Yet another setting inherited from the calling thread, but at least here
there's a public API 'setContextClassLoader()' we can use to clear it :)
With these references removed I can completely unload Guice from GlassFish v2.
I've attached a patch in case people are interested - but because of the hacky
use of reflection to clear certain private fields, I'm not too happy with it.
I'm wondering instead whether it would be better to provide a public API people
could use to shutdown the thread, which would then make a lot of this code moot.
Anyway, Happy Thanksgiving!
Original comment by mccu...@gmail.com
on 28 Nov 2008 at 3:14
Attachments:
Nice detective work, mcculls!
Original comment by crazybob...@gmail.com
on 30 Nov 2008 at 2:38
FYI here are the safe fixes that I think should be used in google-collections,
such
as the use of doPrivileged() when creating the classloader and the Finalizer
thread.
The TCCL should also be cleared, just in case.
The two other references (possible inherited thread-local and access control
context)
are best fixed in the web container - certainly the thread-local issue is
fixable in
GlassFish. However, still not sure about the remaining access control context
issue.
Original comment by mccu...@gmail.com
on 2 Dec 2008 at 9:55
Attachments:
We'll address all of this in time for 1.0. I'm in favor of the reflection hacks
over
an explicit shutdown hook.
Original comment by crazybob...@gmail.com
on 17 Mar 2009 at 9:48
Original comment by kevin...@gmail.com
on 18 Mar 2009 at 2:21
Original comment by kevin...@gmail.com
on 17 Sep 2009 at 5:45
Original comment by kevin...@gmail.com
on 17 Sep 2009 at 5:46
Seems this won't be in 1.0 after all.
Original comment by kevin...@gmail.com
on 16 Oct 2009 at 9:01
Could you please reconsider this? Resource leak like this leads to frequent
PermGenErrors and thus to app server restarts, causing developers to need to
wait for
several minutes on each restart. Additionally, there are other redeployment
issues at
least under Windows because of file-locking (jar files can't be deleted or
moved).
Unfortunately there aren't any (usable) workarounds, so it would be really
great if
this could be included in 1.0.
Original comment by syva...@yahoo.com
on 26 Oct 2009 at 2:06
As for workarounds: I've experimented with a ServletContextListener that finds
any
Finalizer Threads and clears all the fields that Stuart mentioned.
http://pastie.org/686765 . I've tested for Guice Finalizer under Jetty and
Tomcat and
it works.
For Servlet environments afaik ServletContextListener is the only place you're
not
discouraged or explicitly forbidden to spawn threads. Therefore I am using a
custom
Finalizer that creates just one Finalizer in SCL and makes sure it is disposed
at the
end.
Original comment by alen_vre...@yahoo.com
on 6 Nov 2009 at 6:13
Will this cause a resource leak merely by including the jar on the classpath?
Or
will it only cause a problem if FinalizableReferenceQueue or classes using it
(e.g.
MapMaker) is used/referenced? If the latter is the case, one could presumably
work
around the issue by making sure that the class isn't referenced.
Some sort of fix or workaround seems like a fairly critical thing to have,
since I
imagine many (most?) people who would want to use the google collections
library are
running under JEE. The PermGen issue won't necessarily occur immediately as
they
may not redeploy right away, so users may not even know why PermGen errors
suddenly
started happening when they redeploy their war files.
Original comment by pseudony...@gmail.com
on 12 Nov 2009 at 12:26
This issue has been moved to the Guava project (keeping the same id number).
Simply replace 'google-collections' with 'guava-libraries' in your address
bar and it should take you there.
Original comment by kevinb@google.com
on 5 Jan 2010 at 11:09
Original issue reported on code.google.com by
nairb...@gmail.com
on 28 Jul 2008 at 5:39Attachments: