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

[Kim Barrett]

> On Feb 15, 2017, at 10:07 AM, Erik Helin <erik.helin at oracle.com> wrote:
> 
> On 02/15/2017 02:48 AM, David Holmes wrote:
>> Hi Erik,
> 
> Hi David,
> 
> thanks for having a look! Please see new patches at:
> - incremental: http://cr.openjdk.java.net/~ehelin/8168914/00-01/
> - full: http://cr.openjdk.java.net/~ehelin/8168914/01/

------------------------------------------------------------------------------
/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.

------------------------------------------------------------------------------
/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?

------------------------------------------------------------------------------
/src/share/vm/classfile/classLoaderData.hpp
 162   struct ChunkedHandleList : public CHeapObj<mtClass> {

As used, this doesn't seem to need to be CHeapObj.

------------------------------------------------------------------------------
/src/share/vm/classfile/classLoaderData.hpp
 163     struct Chunk : public CHeapObj<mtClass> {
 164       static const size_t CAPACITY = 64;
 165       oop data[CAPACITY];

Large fixed capacity seems problematic?  I think there are lots of
"small" CLDs.  Don't anonymous classes have there own class loader?
A variable sized chunk with a capacity field doesn't seem like it
should introduce any new problems.

The whole Chunk definition could also be moved to the .cpp file, with
only a forward declaration here.

------------------------------------------------------------------------------
/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.