RFR(S) 8218700: infinite loop in HotSpotJVMCIMetaAccessContext.fromClass after OutOfMemoryError

[Vladimir Kozlov]

Looks good to me too.

Thanks
Vladimir

On 5/3/19 11:55 AM, dean.long at oracle.com wrote:
> On 5/3/19 10:45 AM, Tom Rodriguez wrote:
>>
>>
>> dean.long at oracle.com wrote on 5/2/19 11:47 PM:
>>> On 5/1/19 5:44 PM, Tom Rodriguez wrote:
>>>> You'll need to update your webrev after Vladimir's push.  This code has moved into HotSpootJVMCIRuntime.java.
>>>>
>>>
>>> Here's the updated version:
>>>
>>> http://cr.openjdk.java.net/~dlong/8218700/webrev.3/
>>
>> Looks good to me.
> 
> Thanks for the review.
> 
>>
>>>
>>>> Maybe WeakReferenceHolder instead of WeakTypeRef?  It needs a comment explaining that we're intentionally avoiding 
>>>> the use of ClassValue.remove as well. Shouldn't the ref field be volatile? ClassValue includes some barrier 
>>>> semantics and the new code needs similar guarantees.
>>>>
>>>
>>> I went ahead and made it volatile, but I don't understand what guarantee was missing, and what problem we want to 
>>> eliminate, unless it is to reduce the possibility of duplicates.  But the fix for JDK-8201248 assumes that duplicates 
>>> are possible, so I wasn't worried about that.
>>
>> We're publishing a mutable locally created object to other threads so it seems like we need some sort of ordering 
>> barrier when we do so. Presumably the ClassValue would normally provide some ordering though it's a little unclear 
>> from the javadoc if it makes any such guarantees. Is the extra volatile unneeded?
>>
> 
> ClassValue uses volatile internally so that an unsynchronized read sees the latest version.  Using a volatile here 
> should help in a similar way, but I believe there is still a race that allows duplicates if the weak reference gets 
> cleared by GC.  To prevent all duplicates I think we would need both volatile and more synchronization.
> 
> dl
> 
>> tom
>>
>>>
>>> dl
>>>
>>>> tom
>>>>
>>>> dean.long at oracle.com wrote on 4/26/19 12:09 PM:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8218700
>>>>> http://cr.openjdk.java.net/~dlong/8218700/webrev.2/
>>>>>
>>>>> If we throw an OutOfMemoryError in the right place (see JDK-8222941), HotSpotJVMCIMetaAccessContext.fromClass can 
>>>>> go into an infinite loop calling ClassValue.remove.  To work around the problem, reset the value in a mutable cell 
>>>>> instead of calling remove.
>>>>>
>>>>> dl
>>>
>