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

David Holmes david.holmes at oracle.com
Thu Feb 16 03:31:53 UTC 2017

Hi Kim,

If I may ...

On 16/02/2017 12:41 PM, Kim Barrett wrote:

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::loadacquire(&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.

David

------------------------------------------------------------------------------ /src/share/vm/classfile/classLoaderData.cpp 173 VerifyContainsOopClosure cl(op); 174 oopsdo(&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 { As used, this doesn't seem to need to be CHeapObj. ------------------------------------------------------------------------------ /src/share/vm/classfile/classLoaderData.hpp 163 struct Chunk : public CHeapObj { 164 static const sizet 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.

More information about the hotspot-dev mailing list