RFR (XS) 8237080: fatal error: VM thread could block on lock that may be held by a JavaThread during safepoint: SharedDecoder_lock

[coleen.phillimore at oracle.com]

Thanks for the comments and discussion, Patricio!
Coleen

On 1/29/20 5:53 PM, Patricio Chilano wrote:
> Hi Coleen,
>
> On 1/29/20 7:00 PM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 1/29/20 1:32 PM, coleen.phillimore at oracle.com wrote:
>>>
>>>
>>> On 1/29/20 12:07 PM, Patricio Chilano wrote:
>>>> Hi Coleen,
>>>>
>>>> Looks good to me. Not for this change but seems all monitors that 
>>>> are _safepoint_check_never should have allow_vm_block to true since 
>>>> they will not be held during a safepoint. Based on the output 
>>>> message in check_block_state() that's what we are trying to avoid. 
>>>> Also we wouldn't be incrementing _no_safepoint_count in 
>>>> no_safepoint_verifier() otherwise, which I think is what we want if 
>>>> the monitor has the _safepoint_check_never flag.
>>>
>>> I was wondering if someone would ask this question.  Yes, you're 
>>> right.  I had a bigger change at one point that verified that 
>>> _safepoint_check_never flags always implied _allow_vm_block to be 
>>> true, but backed off in the interest of a less invasive change. 
>>> There were a few mismatched locks.
>>>
>>> I'll try the larger change, which does have the effect of 
>>> _safepoint_check_never always implying NoSafepointVerifier, which 
>>> was my intention in the previous cleanups.
>>
>> Unfortunately, there are locks that are _safepoint_check_never where 
>> _allow_vm_block = false.  These locks _may_ want to assert that the 
>> VMthread never calls them.  The ZGC threads take locks with these 
>> values.
>>
>> There are locks that are _safepoint_check_always where 
>> _allow_vm_block = true.  These locks are also taken by the VM 
>> thread.  The NoSafepointVerifier in the code for _allow_vm_block 
>> protect these locks from deadlocking by preventing safepoints while 
>> they are held.
>>
>> As you pointed out, we only make a NoSafepointVerifier for locks that 
>> are _allow_vm_block = true, which covers many but not all of the 
>> _safepoint_check_never locks.   If I make it also check 
>> _safepoint_check_never, there's a JFR lock that very specifically 
>> violates this with comments.
>>
>> I am going to stay with my first change for this bug fix.
> Ok, we can look at that later then. Thanks for investigating and 
> testing that too.
>
> Patricio
>> Thanks,
>> Coleen
>>>
>>> thanks,
>>> Coleen
>>>>
>>>> Thanks,
>>>> Patricio
>>>>
>>>> On 1/28/20 6:36 PM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Set allow_vm_block to true for this lock.  It's 
>>>>> _safepoint_check_never so it's sort of implied (you can't 
>>>>> safepoint holding the lock and block out the vm thread).
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2020/8237080.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8237080
>>>>>
>>>>> Ran test case with hard-coded failure, and tier1-6.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>
>>
>