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

[Coleen Phillimore]

On 8/12/20 8:16 AM, David Holmes wrote:
> 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.

Yes, it's good to know that CDS dumping could turn on biased locking, 
not that it would ever need to.

Coleen

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