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