RFR: 8335217: Fix memory ordering in ClassLoaderData::ChunkedHandleList [v2]

[Stefan Karlsson]

On Thu, 27 Jun 2024 13:26:33 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

> > Did you see that ChunkedHandleList::_size is also updated concurrently? If there's truly a concurrency order that this fixes, then maybe we should at least use Atomic::load when we're reading the _size?
> 
> You mean `ChunkedHandleList::Chunk::_size`, right?

Oops. Right.

> 
> It is updated via `Atomic::release_store()` [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/classfile/classLoaderData.cpp#L199) and read via `Atomic::load_acquire()` [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/classfile/classLoaderData.cpp#L223).
> 
> And now, I believe it also needs `Atomic::load_acquire()` for [this read](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/classfile/classLoaderData.cpp#L225) 

I think the first load_acquire covers the rest of the memory further down the list. So, I don't think it is required there.

> as well and I agree that should use `Atomic::load()` to make it more explicit.

I don't think it is needed in ChunkedHandleList::add because it is modified under a lock. ChunkedHandleList::count should be using `Atomic::load`.

FWIW, I find it odd that we would need a load_acquire in the destructor. We shouldn't have concurrently modifying threads if the CLD is about to be deleted.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19919#issuecomment-2194720215