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