RFR: 8339725: Concurrent GC crashed due to GetMethodDeclaringClass [v2]

[Erik Österlund]

On Mon, 9 Sep 2024 06:04:36 GMT, Liang Mao <lmao at openjdk.org> wrote:

>> Hi,
>> 
>> It's a fix for 8339725. I think getting the oop from Klass::java_mirror() should use a ON_PHANTOM_OOP_REF decorator here which could make sure the oop would be kept alive in concurrent marking and return nullptr while in concurrent reference processing and unloading.
>> 
>> test/hotspot/jtreg/runtime and gc are clean.
>> 
>> Thanks,
>> Liang
>
> Liang Mao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix build error in windows/mac

> Holding a mirror in a Handle must keep the CLD alive.  If it has an oop class loader, it will keep a reference to that class loader alive through the vectors field.  If it is a mirror holder CLD, the holder is the mirror itself.  You can only unload the CLD if the CLD::holder is unreachable.  This Handle makes the CLD holder reachable.
> 
> 
> 
> Not having this property breaks everything.

A handle ensures that an oop that was strongly reachable at the point when the handle was created, keeps the oop strongly reachable across the lifetime of the handle. But it cannot guarantee that an oop that was not strongly reachable at the point of creating the handle, will be kept alive by the handle. That's an illegal use of the handle.

What is happening here is that we load one of the weird CLD oops that we are not allowed to load unless the holder is already kept strongly reachable. That means that we are loading an oop that is not strongly reachable at the point of loading it, and illegally putting it in a handle, which on its own would crash the JVM later if the GC tried to actually trace through the handle.

What we are doing conceptually is the equivalent of peeking past a non-strong reference to an object  and then loading one of its strong references and exposing it in the object graph through a handle, which is illegal. Just because we read from a strong edge, that doesn't mean the oop is strongly kept alive. Because we only got hold of the strong reference by peeking through a non-strong reference.

In this case the non-strong reference is the klass_holder that we skipped loading, and the strong reference of the holder is the klass_mirror weird CLD oop from the OopHandle that logically belongs to the holder, which we didn't load in an appropriate manner.

You gotta love the weird CLD oops.

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

PR Comment: https://git.openjdk.org/jdk/pull/20907#issuecomment-2338532271