PING: Does CollectedHeap::print_on() need Heap_lock?

[Daniel D. Daugherty]

Adding hotspot-gc-dev at ...

Dan


On 4/25/16 6:04 AM, Yasumasa Suenaga wrote:
> Hi all,
>
> I've sent a question for Heap_lock in Universe::print_*().
> Have you read it?
>
> I think we need to lock Heap_lock in it.
> Actually, callers of Universe::print_on() seems to be in safepoint.
>
> However, I'm not sure that, so I want to hear about that before
> filing to JBS.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/04/12 22:56, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> I have a question about Heap_lock in Universe::print_*().
>>
>> Universe::print_heap_at_SIGBREAK() gets Heap_lock.
>> However, HeapInfoDCmd::execute() and Universe::print_on() do not get 
>> Heap_lock.
>>
>> Should we get Heap_lock when we print heap information?
>> I guess that we should get this lock for consistency.
>>
>> --------------------
>> diff -r 87215e99d945 src/share/vm/memory/universe.cpp
>> --- a/src/share/vm/memory/universe.cpp  Wed Apr 06 23:42:52 2016 +0000
>> +++ b/src/share/vm/memory/universe.cpp  Tue Apr 12 22:23:17 2016 +0900
>> @@ -1069,6 +1069,7 @@
>>  }
>>
>>  void Universe::print_on(outputStream* st) {
>> +  MutexLocker hl(Heap_lock);
>>    st->print_cr("Heap");
>>    heap()->print_on(st);
>>  }
>> diff -r 87215e99d945 src/share/vm/services/diagnosticCommand.cpp
>> --- a/src/share/vm/services/diagnosticCommand.cpp       Wed Apr 06 
>> 23:42:52 2016 +0000
>> +++ b/src/share/vm/services/diagnosticCommand.cpp       Tue Apr 12 
>> 22:23:17 2016 +0900
>> @@ -413,6 +413,7 @@
>>  }
>>
>>  void HeapInfoDCmd::execute(DCmdSource source, TRAPS) {
>> +  MutexLocker hl(Heap_lock);
>>    Universe::heap()->print_on(output());
>>  }
>>
>> --------------------
>>
>> If it is correct, I file it to JBS and send review request.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>