New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FinalizableReferenceQueue leaks, preventing reloads in a Servlet container #227

Closed
gissuebot opened this Issue Jul 7, 2014 · 12 comments

Comments

Projects
None yet
1 participant
@gissuebot

gissuebot commented Jul 7, 2014

From limpbizkit on August 04, 2008 15:43:09

Paolo Capriotti:
 "The FinalizableReferenceQueue thread prevents my application  from being unloaded. The thread 
is started from a stack-local instance, so I think there's no way I can stop it. " http://groups.google.com/group/google- guice/browse_thread/thread/1d022bfc3505f7fa/5cdd912e7b8b1095?
show_docid=5cdd912e7b8b1095

Original issue: http://code.google.com/p/google-guice/issues/detail?id=227

@gissuebot

This comment has been minimized.

Show comment
Hide comment

gissuebot commented Jul 7, 2014

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From crazyboblee on November 17, 2008 15:01:34

This has been fixed in Google Collections. We just need to bring over the new version.

gissuebot commented Jul 7, 2014

From crazyboblee on November 17, 2008 15:01:34

This has been fixed in Google Collections. We just need to bring over the new version.

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From gili.tzabari on November 17, 2008 21:21:25

Excellent. I'm eagerly awaiting this update! I've been suffering from major webapp
deployment problems related to memory leaks over the past five (!!) years. Being able
to develop webapps without having to restart the web server every five minutes would
be a dream come true :) Thank you!

gissuebot commented Jul 7, 2014

From gili.tzabari on November 17, 2008 21:21:25

Excellent. I'm eagerly awaiting this update! I've been suffering from major webapp
deployment problems related to memory leaks over the past five (!!) years. Being able
to develop webapps without having to restart the web server every five minutes would
be a dream come true :) Thank you!

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From limpbizkit on November 29, 2008 14:22:44

I brought in the new version of Google Collections in r708 .

Status: Fixed

gissuebot commented Jul 7, 2014

From limpbizkit on November 29, 2008 14:22:44

I brought in the new version of Google Collections in r708 .

Status: Fixed

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From gili.tzabari on November 29, 2008 14:34:22

Jesse, I don't think this is enough. You should probably keep this issue open until https://code.google.com/p/google-collections/issues/detail?id=92#c6 is fixed.

gissuebot commented Jul 7, 2014

From gili.tzabari on November 29, 2008 14:34:22

Jesse, I don't think this is enough. You should probably keep this issue open until https://code.google.com/p/google-collections/issues/detail?id=92#c6 is fixed.

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From limpbizkit on November 29, 2008 15:58:13

Stuart's patch exploits the fact that he can reflectively change some private fields - this is very fragile stuff.
Creating a new thread is a reasonable thing to do, and we shouldn't have to write hacks to make that safe.
Especially since the hacks break portability between JVMs (and possibly between app servers).

So what we really need a safe, hack-free way to use background threads that works in both Java SE and EE.

Gili, could you talk to the Glassfish folks and see if they can address this on their end?

gissuebot commented Jul 7, 2014

From limpbizkit on November 29, 2008 15:58:13

Stuart's patch exploits the fact that he can reflectively change some private fields - this is very fragile stuff.
Creating a new thread is a reasonable thing to do, and we shouldn't have to write hacks to make that safe.
Especially since the hacks break portability between JVMs (and possibly between app servers).

So what we really need a safe, hack-free way to use background threads that works in both Java SE and EE.

Gili, could you talk to the Glassfish folks and see if they can address this on their end?

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From gili.tzabari on November 29, 2008 18:22:19

Jesse,

I filed a new bug report: https://glassfish.dev.java.net/issues/show_bug.cgi?id=6857

gissuebot commented Jul 7, 2014

From gili.tzabari on November 29, 2008 18:22:19

Jesse,

I filed a new bug report: https://glassfish.dev.java.net/issues/show_bug.cgi?id=6857

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From mcculls on November 30, 2008 02:58:19

Yeah, those reflection hacks are totally flaky and I didn't expect them to go into
google-collections, but using doPrivileged when creating the Finalizer thread and
pro-actively setting the TCCL to null are IMHO a good idea.

The dangling inherited-thread-local can be fixed in the GlassFish worker pool, which
just leaves the access-control-context issue in the classloader. Again this might be
possible to fix in GlassFish, but I'm not sure as it involves Java security which is
a headache to follow at the best of times :)

gissuebot commented Jul 7, 2014

From mcculls on November 30, 2008 02:58:19

Yeah, those reflection hacks are totally flaky and I didn't expect them to go into
google-collections, but using doPrivileged when creating the Finalizer thread and
pro-actively setting the TCCL to null are IMHO a good idea.

The dangling inherited-thread-local can be fixed in the GlassFish worker pool, which
just leaves the access-control-context issue in the classloader. Again this might be
possible to fix in GlassFish, but I'm not sure as it involves Java security which is
a headache to follow at the best of times :)

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From mcculls on November 30, 2008 04:46:52

Also be aware that Guice has its own copy of some of the Google-Collections code,
including the Finalizer thread - I coded up a patch to fix this when I was testing
the other issue: http://peaberry.googlecode.com/svn/branches/spike/lib/build/google-guice-patch-227.txt I still had to keep copies of a few classes from Google-Collections (such as the
ReferenceMap) to fix some visibility issues but I sync'd them with the version in
Google-Collections. Also the tests for ReferenceMap have an unfortunate dependency on
the relocated package name (because they aren't JarJar'd) but it's a start...

gissuebot commented Jul 7, 2014

From mcculls on November 30, 2008 04:46:52

Also be aware that Guice has its own copy of some of the Google-Collections code,
including the Finalizer thread - I coded up a patch to fix this when I was testing
the other issue: http://peaberry.googlecode.com/svn/branches/spike/lib/build/google-guice-patch-227.txt I still had to keep copies of a few classes from Google-Collections (such as the
ReferenceMap) to fix some visibility issues but I sync'd them with the version in
Google-Collections. Also the tests for ReferenceMap have an unfortunate dependency on
the relocated package name (because they aren't JarJar'd) but it's a start...

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From gili.tzabari on October 22, 2009 10:03:18

How can this issue be fixed if https://code.google.com/p/google-collections/issues/detail?id=92 is still open?

gissuebot commented Jul 7, 2014

From gili.tzabari on October 22, 2009 10:03:18

How can this issue be fixed if https://code.google.com/p/google-collections/issues/detail?id=92 is still open?

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From syvalta@yahoo.com on October 26, 2009 07:10:51

There seems to be another issue report for it: https://code.google.com/p/google-guice/issues/detail?id=288 .

gissuebot commented Jul 7, 2014

From syvalta@yahoo.com on October 26, 2009 07:10:51

There seems to be another issue report for it: https://code.google.com/p/google-guice/issues/detail?id=288 .

@gissuebot

This comment has been minimized.

Show comment
Hide comment
@gissuebot

gissuebot Jul 7, 2014

From syvalta@yahoo.com on October 26, 2009 07:13:18

But you probably knew that already as you seem to have originally reported it :)

gissuebot commented Jul 7, 2014

From syvalta@yahoo.com on October 26, 2009 07:13:18

But you probably knew that already as you seem to have originally reported it :)

@gissuebot gissuebot closed this Jul 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment