On Mar 21, 2016, at 8:20 AM, Per Liden <per.li...@oracle.com> wrote:
Hi Peter & David,
(Resurrecting an old thread here...)
On 2014-01-22 03:19, David Holmes wrote:
Hi Peter,
On 22/01/2014 12:00 AM, Peter Levart wrote:
Hi, David, Kalyan,
Summing up the discussion, I propose the following patch for
ReferenceHandler:
http://cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.01/
I can live with it, though it maybe that once Cleaner has been preloaded
instanceof can no longer throw OOME. Can't be 100% sure. And there's
some duplication/verbosity in the commentary that could be trimmed down :)
While investigating a Reference pending list issue on the GC side of things I looked at the
ReferenceHandler thread and noticed something which made me uneasy. The fix for JDK-8022321 added
pre-loading of the Cleaer class to avoid OMME, but also moved the "instanceof Cleaner"
inside the try/catch with a comment that it "sometimes" can throw an OOME. I understand
this was done because we're not 100% sure if a OOME can still happen here, despite the pre-loading.
However, if it can throw an OOME that means it's allocating, which in turn means it can provoke a
GC. If that happens, it looks to me like we have a bug here. The ReferenceHandler thread is not
allowed to provoke a GC while it's holding on to the pending list lock, since the pending list
might be updated during a GC and "pending = r.discovered" will than overwrite something
other than "r", silently dropping any newly discovered References which will never be
discovered by the the GC again.
On the other hand, if an OOME can never happen (i.e. no GC) here then we're
good the comment is just incorrect. The instanceof check could be moved out of
the try/catch block again, like it was prior to this change, just to make it
obvious that we will not be able to cause new allocations inside the critical
section. Or at a minimum, the comment saying OOME can still happen should be
adjusted.
Thoughts?
thanks,
Per
Btw, to the best of my knowledge, the pre-loading of Cleaner should avoid any
GC activity from instanceof, but I can't say that am a 100% sure either.
