Skip to content

FinalizableReferenceQueue keeps ClassLoaders around. #92

Description

@gissuebot

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.

Activity

  1. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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

  2. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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.

  3. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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 :)

  4. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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/ff53f95f422edabf

  5. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original 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.

  6. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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:

    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

    1. 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 :(

    1. 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 :(

    1. 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!

  7. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original comment posted by crazyboblee on 2008-11-30 at 02:38 AM


    Nice detective work, mcculls!

  8. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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.

  9. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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.

  10. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original comment posted by kevinb9n on 2009-03-18 at 02:21 AM


    (No comment entered for this change.)


    Labels: 1.0

  11. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original comment posted by kevinb9n on 2009-09-17 at 05:45 PM


    (No comment entered for this change.)


    Labels: Milestone-1.0

  12. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original comment posted by kevinb9n on 2009-09-17 at 05:46 PM


    (No comment entered for this change.)


    Labels: -1.0

  13. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original 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.0

  14. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    Original 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.

  15. gissuebot commented on Oct 31, 2014

    @gissuebot
    Author

    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.

  16. 89 remaining items

  17. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    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();
      }

  18. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    Original comment posted by jerith666 on 2013-08-16 at 05:57 PM


    issue 1505 may or may not be relevant here

  19. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    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: Fixed

  20. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    Original 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.

  21. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    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.

  22. gissuebot commented on Nov 1, 2014

    @gissuebot
    Author

    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.

  23. modified the milestone: 12.0 on Nov 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions