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

[Stefan Karlsson]

On Thu, 27 Jun 2024 20:51:32 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> ClassLoaderData::ChunkedHandleList's head is installed via Atomic::release_store(). Therefore, readers need Atomic::Atomic::load_acquire() barrier to access its members safely.
>
> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert some of previous changes

src/hotspot/share/classfile/classLoaderData.cpp line 184:

> 182: 
> 183: ClassLoaderData::ChunkedHandleList::~ChunkedHandleList() {
> 184:   Chunk* c = _head;

This really should not be needed and just becomes something that people will wonder about why it is there. If we ever have concurrent accesses here we have a use-after-free situation and have much bigger issues to deal with. I'd like to see this reverted.

src/hotspot/share/classfile/classLoaderData.cpp line 261:

> 259: #ifndef PRODUCT
> 260: bool ClassLoaderData::ChunkedHandleList::owner_of(oop* oop_handle) {
> 261:   Chunk* chunk = Atomic::load_acquire(&_head);

It's unclear to me if this is actually concurrently accessed, but if we want to be conservative and add it here, then that's fine. However, then I'd also like to see an Atomic::load of chunk->_size.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19919#discussion_r1658456870
PR Review Comment: https://git.openjdk.org/jdk/pull/19919#discussion_r1658459121