RFR(M): 7009266: G1: assert(obj->is_oop_or_null(true )) failed: Error

[John Cuthbertson]

Hi Everyone,

A new webrev for this fix can be found at: 
http://cr.openjdk.java.net/~johnc/7009266/webrev.5/

The changes in this revision include:

* Revised barrier set calls in JNI_GetObjectField and Unsafe_getObject 
so that the value that is returned is the value that gets recorded in an 
SATB buffer. The code in the previous revision re-read the value of the 
field potentially causing the value the is logged to be different from 
that returned.

* Added the static checks, suggested by Tom, in library_call.cpp.

* Removed the complicated control flow from the C1 LIR implementation of 
Unsafe.getObject. Instead the checks have been moved into a code stub 
while some static checks have been added.

* Re-enabled reference discovery (and therefore reference processing) 
during concurrent marking as a result of pushing the changes for 7020042 
to hs21.

Thanks,

JohnC

On 03/28/11 10:35, John Cuthbertson wrote:
> Hi Everyone,
>
> A new webrev with changes based upon comments from Tom can be found 
> at: http://cr.openjdk.java.net/~johnc/7009266/webrev.4/.
>
> The latest changes include inserting a suitably guarded barrier call 
> in case the referent field of a Reference object is being read/fetched 
> using JNI, reflection, or Unsafe.
>
> Thanks,
>
> JohnC
>
> On 3/11/2011 5:34 PM, John Cuthbertson wrote:
>> Hi Everyone,
>>
>> I'm looking for a few of volunteers to review the changes that fix 
>> this assertion failure. The latest changes can be found at: 
>> http://cr.openjdk.java.net/~johnc/7009266/webrev.3/ and include 
>> changes based upon earlier internal reviews. The earlier changes are 
>> also on cr.openjdk.java.net for reference.
>>
>> Background:
>> The G1 garbage collector includes a concurrent marking algorithm that 
>> makes use of snapshot-at-the-beginning or SATB. With this algorithm 
>> the GC will mark all objects that are reachable at the start of 
>> marking; objects that are allocated since the start of marking are 
>> implicitly considered live. In order to populate the "snapshot" of 
>> the object graph that existed at the start of marking, G1 employs a 
>> write barrier. When an object is stored into another object's field 
>> the write-barrier records the previous value of that field as it was 
>> part of the "snapshot" and concurrent marking will trace the 
>> sub-graph that is reachable from this previous value.
>>
>> Unfortunately, in the presence of Reference objects, SATB might not 
>> be sufficient to mark a referent object as live. Consider that, at 
>> the start of marking, we have a weakly reachable object i.e. an 
>> object where the only pointer to that object. If the referent is 
>> obtained from the Reference object and stored to another object's 
>> field (making the referent now strongly reachable and hence live) the 
>> G1 write barrier will record the field's previous value but not the 
>> value of the referent.
>>
>> If the referent object is strongly reachable from some other object 
>> that will be traced by concurrent marking, _or_ there is a subsequent 
>> assignment to the field where we have written the referent (in which 
>> case we record the previous value - the referent - in an SATB buffer) 
>> then the referent will be marked live. Otherwise the referent will 
>> not be marked.
>>
>> That is the issue that was causing the failure in this CR. There was 
>> a Logger object that was only reachable through a WeakReference at 
>> the start of concurrent marking. During marking the Logger object is 
>> obtained from the WeakReference and stored into a field of a live 
>> object. The G1 write barrier recorded the previous value in the field 
>> (as it is part of the snapshot at the start of marking). Since there 
>> was no other assignment to the live object's field and there was no 
>> other strong reference to the Logger object, the Logger object was 
>> not marked. At the end of concurrent marking the Logger object was 
>> considered dead and the link between the WeakReference and the Logger 
>> was severed by clearing the referent field during reference processing.
>>
>> To solve this (entirely in Hotspot and causing a performance overhead 
>> for G1 only) it was decided that the best approach was to intrinsify 
>> the Reference.get() method in the JIT compilers and add new 
>> interpreter entry points so that the value in the referent field will 
>> be recorded in an SATB buffer by the G1 pre-barrier code.
>>
>> The changes for Zero and the C++ interpreters are place holder 
>> routines but should be straight forward to implement.
>>
>> None of the individual changes is large - they are just well 
>> distributed around the JVM. :)
>>
>> Testing: white box test; eyeballing the generated compiled and 
>> interpreter code; the failing Kitchensink big-app on x86 (32/64 bit), 
>> sparc (32/64 bit), Xint, Xcomp (client and server), with and without 
>> G1; the GC test suite with and without G1; and jprt.
>>
>> Thanks and regards,
>>
>> JohnC
>