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

[dean.long at oracle.com]

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
>>