CRR (S): 7121623: G1: always be able to reliably calculate the length of a forwarded chunked array
tony.printezis at oracle.com
Fri Dec 23 00:37:12 PST 2011
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:
is X forwarded? no
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
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.
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?
> On 12/14/2011 05:17 PM, Tony Printezis wrote:
>> Hi all,
>> Can I have a couple of code review for this small change?
>> 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.
More information about the hotspot-gc-dev