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

[Liang Mao]

On Mon, 9 Sep 2024 16:59:18 GMT, Erik Österlund <eosterlund 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.
>
>> 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.

@fisk @stefank , I used the klass_holder to keep it alive and check alive via is_loader_alive. Thanks for the suggestion!

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

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