RFR: 8150518: G1 GC crashes at G1CollectedHeap::do_collection_pause_at_safepoint(double)

[David Holmes]

On 9/03/2016 9:33 PM, Fairoz Matte wrote:
> Background:
>
> After the backport of https://bugs.openjdk.java.net/browse/JDK-8017462, The flag -XX:+UseG1GC combined with -XX:ParallelGCThreads=0 makes the _workers to null in 8u.
>
> As there is no condition to handle such scenario in share/vm/memory/sharedHeap.cpp, which causes the crash.
>
> The similar condition is already implemented for following scenarios
>
> 1.       -XX:+UseParallelGC -XX:ParallelGCThreads=0
>
> 2.       -XX:+UseParNewGC -XX:ParallelGCThreads=0
>
> 3.       -XX:+UseConcMarkSweepGC -XX:ParallelGCThreads=0
>
>
>
> Fix:
>
> Condition check is added in src/share/vm/runtime/arguments.cpp file to verify "-XX:+UseG1GC -XX:ParallelGCThreads=0"
>
> Thanks for the base patch from Jon.
>
> Due to this patch it makes some of the test cases absolute. They have been removed.
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8150518
>
> Webrev:  http://cr.openjdk.java.net/~rpatil/8150518/webrev.00/

This existing code looks wrong:

1675   FLAG_SET_DEFAULT(ParallelGCThreads,
1676                    Abstract_VM_Version::parallel_worker_threads());
1677   if (ParallelGCThreads == 0) {
1678     FLAG_SET_DEFAULT(ParallelGCThreads,
1679                    Abstract_VM_Version::parallel_worker_threads());

Line 1678 seems to do the same as 1675 - is 
Abstract_VM_Version::parallel_worker_threads() somehow expected to 
return a different value on the second call ??

That aside I'm confused by the fix as we have:

./share/vm/runtime/globals.hpp:  product(uintx, ParallelGCThreads, 0,

so it seems odd to report an error for setting a value that is already 
the default ??

Thanks,
David