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

[dean.long at oracle.com]

Thanks Vladimir.

dl

On 5/6/19 2:34 PM, Vladimir Kozlov wrote:
> 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
>>>>
>>