RFR: 8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking (original) (raw)

David Holmes david.holmes at oracle.com
Tue Feb 21 02:05:09 UTC 2017

On 21/02/2017 11:57 AM, Kim Barrett wrote:

On Feb 20, 2017, at 5:52 PM, David Holmes <david.holmes at oracle.com> wrote:

02 -> 03: - Only use loadacquire when reading the size for the "head" chunk

Sorry but I don't like the way this is done - the conditional may end up being more expensive than the unnecessary load-acquire. Unrolling the first loop iteration, as per the email discussion, is a better way to go IMO. +1 And as Kim indicated you shouldn't need any casts for the OrderAccess ops as you should be using the "void*" versions.

- Fix grammar in comment - Refer to the correct lock in comment

AFAICS (scanning through all the emails) there are now only three remaining comments: - Thomas: can we assert in ChunkedHandeList::add that the correct mutex is locked? - Then we would have to pass the mutex along as a parameter to add, Why do you need to pass it in? oop* ClassLoaderData::ChunkedHandleList::add(oop o) { assert(ClassLoaderData::metaspacelock()->ownedbyself(), "should own lock"); The metaspacelock() is per-CLD, so that doesn’t work.

Ah! The use of ClassLoaderData::metaspace_lock() in the comment threw me. So we could have the ChunkHandleList store a back-pointer to the CLD that owns it. But for a single method with a single caller this seems like overkill.

BTW not sure about nested classes in C++ but does anything in ChunkHandleList need to be public?

Thanks, David

More information about the hotspot-dev mailing list