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

[Erik Österlund]

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

> Okay I agree that you can't use a Handle to reference this mirror if it's not already referenced by other code (already alive).  Fetching out of the CLD::_handles doesn't keep it alive.
> 
> 
> 
>     // method - pre-checked for validity, but may be null meaning obsolete method
> 
>     // declaring_class_ptr - pre-checked for null
> 
>     jvmtiError
> 
>     JvmtiEnv::GetMethodDeclaringClass(Method* method, jclass* declaring_class_ptr) {
> 
>       NULL_CHECK(method, JVMTI_ERROR_INVALID_METHODID);
> 
>       (*declaring_class_ptr) = get_jni_class_non_null(method->method_holder());
> 
>       return JVMTI_ERROR_NONE;
> 
>     } /* end GetMethodDeclaringClass */
> 
> 
> 
> So here, I don't see anything holding the method_holder() mirror through the Method, unless it's in the caller (a global jobject or something).  Same with the GetFieldDeclaringClass function.

Exactly. As for the GetFieldDeclaringClass method, the XSL generated C++ code that calls it has a jclass of the relevant class or a subclass of it, which is fine in terms of ensuring the holder is kept alive. So it's really GetMethodDeclaringClass that is missing something. Its caller (also XSL generated C++ code) checks that the Method* has not been cleared in the jmethodID handle, and bails if the CLD is not alive. But nowhere do we call klass_holder() which is what safely reads the holder and ensures it is made strongly reachable.

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

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