CRR (S): 7121623: G1: always be able to reliably calculate the length of a forwarded chunked array

[Tony Printezis]

Hi all,

I have some bad news, it turns out that there's a subtle race in the 
code. I've been testing the marking changes for quite a while now (which 
rely on this patch) and I got a single failure. I'm pretty sure it's the 
race I describe below. BTW, there's a question at the end of the e-mail. :-)

Let's assume two threads, A and B, are racing to forward the same object 
array X. Consider the following interleaving:

Thread A:
is X forwarded? no

Thread B:
is X forwarded? no
size = X.size();
X' = allocate size
try to forward X to X'? yes
copy X to X'
chunk X, X.length is negative

Thread A:
size = X.size(); -> BOOM, as size() just read a negative length

Interesting this race exists today but it's benign due to either careful 
design or sheer luck. When A reads size = X.size() it will actually get 
a smaller size than the actual size of X and will allocate a chunk 
smaller than X (!!!). However, given that X is already forwarded it 
won't need to copy it and will undo the allocation. So, no harm done.

Question: Is there a reason why we use the from-space length field to 
encode the next index instead of the to-space length field? If we used 
the latter we can simplify the code by quite a lot. I can't immediately 
think of any issues that this might cause.

Tony

On 12/21/2011 5:25 PM, Tony Printezis wrote:
> Hi all,
>
> I have two code reviews (thanks to John and Ramki!). Can I push or is 
> anybody else still looking at this?
>
> Tony
>
> On 12/14/2011 05:17 PM, Tony Printezis wrote:
>> Hi all,
>>
>> Can I have a couple of code review for this small change?
>>
>> http://cr.openjdk.java.net/~tonyp/7121623/webrev.0/
>>
>> The CR has a bit more explanation. The short version is that I'm now 
>> encoding the "start index of the next chunk" in the from-space length 
>> field of a chunked array (say *that* quickly!) as a negative value to 
>> always be able to distinguish it from the real length. This will 
>> simplify the code for the CR John is currently working on (6484965) 
>> but it's a small, self-contained change so it's good to get it code 
>> reviewed and pushed separately.
>>
>> Tony
>>
>>