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

Zhengyu Gu zgu at openjdk.org
Sat Jul 13 16:00:57 UTC 2024


On Fri, 28 Jun 2024 15:06:04 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> src/hotspot/share/classfile/classLoaderData.cpp line 184:
>> 
>>> 182: 
>>> 183: ClassLoaderData::ChunkedHandleList::~ChunkedHandleList() {
>>> 184:   Chunk* c = Atomic::load_acquire(&_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.
>
> @zhengyu123 I still would like to see this line getting reverted.

I believe that is exactly what @stefank and @fisk tried to point out: the relationship must have been established. Otherwise, we have bigger problem. 

I agree with them. None concurrent GCs unload classes at a safepoint and concurrent GCs rendezvous all threads (e.g. https://github.com/openjdk/jdk/blame/master/src/hotspot/share/gc/z/zGeneration.cpp#L1344-L1350) before purging dead class loaders.

My intention of adding `Atomic::load_acquire()` is to make code easy to reason without above knowledge, and I don't believe it has much performance impact.

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

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


More information about the hotspot-runtime-dev mailing list