JDK-8268364 : jmethod clearing should be done during unloading
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2021-06-08
  • Updated: 2024-10-02
  • Resolved: 2021-07-02
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.
JDK 17 JDK 18
17.0.14Fixed 18 b05Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
jmethodIDs are basically handles of Method*.

In generated JVMTI functions such as jvmti_GetMethodDeclaringClass, we will first check if a jmethodID is valid (i.e., if it contains a non-NULL Method*). If so, we will dereference the Method*:

===================================
jvmti_GetMethodDeclaringClass(jvmtiEnv* env,
            jmethodID method,
            jclass* declaring_class_ptr) {
  if(JvmtiEnv::get_phase(env)!=JVMTI_PHASE_START && JvmtiEnv::get_phase()!=JVMTI_PHASE_LIVE) {
    return JVMTI_ERROR_WRONG_PHASE;
  }
  Thread* this_thread = Thread::current_or_null(); 
  if (this_thread == NULL || !this_thread->is_Java_thread()) {
    return JVMTI_ERROR_UNATTACHED_THREAD;
  }
  JavaThread* current_thread = this_thread->as_Java_thread();
  MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, current_thread));
  ThreadInVMfromNative __tiv(current_thread);
  VM_ENTRY_BASE(jvmtiError, jvmti_GetMethodDeclaringClass , current_thread)
  debug_only(VMNativeEntryWrapper __vew;)
  PreserveExceptionMark __em(this_thread);
  JvmtiEnv* jvmti_env = JvmtiEnv::JvmtiEnv_from_jvmti_env(env);
  if (!jvmti_env->is_valid()) {
    return JVMTI_ERROR_INVALID_ENVIRONMENT;
  }
  jvmtiError err;
  Method* checked_method = Method::checked_resolve_jmethod_id(method);
  if (checked_method == NULL) {
      return JVMTI_ERROR_INVALID_METHODID;
  }
  if (declaring_class_ptr == NULL) {
      return JVMTI_ERROR_NULL_POINTER;
  }

  <<<<<<< ZGC can concurrently deallocate the checked_method

  err = jvmti_env->GetMethodDeclaringClass(checked_method, declaring_class_ptr);  <<<<< crash
  return err;
}


===================================
 
This in not compatible with GCs that perform concurrent class deallcation (such as ZGC). If <method> belongs to a class that has been collected (see JDK-8268088), but not yet deallocated, <checked_method> could be non-NULL, but will be concurrently deallocated by ZGC. So by the time we call into jvmti_env->GetMethodDeclaringClass, <checked_method> already points to bad memory.

Note: ZGC performs metadata deallocation outside of a safepoint. See JDK-8267879 for a case where ClassLoaderData::~ClassLoaderData() is called by ZGC while a safepoint is still in session.
Comments
[jdk11u-fix-request] Approval Request from Kerem Kat All checks passing. The fix is required to allow JVM TI tools, especially profilers, to run safely with class unloading enabled. Low risk: a small localized change that only affects liveness of jmethodIDs.
02-10-2024

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk11u-dev/pull/2935 Date: 2024-09-18 15:10:33 +0000
18-09-2024

See JDK-8339725 for more issues in this area
11-09-2024

[17u-fix-yes] The related issues are either fixed in 17 or duplicates. Approving.
11-09-2024

[17u-fix-yes] The related issues are either fixed in 17 or duplicates. Approving.
11-09-2024

[jdk17u-fix-request] Approval Request from Kerem Kat The fix is required to allow JVM TI tools, especially profilers, run safely with class unloading enabled. Low risk: a small localized change that only affects liveness of jmethodIDs.
09-09-2024

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk17u-dev/pull/2839 Date: 2024-09-02 10:31:10 +0000
05-09-2024

Changeset: 3d84398d Author: Coleen Phillimore <coleenp@openjdk.org> Date: 2021-07-02 18:04:45 +0000 URL: https://git.openjdk.java.net/jdk/commit/3d84398d128bb2eed6280ebbc3f57afb3b89908f
02-07-2021

