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

[David Holmes]

Hi Dan,

On 13/08/2020 1:26 am, Daniel D. Daugherty wrote:
>> webrev: http://cr.openjdk.java.net/~dholmes/8251460/webrev/ 
> 
> src/hotspot/share/runtime/synchronizer.cpp
>      No comments.
> 
> Thumbs up!

Thanks for the review.

> Thanks for the info about how this was tested. Any Mach5 testing
> done yet?

Putting it through t1-3 now. Will push once that passes.

Thanks,
David
-----

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