RFR (M): 8218880: G1 crashes when issuing a periodic GC while the GCLocker is held

[Thomas Schatzl]

Hi Kim,

  thanks for your review.

On Wed, 2019-02-27 at 17:19 -0500, Kim Barrett wrote:
> > On Feb 27, 2019, at 4:29 AM, Thomas Schatzl <
> > thomas.schatzl at oracle.com> wrote:
> > 
> > Hi all,
> > 
> >  can I have reviews for this change that fixes execution of
> > periodic GCs in G1?
> > 
> > In the current implementation, if periodic GC is triggered when the
> > gc locker is held, the VM crashes because the GC locker is supposed
> > to be only held by Java threads - but the periodic GCs are
> > triggered by some GC thread.
> > 
> > The fix is to ignore the periodic GC request if the GC locker is
> > held - after all this means that the application is active anyway,
> > and a GC will be triggered anyway as soon as the critical section
> > ends.
> > 
> > To make it clear to end users that the periodic GC has been
> > cancelled due to that reason, the change prints a cancellation
> > reason too if appropriate logging is enabled.
> > 
> > The change itself is pretty small, it's just the test that's a bit
> > longer but straightforward.
> > 
> > I will backport this to 12u later.
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8218880
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8218880/webrev/
> > Testing:
> > New test case, hs-tier1-5
> > 
> > Thanks,
> >  Thomas
> 
> I think I would prefer "try_collect" rather than "attempt_collect".
> 

Done.

> At first I was thinking this seems very specific to the particular
> bug at hand, and there is a need for something more general. But I've
> decided I'm okay with this change as is, and leaving generalization
> to the still-in-progress work on improving the whitebox control
> mechanism.

Thanks. Please also consider that this change is supposed to be
backported.

> 
> So other than the possible name change, looks good.
> 

New webrevs:

http://cr.openjdk.java.net/~tschatzl/8218880/webrev.1_to_2/ (diff)
http://cr.openjdk.java.net/~tschatzl/8218880/webrev.2/ (full)

Thanks,
  Thomas