RFR(S) 8153843: G1CardLiveDataHelper incorrectly sets next_live_bytes on dead humongous regions

[Thomas Schatzl]

Hi,

On Mon, 2016-04-11 at 16:15 +0200, Mikael Gerdin wrote:
> Hi,
> 
> On 2016-04-08 20:53, Thomas Schatzl wrote:
> > Hi,
> > 
> > On Fri, 2016-04-08 at 12:09 -0400, Tom Benson wrote:
> > > Hi Mikael,
> > > It looks like there is an indentation problem at lines 214/215.
> > > Dumb question:  The code now extends the range down to the object
> > > start, if current region continues_humongous.  Do you want to
> > > extend
> > > to the true end region of the object, which might not be the
> > > current
> > > one?
> > 
> >    neither "hr" nor "start" are reassigned as they are used for the
> > actual marking.
> > 
> > Other issues:
> >    - start_region can always use humongous_start_region(). For
> > humongous
> > starts regions, humongous_start_region() points to itself.
> 
> Fixed.
> 
> > 
> >    - the change misses that during humongous object registration
> > for
> > eager reclaim the klass we access to determine the kind of the
> > object
> > can already be invalid.
> > 
> > I saw some of the crashes were during humongous candidate region
> > selection. My suggestion on fixing this is to replace
> > g1CollectedHeap.cpp:~3003 with
> > 
> > bool is_candidate = !g1h->is_obj_dead((oop)(r->bottom())) &&
> > humongous_region_is_candidate(g1h, r);
> > 
> > These two changes also seem to make JDK-8153834 disappear at least 
> > when running previously failing tests locally too :)
> 
> Ok, I've decided to add that to the fix as well.
> I believe that there is a possibility that between the remark pause, 
> where classes are unloaded, and the cleanup pause, where humongous 
> regions potentially containing dead classes, a young GC with eager 
> reclaim could attempt to inspect the class pointer of a 
> dead-but-not-yet-free humongous object and if that class pointer is
> no 
> longer valid due to class unloading we crash.
> 
> 
> Unfortunately I've reproduced 8153834 with my patches applied so that
> appears to be a different issue.
> 
> Webrev: http://cr.openjdk.java.net/~mgerdin/8153843/webrev.1/

Ok. Looks good, thanks.

> > > On 4/8/2016 11:47 AM, Mikael Gerdin wrote:
> > > > Hi all,
> > > > 
> > > > Please review this small change to solve an integration blocker
> > > > in
> > > > hs-rt which surfaced with the latest integration from main.
> > > > 
> > > > The problem is that humongous regions are kept around for
> > > > longer
> > > > (possibly indefinitely) because the card live data helper
> > 
> > Eager reclaim will catch them, as it only considers reachability.
> 
> Eager reclaim only considers primitive type arrays, so object arrays 
> or humongous non-arrays will not be reclaimed, right?

Agree.

Thomas