RFR: JDK-8268088: Clarify Method::clear_jmethod_ids() related comments in ClassLoaderData::~ClassLoaderData() [v2]

[Ioi Lam]

On Mon, 7 Jun 2021 20:00:40 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>>> Do we have any use cases of what a real world application can do with a NULL'ed out jmethodID (other than JVM TI will give an JVMTI_ERROR_INVALID_METHODID for it)?
>>> All or most of the JNI functions will crash with a NULL'ed out jmethodID. You can catch this error by using -Xcheck:jni. Maybe we could deprecate this feature of the JVMTI spec and release the memory?
>> 
>> The JVMTI agent use case that ran into issues (with releasing memory) calls GetMethodDeclaringClass for a stale jmethod_id (with unloaded class and loader) and crashes in Method::checked_resolve_jmethod_id when it's dereferencing the 'mid'.
>> 
>> The agent native code does check for JVMTI_ERROR_NONE after calling GetMethodDeclaringClass in this case, but it's too late.
>
>> This comment looks like an expanded version of the one I wrote a long time ago and correct. 
> 
> Thanks for confirming the correctness!
> 
>> We were wondering if this is a feature that we actually need.
> 
> That seems to be the case. I learned this the hard way with crashes observed in production when attempting to release memory for stale jmethod_ids from VM side. Clarifying the comments hopefully will benefit others and avoid similar issues in the future.

If the issue is specific to JVMTI code only, the comment should reflect that. Currently it seems to suggest that all possible JNI code could use an invalid jmethodID. However, using an invalid jmethodID with JNI APIs such as CallStaticVoidMethod will lead to a crash. I don't think the JNI spec allows invalid jmethodID to be used.

Furthermore, we should consider freeing the JNIMethodBlock when JVMTI is not enabled, so we can avoid the memory leak.

We may even want to update the JVMTI spec to disallow invalid jmethodID, but will that introduce incompatibility with 3rd party code?

Lastly, do we have any existing test cases that cover the use of invalid jmethodIDs?

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

PR: https://git.openjdk.java.net/jdk/pull/4383