RFR (s): 8251460: Fix the biased-locking code in ObjectSynchronizer::FastHashCode

[David Holmes]

Thanks Coleen!

I forgot to mention that the testing for this was to reenable (in 
arguments.cpp) biased-locking for CDS (and fix an errant assert that Ioi 
is going to fix) and run the CDS tests. None of those changes will be 
pushed of course.

David

On 12/08/2020 9:58 pm, Coleen Phillimore wrote:
> Looks good!
> Coleen
> 
> On 8/12/20 12:23 AM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8251460
>> webrev: http://cr.openjdk.java.net/~dholmes/8251460/webrev/
>>
>> During the review for JDK-8250606 it was noted that the biased-locking 
>> section of FastHashCode contained a similar assertion regarding 
>> execution at a safepoint:
>>
>>       // Relaxing assertion for bug 6320749.
>>       assert(Universe::verify_in_progress() ||
>>              !SafepointSynchronize::is_at_safepoint(),
>>              "biases should not be seen by VM thread here");
>>
>> which gives the impression that it is okay to execute this code block 
>> at a safepoint if verify_in_progress() is true. But if we examine the 
>> next line of code:
>>
>>   BiasedLocking::revoke(hobj, JavaThread::current());
>>
>> we find that at a safepoint:
>> a) JavaThread::current() will assert because the current thread will 
>> be the VMThread; and
>> b) BiasedLocking::revoke itself contains an assertion that it is not 
>> invoked at a safepoint.
>>
>> In practice there is no issue with revoking the bias at a safepoint, 
>> we just have to use the right code to do so (as done elsewhere in the 
>> same file):
>>
>> if (SafepointSynchronize::is_at_safepoint()) {
>>   BiasedLocking::revoke_at_safepoint(hobj);
>> } else {
>>   BiasedLocking::revoke(hobj, self);
>> }
>>
>> Thanks,
>> David
>