RFR (S): 8038930: G1CodeRootSet::test fails with assert(_num_chunks_handed_out == 0) failed: No elements must have been handed out yet

[Bengt Rutisson]

Hi Thomas,

On 2014-04-15 12:23, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2014-04-14 at 16:56 +0200, Bengt Rutisson wrote:
>> Hi Thomas,
>> On 4/14/14 10:33 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>> On Wed, 2014-04-02 at 16:23 +0200, Thomas Schatzl wrote:
>>>> Hi all,
>>>>
>>>>    can I have reviews for the following fix for a test case?
>>>    Bengt found a race situation where the test fails when compilation is
>>> running while the test is run.
>>>
>>> Instead of trying to find a way to synchronize the test with the
>>> compiler, I changed the code to allow multiple code root chunk free
>>> lists - the test uses one separate from the global one, so there are no
>>> more synchronization problems, and the original change to remember the
>>> number of already allocated code root chunks is superfluous too.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8038930
>>>
>>> Here is a new webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8038930/webrev.1/
>> This looks good to me.
>>
>> I think I would have preferred passing exposing
>> G1CodeRootSet::_default_chunk_manager and explicitly passing that when
>> initializing HeapRegionRemSet::_code_roots rather than having the
>> special treatment of the default value NULL in the G1CodeRootSet
>> constructor. I'll leave it to you to decide.
> This would require me to make the default chunk manager public. I want
> to avoid that. Passing that reference would be the only valid use, no
> other uses of this reference are desired.

Yes, that's what I meant. It's trading one ugliness for another ;)

> If nobody else objects, I would like to keep the code as is.

Ok. I'm fine with leaving it as it is.

Bengt

>
>> Otherwise it looks good.
> Thanks,
>    Thomas
>
>