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

[Kim Barrett]

> On Feb 16, 2017, at 8:18 AM, Erik Helin <erik.helin at oracle.com> wrote:
> 
> Hi Kim,
> 
> thanks for reviewing!
> 
> On 02/16/2017 03:41 AM, 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.
> 
> Please see my reply to my David. I agree that this can be optimized, but I'm not sure if it is worth it (the code will not become harder to understand)?

I knew when I made the suggestion that it might increase the raw footprint of the source code.  But I agree with David’s sentiment that removing dynamically unnecessary load_acquires helps readability.

>> ------------------------------------------------------------------------------
>> /src/share/vm/classfile/classLoaderData.cpp
>> 173   VerifyContainsOopClosure cl(op);
>> 174   oops_do(&cl);
>> 
>> We don't care that this iterates over the entire list even if found
>> early?
> 
> Please see my reply to Thomas. We don't care :)

That’s what I guessed, hence making it a question.  Okay.

> 
>> ------------------------------------------------------------------------------
>> /src/share/vm/classfile/classLoaderData.cpp
>> 626   return (jobject) _handles.add(h());
>> and
>> 631   *((oop*) h) = NULL;
>> 
>> Having spent a bunch of time recently dealing with and cleaning up a
>> bunch of places that break the jobject abstraction (though by no means
>> all), I'm not happy to see new ones.
>> 
>> As Coleen said, there's discussion about how to refer to indirect
>> pointers to oops in runtime code.  jobject has been used as an answer
>> for that in some places, but they might need to be changed.  Hiding
>> the conversions as casts will just make that harder.  I'm thinking
>> some named conversion functions are in order, instead of bare casts.
> 
> Hmm, I would not like to introduce too much of Coleen's work in this patch. Do you have something minimal in mind that you think improves the situation? Or should we just note this place in the code and come back to it in 10?

Some sort of “convert_x_to_y” functions in JNIHandles.  I added parts of one direction in 8166188, though that change has run into problems because more testing uncovered more abstraction breakage. 

> 
> Thanks,
> Erik