RFR: 8069367: assert(_nextMarkBitMap->isMarked((HeapWord*) obj)) failed

[Bengt Rutisson]

On 09/03/15 14:42, Thomas Schatzl wrote:
> Hi Bengt,
>
> On Mon, 2015-03-09 at 14:19 +0100, Bengt Rutisson wrote:
>> On 2015-03-09 13:24, Thomas Schatzl wrote:
>>> Hi Bengt,
>>>
>>> On Mon, 2015-03-09 at 12:49 +0100, Bengt Rutisson wrote:
>>>> Hi Kim,
> [...]
>>>> Your fix will make sure the marking doesn't crash,
>>>> but doesn't this behavior (even prior to your fix) cause other problems?
>>> None that I know. The eager reclaim already made sure that there is no
>>> other reference from a live object to the reclaimed object on the heap,
>>> assuming the remembered sets were correct. So nobody else can
>>> dereference the object.
>>> There is the mentioned race where mark stacks had some references to
>>> these objects left.
>> Ok. So, what happens if I modify the example a little bit?
>>
>> H = new Object[BIGNUMBER];
>> H[4711] = new B();
>> A.h = H;
>> <G1 initial mark>
>> <Marking scans A and pushes H on the mark stack>
>> A.h = A.h[4711];
>> <G1 young GC>
>> <H is reclaimed since no one references it>
>> <Marking continues and finds H on the mark stack but skips it>
>>
>> Who will discover B and mark it live?
> Nobody. This situation cannot occur because we do not reclaim humongous
> object arrays.

Right. Sorry. That's the bit of the puzzle that I was missing. I forgot 
that we don't reclaim object arrays or objects with references.

In that case I guess this approach will work.

One request for the webrev though. The method 
CMTask::is_stale_humongous_queue_entry() does not check that obj is a 
humongous object and I don't think it has any chance of doing that 
either. It can just check that the object looks like it is newly 
allocated and that could only ahppen for reclaimed humongous objects. 
Given that I think I would prefer to not have the method called 
something with humongous.

If we do what Thomas suggested to avoid code duplication, which was to 
move these lines into a separate method:


3822
3823       if (is_stale_humongous_queue_entry(obj)) {
3824         statsOnly( ++stale_humongous_queue_entries );
3825       } else {
3826         assert(!_g1h->is_on_master_free_list(
3827                  _g1h->heap_region_containing(obj)), "invariant");
3828         scan_object(obj);
3829       }
3830

Then maybe we can just inline the code from 
is_stale_humongous_queue_entry() to avoid having to give it a somewhat 
confusing name.

Thanks,
Bengt


>
> See https://bugs.openjdk.java.net/browse/JDK-8048180 for some thoughts
> on how this could be done.
>
> We do not want to reclaim humongous regular objects with references
> either (CR: https://bugs.openjdk.java.net/browse/JDK-8073288) for the
> similar reason, although this is buggy at the moment. See
> https://bugs.openjdk.java.net/browse/JDK-8072598 for a proposed fix.
>
> There is no problem with class unloading either in this case (we need to
> keep alive the j.l.Class/j.l.Classloader of these objects) because the
> j.l.Classloader of large arrays of primitive data lives in the
> system/NULL class loader which is always live.
>
> The situation is a bit trickier if you want to remove large humongous
> regular objects without references (with e.g. j.l.Class X), but it would
> be okay too as far as I can see, because:
> - if the j.l.Classloader is reachable by at least one other j.l.Class of
> the same j.l.ClassLoader, j.l.Class X will stay alive because the
> j.l.ClassLoader  keeps references to all j.l.Class'es it has loaded.
> (This is not what the spec says, but it's what a VM needs to do).
> - if the j.l.Class referenced by this object that we reclaim has really
> been the last j.l.Class of the j.l.Classloader (and neither has been
> marked yet), we are free to unload it because it has really ben
> unreachable.
>
> Thanks,
>    Thomas
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150309/165cd1e2/attachment.htm>