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

[Erik Österlund]

On Mon, 9 Sep 2024 14:44:38 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> I think this is wrong.  We want 'resolve' to read the oop and tell the GC that this oop should be alive.  If GetMethodDeclaringClass is crashing, make sure the holder is alive before the iteration could safepoint.
> 
> 
> 
> This code does keep it in a Handle, so that should be sufficient.
> 
> 
> 
>     jclass
> 
>     JvmtiEnvBase::get_jni_class_non_null(Klass* k) {
> 
>       assert(k != nullptr, "k != null");
> 
>       Thread *thread = Thread::current();
> 
>       return (jclass)jni_reference(Handle(thread, k->java_mirror()));
> 
>     }
> 
> 
> 
> 

The trouble is that java_mirror does not keep the holder alive. It's an OopHandle to an oop in the CLD. Using ths OopHandle at all is only allowed if the holder is in some other way guaranteed to be held strongly reachable.

The naming of the java_mirror* functions are a bit unfortunate. It's tempting to believe java_mirror will keep the mirror alive when there is another java_mirror_no_keepalive next to it. But it does not, and that is intentional. A better name would perhaps be java_mirror_already_kept_alive or something like that. Neither one of them keeps the class alive.

This can be made prettier once non-generational ZGC is removed. But until then I'm hoping we can do what the other code is doing which is to call klass_holder() to keep the class alive, and then fetch the mirror separately knowing it is safe.

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

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