RFR: 8031703 - Missing post-barrier in ReferenceProcessor

[Per Liden]

Hi Tony,

On 01/29/2014 11:06 PM, Tony Printezis wrote:
> Per,
>
> Good catch. Quick question: Is there a reason why pre-barriers are not
> needed here?

Thanks for looking at it.

My reason for not needing a pre-barrier there goes like this:

The Reference was inserted into the discovered list (ref_list) because 
it was discovered during evacuation or marking and it's thus strongly 
reachable from some other place and is marked (or will be marked) live. 
If we later remove the Reference from the discovered list we don't need 
to enqueue it for SATB because we know it has already been found/marked, 
that's how it ended up in the discovered list in the first place.

Makes sense?

Also note that if we had a pre-barrier there it would cause
verify_no_cset_oops() to fail if we get an young collection during 
concurrent mark.

cheers,
/Per

>
> Tony
>
> On 1/28/14, 4:57 AM, Per Liden wrote:
>> http://cr.openjdk.java.net/~pliden/8031703/webrev.0/
>>
>> https://bugs.openjdk.java.net/browse/JDK-8031703
>>
>> Summary:  In ReferenceProcessor::process_discovered_references() the
>> lists of discovered references are pruned (a reference who's referent
>> is still alive is unlinked, etc). During this pruning the
>> Reference.discovered field is rewritten without a post-barrier (in
>> DiscoveredListIterator::remove()), which means we fail to dirty the
>> appropriate card. This only affect G1's CM reference processor, which
>> is the only case where the reference processor needs post-barriers for
>> the discovered list (i.e. _discovered_list_needs_barrier is true). The
>> window for this to happen was widened after fixing JDK-8029255. Before
>> that fix the appropriate card was usually dirtied by pure luck because
>> of a write (in a different context) to the Reference.next field in the
>> same object. However, if the object was unlucky and the
>> next/discovered fields spanned two different cards the problem would
>> have appeared. The patch also corrects the post-barrier in
>> ReferenceProcessor::set_discovered(), which used an incorrect field
>> offset or 0 instead of discovered_offset.
>>
>> The problem could be easily reproduced by running Kitchensink on a
>> 32-bit x86 JVM with G1. The bug is however not 32-bit related, it just
>> happened to expose the problem more frequently.
>>
>> Testing:
>> Kitchensink 32-bit x86 JVM with G1 (6 hours)
>> Kitchensink 64-bit x86 JVM with G1 (30 minutes)
>> Kitchensink 32-bit x86 JVM with CMS (30 minutes)
>>
>> /Per
>>
>