RFR: 8209758: 2 classes with same name G1PrintCollectionSetClosure cause crash when logging is enabled

Thomas Schatzl thomas.schatzl at oracle.com
Tue Aug 21 08:27:17 UTC 2018


Hi,

On Mon, 2018-08-20 at 15:25 -0700, Leonid Mesnik wrote:
> Hi
> 
> Could you please review small fix which rename
> class G1PrintCollectionSetClosure in g1CollectionSet.cpp.
> 
> While I was developing test for JDK-8209150 [TESTBUG] Add logging to
> verify JDK-8197901 to a different test 
> I observed SEGV on macosx. Actually a lot of tests fail because of
> this same issue when logging is enabled.
> 
> 
[...]
> because class 'G1PrintCollectionSetClosure' is used from
> http://hg.openjdk.java.net/jdk/jdk/file/d96e6839e83d/src/hotspot/shar
> e/gc/g1/g1CollectionSet.cpp#l331
> instead of 
> http://hg.openjdk.java.net/jdk/jdk/file/d96e6839e83d/src/hotspot/shar
> e/gc/g1/g1CollectedHeap.cpp#l2751
> so it tries to do completely wrong thing... 
> 
> I think it would be enough just to rename one of classes to avoid
> confusion.

Although I think this is a clang compiler bug, let's rename it...

> webrev: http://cr.openjdk.java.net/~lmesnik/8209758/webrev.00/
> bug: https://bugs.openjdk.java.net/browse/JDK-8209758
> I run gc & serviceability tests with enabled logging to verify that
> they work fine now.


... however I would prefer if the closure were named
G1PrintCollectionSetDetailClosure. While not a lot better, the proposed
name seems awkward to read to me.

No need for re-review if you change that.

Looks good.

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list