It's not really a ZGC issue. This is more of a runtime issue. Feel free to reassign it to us. I think the title should be changed to jmethodID clearing too late. The late clearing creates a race when using collectors like ZGC that can purge metaspace while JVMTI is looking at jmethodIDs.
09-06-2021

I think, this issue has to be moved to ZGC team after some additional investigation and prove. Targeting this to TBD for now.
09-06-2021

I agree with Coleen, this looks like a race in ZGC. [~jiangli] Thank you for the comment. [~iklam] Thank you for catching and filing this issue! However, I doubt, the approach to require JVM TI agents to keep jmethodID's alive you suggest would really work as it breaks compatibility. A jmethodID can become invalid if class is unloaded as stated here: http://cr.openjdk.java.net/~sspitsyn/webrevs/2021/jvmti-deprecate-heap-funcs.1/jvmti.html#jmethodID Do you have a test case reproducing this issue?
09-06-2021

>A JVMTI agent should not rely on JVMTI_ERROR_INVALID_METHODID when saving aside a jmethodID (e.g., from the results of GetStackTrace). Instead, it should ensure that the classloader associated with this jmethodID is not GCed. This can be done by creating a GlobalRef on the classloader: > >- GetMethodDeclaringClass -> GetClassLoader -> NewGlobalRef > >The agent can release the GlobalRef when it no longer references the jmethodID. This shouldn't be a recommended solution for users, because it would keep more memory for longer time. The agent behavior would dictate the memory usages of the VM and prevent classes from being unloaded unnecessarily. If an agent only processes all jmethod_ids at the end, it would keep all loaders alive without being able to be unloaded. Also, a bug (fail to release any of the GoabalRef) in the agent might cause memory leaks.
08-06-2021

This bug report shows a race in ZGC. The jmethodIDs are cleared in ~ClassLoaderData after the metaspace is returned, so that this line can get a bad pointer before the jmethodIDs are cleared: Method* checked_method = Method::checked_resolve_jmethod_id(method); The jmethodIDs should be cleared during unload not purge. I think you should post your previous comment as a separate RFE as an idea to be able to deallocate jmethodIDs.
08-06-2021

I think we should require the JVMTI agents to keep the jmethodIDs alive. [1] Add a new JVMTI capability can_handle_concurrent_class_unloading. Let's call the JVMTI agents that don't implement this capability as "old agents". [2] If the VM has not loaded any old agents, it's safe to free the JNIMethodBlocks [3] Old agents should be deprecated and eventually not supported. We can do something like this to encourage migration JDK 18 - print warning message when loading old agents with GCs that perform concurrent class deallcation (such as ZGC). JDK 19 - refuse to load old agents with ZGC, etc, print warning messages with all other GCs JDK 20 - refuse to load old agents with all GCs. Eventually, JNIMethodBlocks will be unconditionally freed when a class loader is deallocated. If a JVMTI agent attempts to use a jmethodIDs from a deallocated class, the behavior is unspecified (same as in JNI).
08-06-2021

I updated the bug description to include the entire body of jvmti_GetMethodDeclaringClass. declaring_class_ptr is used for returning the class that declares the jmethodID. It's not set until we get into jvmti_env->GetMethodDeclaringClass(...).
08-06-2021

If declaring_class_ptr is a pointer to the class owning the Method, it's a strong reference and won't be unloaded.
08-06-2021

I don't think this is specific to ZGC but rather a consequence of a concurrent GC. If a class is not referenced whilst jmethodIDs from it are still in use then the class and associated methods can be GC'd rendering the jmethodID a dangling pointer. But it is not up to the JVM TI implementation to try and ensure this, it is the user of JVM TI. So I'm not sure what might be considered a "fix" for this issue?
08-06-2021

A JVMTI agent should not rely on JVMTI_ERROR_INVALID_METHODID when saving aside a jmethodID (e.g., from the results of GetStackTrace). Instead, it should ensure that the classloader associated with this jmethodID is not GCed. This can be done by creating a GlobalRef on the classloader: - GetMethodDeclaringClass -> GetClassLoader -> NewGlobalRef The agent can release the GlobalRef when it no longer references the jmethodID.
08-06-2021