RFR: 8016341 java/lang/ref/OOMEInReferenceHandler.java failing intermittently

Chris Hegarty chris.hegarty at oracle.com
Mon Jul 8 09:09:12 UTC 2013


On 8 Jul 2013, at 03:33, David Holmes <david.holmes at oracle.com> wrote:

> Still looking for a reviewer for this please.

The test change looks ok to me.

-Chris


> Thanks,
> David
> 
> On 5/07/2013 9:22 AM, David Holmes wrote:
>> Hi Peter,
>> 
>> On 2/07/2013 5:19 PM, Peter Levart wrote:
>>> Looking at original code once again, I think this was actually a bug.
>>> The WeakReference instance constructed in (old) line 82, can be GCed
>>> right away, since nobody is using the local variable after assignment.
>> 
>> Of course. Doh! I was to busy thinking about the lifetime of the
>> referent object to consider the reference itself.
>> 
>>> If WeakReference is GCed it can not be enqueued. The promotion of local
>>> variable into a field is one way to fix this. The other would be to use
>>> the local variable somewhere down the code path, like for example in a
>>> final throw statement:
>>> 
>>>  110          throw new IllegalStateException("Reference Handler thread
>>> stuck. weakRef.get(): " + weakRef.get());
>>> 
>>> 
>>> This would also reveal some more info about the WeakReference when
>>> there's no sure answer after 10 seconds and could be added to the test
>>> anyway.
>> 
>> Okay I've modified the test as suggested. Updated webrev at same location:
>> 
>> http://cr.openjdk.java.net/~dholmes/8016341/webrev/
>> 
>> In testing it though I simply exposed the remaining flaws in the
>> ReferenceHandler code. We can still kill the ReferenceHandler thread
>> with an OOME when it tries to load the Cleaner class (running with a 5M
>> heap triggers this nicely if you use G1):
>> 
>>                // Fast path for cleaners
>>                 if (r instanceof Cleaner) {
>>                     ((Cleaner)r).clean();
>>                     continue;
>>                 }
>> 
>> and if that passes the clean() might throw OOME (it internally tries to
>> do a System.exit if an exception occurs but will likely encounter
>> another OOME trying to create the PrivilegedAction).
>> 
>> Even the:
>> 
>>                ReferenceQueue q = r.queue;
>>                 if (q != ReferenceQueue.NULL) q.enqueue(r);
>> 
>> might throw OOME because enqueue() might have to load the FinalReference
>> class.
>> 
>> So really catching the OOME around the wait() only patches a small hole.
>> We can't simply put a try/catch in the for(;;) loop because that doesn't
>> address the problem that if the class loading throws OOME then
>> subsequent attempts to load that class will also fail. We would have to
>> preload all possible classes. Even then we might just send the
>> ReferenceHandler thread into a busy loop of throwing OOME catching it
>> and retrying!
>> 
>> So I can fix the test to deal with the Xcomp issue but we may still see
>> intermittent failures, and the ReferenceHandler thread may still die
>> from OOME.
>> 
>> David
>> -----
>> 
>> 
>>> 
>>> Regards, Peter
>>> 
>>> On 07/02/2013 06:38 AM, David Holmes wrote:
>>>> This recently added test was found to fail under some conditions -
>>>> namely client compiler with -Xcomp. It seems that the use of all local
>>>> variables enabled the compiler to optimize things in a way that
>>>> stopped the weakref from being enqueued as expected. Simple fix was to
>>>> make the weakref a field.
>>>> 
>>>> http://cr.openjdk.java.net/~dholmes/8016341/webrev/
>>>> 
>>>> Thanks,
>>>> David
>>> 



More information about the core-libs-dev mailing list