RFR: 8272797: Mutex with rank safepoint_check_never imply allow_vm_block [v7]

[Coleen Phillimore]

On Mon, 30 Aug 2021 02:34:17 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reverse the order of safepoint_check_required and allow_vm_block arguments so that the first can imply the value of the second.  This is good because if we decide to rename the second to something like _allow_safepoint_use instead there's less to change. Also remove default arguments as they are evil.
>
> src/hotspot/share/memory/heapInspection.hpp line 248:
> 
>> 246:       _missed_count(0),
>> 247:       _success(true),
>> 248:       _mutex(Mutex::leaf, "Parallel heap iteration data merge lock", Mutex::_safepoint_check_always) {}
> 
> Is this lock even used by JavaThreads. ??

It doesn't look like this is.  If we change the _allow_vm_block check in Mutex::check_block_state to be:

     if (!_enforces_nsv && (!thread->isJavaThread() && is_at_safepoint())) {
         // some good explanatory comment
         fatal("A NonJava thread could block on lock that may be held by a JavaThread during safepoint: %s", name());
    }

I think this lock will get caught by this assert.

> test/hotspot/gtest/metaspace/test_is_metaspace_obj.cpp line 52:
> 
>> 50: 
>> 51:   void do_test(Metaspace::MetadataType mdType) {
>> 52:     _lock = new Mutex(Monitor::leaf, "gtest-IsMetaspaceObjTest-lock", Monitor::_safepoint_check_never);
> 
> Again unclear what impact false to true might have here - assuming VM threads even lock this? Ditto for other metaspace tests.

It has zero impact.  These locks just want no safepoint checking.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5218