Repository navigation
FinalizableReferenceQueue keeps ClassLoaders around. #92
Description
Activity
Original comment posted by kevinb9n on 2008-07-28 at 07:00 PM
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.
Owner: crazyboblee
Original comment posted by nairb774 on 2008-07-28 at 07:43 PM
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 posted by mcculls on 2008-11-26 at 05:24 PM
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 posted by gili.tzabari on 2008-11-27 at 02:31 AM
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/ff53f95f422edabfOriginal comment posted by mcculls on 2008-11-27 at 03:01 AM
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 posted by mcculls on 2008-11-28 at 03:14 PM
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:
- 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
- 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 :(- 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 :(- 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 posted by mcculls on 2008-12-02 at 09:55 AM
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 posted by crazyboblee on 2009-03-17 at 09:48 PM
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 posted by kevinb9n on 2009-03-18 at 02:21 AM
(No comment entered for this change.)
Labels:
1.0Original comment posted by kevinb9n on 2009-09-17 at 05:45 PM
(No comment entered for this change.)
Labels:
Milestone-1.0Original comment posted by kevinb9n on 2009-09-17 at 05:46 PM
(No comment entered for this change.)
Labels: -
1.0Original comment posted by kevinb9n on 2009-10-16 at 09:01 PM
Seems this won't be in 1.0 after all.
Labels: -
Milestone-1.0,Milestone-Post1.0Original comment posted by syva...@yahoo.com on 2009-10-26 at 02:06 PM
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 posted by alen_vre...@yahoo.com on 2009-11-06 at 06:13 PM
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.89 remaining items
Original comment posted by emcmanus@google.com on 2012-08-01 at 04:41 PM
You don't need to use reflection to invoke the close() method even if you are concerned about compatibility with earlier Guava versions. Just write
if (frq instanceof Closeable) {
((Closeable) frq).close();
}Original comment posted by jerith666 on 2013-08-16 at 05:57 PM
issue 1505 may or may not be relevant here
Original comment posted by emcmanus@google.com on 2013-11-20 at 11:23 PM
Marking this as Fixed since it is possible to avoid the problem by calling close() at the appropriate time. We do not currently plan to implement a mechanism that closes automatically when there are no FinalizableReferences left.
Status:
FixedOriginal comment posted by Karsten.Ohme on 2014-03-28 at 02:33 PM
For Guava 13.0.1 (which is a requirement for BoneCP) I had to modify the fix from comment #15:
http://pastie.org/8976099
Insteas of thread.getClass().getName(); I had to use thread.getName();Also important is, that this must not be proxied by aspectj. The method was not invoke here if this was the case.
Original comment posted by cow...@bbs.darktech.org on 2014-03-28 at 02:37 PM
I suggest opening a new bug report that references this one because most committers will ignore comments on closed issues.
Original comment posted by emcmanus@google.com on 2014-03-28 at 04:43 PM
I believe comment #110 is a note about a workaround that is needed if you have to use an old version of Guava, correct? Obviously we can't modify old Guava versions retroactively. If there is something that needs to be done to the current version then by all means open a new issue.
- addedtype=defectBug, not working as expectedBug, not working as expected
on Nov 1, 2014
Original issue created by nairb774 on 2008-07-28 at 05:39 PM
If the class loader which loaded the google-collections jar is let go
(consider redeploying a war to Apache Geronimo) the class loader will not
be able to be garbage collected. This is caused by the thread started by
FinalizableReferenceQueue never ending. In other words the garbage
collection (for the class loader) can only happen when the thread stops.
The solution is to stop the thread when there are no more objects on the
queue. This will happen eventually if the rest of the app behaves
correctly by not placing state outside of its class loader's environment.
Attached is a version of the class which I have used to this effect.
This is also a problem in the OSGi space where every jar gets its own class
loader and they can be refreshed easily at runtime to do rolling
upgrades/bug fixes.