JDK-8268406 : Deallocate jmethodID native memory
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 18
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2021-06-08
  • Updated: 2024-04-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
Relates :  
Relates :  
Description
Today, when a ClassLoaderData is deallocated, its JNIMethodBlocks are kept around, resulting in a slow memory leak. We do this because JVMTI agents can reuse a jmethodID that belongs to a class that has been deallocated. The JVMTI implementation tolerates this by checking if the jmethodID (which is basically a handle to a Method*) contains a NULL pointer. If so, this indicates that the jmethodID was referring to a Method that has since been deallocated. JVMTI APIs will return JVMTI_ERROR_INVALID_ENVIRONMENT instead of proceeding further. E.g.,

jvmti_GetMethodDeclaringClass(jvmtiEnv* env,
            jmethodID method,
            jclass* declaring_class_ptr) {
  ...
  Method* checked_method = Method::checked_resolve_jmethod_id(method);
  if (checked_method == NULL) {
      return JVMTI_ERROR_INVALID_METHODID;
  }

However, the "regular" JNI APIs (those declared in jni.h) as implemented in HotSpot do not tolerate such invalid jmethodIDs. I have checked all functions under jni.cpp in JDK 16. Every single one of them will crash immediately when an invalid jmethodID is given:

E.g., the following will crash by dereferencing the invalid pointer <m>.

JNI_ENTRY(jobject, jni_ToReflectedMethod(JNIEnv *env, jclass cls, jmethodID method_id, jboolean isStatic))
  ...
  methodHandle m (THREAD, Method::resolve_jmethod_id(method_id));
  ...
  if (m->is_initializer()) {

Therefore, we can conclude that no working JNI code today can use invalid jmethodID without crashing (except for JVMTI agent code). As a result, we can free the JNIMethodBlocks to avoid memory leaks.
Comments
The PoC that I'm working on (which needs performance testing), will look the same as current jmethodIDs. Ie. if the class is unloaded and you try to get the Method from the jmethodID, it will return nullptr, otherwise will return the Method. It just finds it or doesn't find it in a hashtable, and it's not an indirection. https://github.com/openjdk/jdk/pull/14575
08-08-2023

Will this not break JVMTI? We are already having issues with working with stacktraces created by JVMTI - https://bugs.openjdk.org/browse/JDK-8313816. If the jmethodids get deallocated working with JVMTI stacktraces will be a walk in minefield. And situation will be similar for stacktraces collected by ASGCT - there is no practical way to 'lock' all classes which are referenced from a stacktrace in order to make sure the jmethodids will remain valid. Therefore, any attempt to resolve the jmethodid from such stacktrace can crash JVM. Currently, we are trying to work this around by having jmethodID->method metadata cache but this kind of futile endeavour because we can not get any information about class being unloaded so we need to either keep all jmethodids forever and risk OOME or just apply an arbitrary retention, trading the data (we will not be able to resolve old jmethodids) for safety (no JVM crashes).
08-08-2023

[~jiangli] Do you have good performance tests for jmethodID creation?
28-07-2023

Need to make a non-racy jmethodID cache to map Method->jmethodID in order to implement the hashtable idea, so linking to the cleanup JDK-8313332.
28-07-2023

I am not sure how much leak we have in real world programs: - I ran Eclipse and I found jmethodIDs created in custom loaders by JNI code (but a total of only 600 jmethodIDs were created for all loaders). - But I ran a SpringBoot helloworld program, which uses a bunch of custom loaders, and it never creates jmethodIDs for custom loaders. So perhaps we will have a non-trivial memory leak only when running with JVMTI enabled.
08-06-2021

> As an implementation, we need to decide whether the benefit of this RFE (fixing memory leaks) is worth the risk of potential arbitrary execution caused by buggy JNI code. Some input based on my observation: Most the cases that create jmethod IDs are related to JVMTI agents. Agents are more commonly enabled in real world usages in production.
08-06-2021

Note: today jmethodIDs are allocated from the C heap. When a jmethodID becomes invalid (i.e., its class has been deallocated), we keep the jmethodID around, but set its content to NULL. As a result, trying to use such an jmethodID with JNI will predictably lead to a NULL dereference and thus a guaranteed crash. By implementing this RFE, the jmethodID are freed, and its memory can potentially be reused. So we can no longer guarantee the NULL dereference if JNI code uses an "old" jmethodID. The spec allows this: https://docs.oracle.com/en/java/javase/16/docs/specs/jni/design.html#reporting-programming-errors "The programmer must not pass illegal pointers or arguments of the wrong type to JNI functions. Doing so could result in arbitrary consequences, including a corrupted system state or VM crash." As an implementation, we need to decide whether the benefit of this RFE (fixing memory leaks) is worth the risk of potential arbitrary execution caused by buggy JNI code. Risk mitigation: [1] We can keep the existing behavior (do not free jmethodID but NULL its content) when -Xcheck:jni is enabled [2] Write a random value into the jmethodID before freeing it
08-06-2021