Does CollectedHeap::print_on() need Heap_lock?

[Thomas Schatzl]

Hi Yasumasa,

  sorry for the long wait on this...

On Tue, 2016-04-12 at 22:56 +0900, 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?

It is not required. The methods are only ever called at a safepoint. 

> I guess that we should get this lock for consistency.

That would be something that could be done, or just adding
asserts/guarantees about the intended places to call (i.e. either at
safepoint or heap locked - there should be some method like that
somewhere actually).

> 
> --------------------
> 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,
  Thomas