RFR: 8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking

[David Holmes]

On 16/02/2017 5:46 PM, Kim Barrett wrote:
>> On Feb 16, 2017, at 2:41 AM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> On 16/02/2017 3:53 PM, Kim Barrett wrote:
>>>>> /src/share/vm/classfile/classLoaderData.cpp
>>>>> 138     const juint size = OrderAccess::load_acquire(&c->size);
>>>>>
>>>>> I think all but the first chunk have a constant size of the
>>>>> maximum size, so no need for acquire except for the first.
>>>>
>>>> The size is incremented on each add, so this acquire sync's with the release of the new size, so that we are sure to see the oop that was just added.
>>>>
>>>> Though given the races between add and oops_do it may not actually matter.
>>>
>>> By “first” I mean the first chunk.  only the first chunk’s size is subject to modification.  The release_store of _head that made the first chunk accessible will be after the setting of the next chunk’s size to its final (max) value.  The corresponding load_acquire of _head ensures any _next chunks are visibly complete.  It’s the same rationale for _next not requiring any special handling. At least, that’s how it looks to me right now; am I missing something.
>>
>> The _head chunk has its _size incremented on each add().
>>
>> David
>
> Agreed, and a load_acquire is needed for the _head chunk.
>
> I’m suggesting an ordinary load is sufficient for any _next chunks as we loop down the list.

Ah! Now I get what you mean - sorry. Yes only the first loop iteration 
needs the load-acquire. So just:

Chunk* c = (Chunk*) OrderAccess::load_ptr_acquire((volatile 
intptr_t*)&_head);
const juint size = (c == NULL ? 0 : OrderAccess::load_acquire(&c->size));
while (c != NULL) {
     size = OrderAccess::load_acquire(&c->size);
     ...
}


Cheers,
David
>