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

Thomas Schatzl thomas.schatzl at oracle.com
Thu Feb 16 08:55:17 UTC 2017

Hi,

On Thu, 2017-02-16 at 18:09 +1000, David Holmes wrote:

Bah! didn't finish my edit ...

On 16/02/2017 5:54 PM, David Holmes wrote: > > 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::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 oopsdo it may not > > > > > actually > > > > > matter. > > > > By “first” I mean the first chunk.  only the first chunk’s > > > > size is > > > > subject to modification.  The releasestore 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 > > > > loadacquire 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 loadacquire 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::loadptracquire((volatile > intptrt*)&head); > const juint size = (c == NULL ? 0 : OrderAccess::loadacquire(&c- > >size)); > while (c != NULL) { >     size = OrderAccess::loadacquire(&c->size);  size = c->size;

Maybe I'm wrong, but just looking at this suggestion without more thought: what if the value of size changes just between the above load_acquire and the re-read of the size, and the actual value in the array has not been updated yet?

If that optimization is done, please just peel out the first iteration.

Thanks,   Thomas

More information about the hotspot-dev mailing list