Review request (S): 7021322: assert(object_end <= top()) failed: Object crosses promotion LAB boundary

Bengt Rutisson bengt.rutisson at oracle.com
Mon Sep 12 20:25:08 PDT 2011


Looks good to me too!

Impressed that you found the issue and fixed it this fast!

If you want to you can update the copyright year to 2011 in 
psPromotionLAB.cpp and psPromotionLAB.hpp.

Bengt

On 2011-09-13 02:21, Ramki Ramakrishna wrote:
> Sorry, that came out all wrong. The filling, as the comment says, is 
> necessary when
> the allocation is not from a PLAB but direct into the space. So that's 
> fine.
>
> The issue is when the object start is in the current PLAB, but its end 
> is not
> equal to the top. It seems as though it should not ever occur that the
> object start is in the PLAB but its end is anywhere other than the top of
> the PLAB. So it would be nice to check for that in the unallocate code.
>
> Sorry for the incorrect comments last time.
> -- ramki
>
> On 9/12/2011 5:14 PM, Ramki Ramakrishna wrote:
>> Good catch. Fix looks good.
>>
>> I am also a bit leery of any unallocate failing. It seems to me that 
>> each unallocate
>> should provably succeed. Thus the "return false" path seems bogus to 
>> me, and should
>> (in my book) be replaced with either an assert(false) or 
>> ShouldNotReachHere.
>> Then the object filling code becomes redundant and can be removed.
>>
>> On the other hand, may be I am missing some reason why the unallocate 
>> might
>> legitimately fail and the object filling becomes necessary in that case.
>>
>> Looks good otherwise.
>> -- ramki
>>
>> On 9/12/2011 1:49 PM, Stefan Karlsson wrote:
>>> http://cr.openjdk.java.net/~stefank/7021322/webrev/
>>>
>>> There's a bug in parallel scavenge when array chunking is used. 
>>> After a thread has succeeded in forwarding the pointer to a newly 
>>> copied array, it might change the length of the old object. This is 
>>> done as a part of the load balancing.
>>> If another thread races with the forwarding thread, it might read 
>>> the incorrect array length and copy just a part of the array. When 
>>> it later sees that the object has already been forwarded and calls 
>>> unallocate_object, it uses the original length of the array to 
>>> determine the size to unallocate.
>>>
>>> The fix is to pass the actual amount of memory that was allocated, 
>>> to unallocate_object.
>>>
>>> Tested with the failing test.
>>>
>>> StefanK



More information about the hotspot-gc-dev mailing list