RFR: JDK-8155730: HeapInfoDCmd should get Heap_lock

[David Holmes]

On 9/05/2016 10:34 PM, Yasumasa Suenaga wrote:
> Hi David,
>
>> Taking locks during VMError handling is problematic - we're executing
>> from a signal handler and the VM is crashing. Much of what we do from
>> VMError is "best effort".
>
> I will remove MutexLocker from VMError::report().
> But I added assert_locked_or_safepoint(Heap_lock) in each CollectedHeap.
> If we remove Heap_lock from VMError::report(), fastdebug HotSpot is failed
> when it prints heap info to hs_err log.
> Should I remove assert_locked_or_safepoint()?

Yes I think you need to do this.

> VMError::print_vm_info() calls from VMInfoDCmd::execute() only.
> So I think we should take Heap_lock at this function.
> Or should we take Heap_lock at VMInfoDCmd::execute() ?

If this is called from a DCmd only then I think the lock can be taken in  
print_vm_info().

Need GC folk to add their weight here.

Thanks,
David

>
> Thanks,
>
> Yasumasa
>
>
> On 2016/05/09 21:26, David Holmes wrote:
>> On 9/05/2016 10:20 PM, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> I've sent review request for JDK-8155730.
>>> However, I found caller of Universe::heap()->print_*() in vmError.cpp .
>>> Thus I added MutexLocker(Ex) for Heap_lock to VMError::report() and
>>> VMError::print_vm_info().
>>
>> Taking locks during VMError handling is problematic - we're executing
>> from a signal handler and the VM is crashing. Much of what we do from
>> VMError is "best effort".
>>
>> David
>>
>>> Could you review again?
>>>
>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.01/
>>>
>>>
>>>> This needs to be reviewed by the GC folk not runtime. Please direct
>>>> followups to hotspot-gc-dev.
>>>
>>> Thanks, David!
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2016/05/06 13:11, David Holmes wrote:
>>>> Hi Yasumasa,
>>>>
>>>> This needs to be reviewed by the GC folk not runtime. Please direct
>>>> followups to hotspot-gc-dev.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 30/04/2016 12:05 AM, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> This review request is related to [1].
>>>>>
>>>>> HeapInfoDCmd calls CollectedHeap::print_on().
>>>>> However, its call does not get Heap_lock.
>>>>>
>>>>> Heap_lock should be gotten from all heap operations.
>>>>>
>>>>> I uploaded webrev for this issue.
>>>>> Could you review it?
>>>>>
>>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155730/webrev.00/
>>>>>
>>>>> I cannot access JPRT.
>>>>> So I need a sponsor.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> [1]
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019257.html
>>>>>
>>>>>
>>>>>
>>>>>