RFR (XS): JDK-8040977: G1 crashes when run with -XX:-G1DeferredRSUpdate

[Mikael Gerdin]

Thomas,

On Thursday 26 June 2014 13.28.49 Bengt Rutisson wrote:
> Hi Thomas,
> 
> Sorry for the very late reply.
> 
> I think the dependency between G1ParScanClosure is still very awkward,
> but I think your change is a step in the right direction. Thanks for
> fixing this.

I agree with Bengt.
The change seems reasonable.

/Mikael

> 
> Reviewed.
> Bengt
> 
> On 2014-05-27 12:39, Thomas Schatzl wrote:
> > Hi Bengt,
> > 
> >    thanks for the review.
> > 
> > On Wed, 2014-04-30 at 12:50 +0200, Bengt Rutisson wrote:
> >> Hi Thomas,
> >> 
> >> On 2014-04-22 14:56, Thomas Schatzl wrote:
> >>> Hi all,
> >>> 
> >>>     can I have reviews for this change? It fixes wrong order of
> >>> 
> >>> declaration of members of G1ParScanThreadState that causes crashes when
> >>> G1DeferredRSUpdate is disabled.
> >>> 
> >>> The change is based on the changes for 8035400 and8035401 posted
> >>> recently.
> >>> 
> >>> CR:
> >>> https://bugs.openjdk.java.net/browse/JDK-8040977
> >>> 
> >>> Webrev:
> >>> http://cr.openjdk.java.net/~tschatzl/8040977/webrev/
> >> 
> >> I realize that this fixes the code but I would really appreciate a more
> >> stable way of handling the dependencies.
> >> 
> >> As it it now we end up calling methods on a G1ParScanThreadState
> >> instance while we are setting it up. This seems broken to me and will
> >> probably lead to similar initialization order issues again. Best would
> >> be to not pass "this" to the constructor of G1ParScanClosure and instead
> >> manage the circular dependency between G1ParScanClosure and
> >> G1ParScanThreadState more explicitly after they have both been properly
> >> set up.
> >> 
> >> Second best would be to at least pass the worker id/queue num as a
> >> separate parameter to avoid having to call methods on an uninitialized
> >> object.
> > 
> > I fixed this implementing the former idea. Also added some
> > 
> > New webrev at
> > http://cr.openjdk.java.net/~tschatzl/8040977/webrev.1/
> > 
> > (Sorry, I already had merged the changes before making a diff webrev -
> > however, most changes in the VM code have been redone anyway. The test
> > case stayed the same).
> > 
> > Thanks,
> > 
> >    Thomas