8347564: ZGC: Crash in DependencyContext::clean_unloading_dependents

xmas92 · TheRealMDoerr · commit 352a7a97823b · 2025-02-18T14:52:18.000Z
Backport-of: 14136f8b1106137317393bc2ab0a2db0d212f8d8
diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp
@@ -4674,28 +4674,31 @@ int java_lang_invoke_MethodType::rtype_slot_count(oop mt) {
 // Support for java_lang_invoke_CallSite
 
 int java_lang_invoke_CallSite::_target_offset;
-int java_lang_invoke_CallSite::_context_offset;
+int java_lang_invoke_CallSite::_vmdependencies_offset;
+int java_lang_invoke_CallSite::_last_cleanup_offset;
 
 #define CALLSITE_FIELDS_DO(macro) \
   macro(_target_offset,  k, "target", java_lang_invoke_MethodHandle_signature, false); \
-  macro(_context_offset, k, "context", java_lang_invoke_MethodHandleNatives_CallSiteContext_signature, false)
 
 void java_lang_invoke_CallSite::compute_offsets() {
   InstanceKlass* k = vmClasses::CallSite_klass();
   CALLSITE_FIELDS_DO(FIELD_COMPUTE_OFFSET);
+  CALLSITE_INJECTED_FIELDS(INJECTED_FIELD_COMPUTE_OFFSET);
 }
 
 #if INCLUDE_CDS
 void java_lang_invoke_CallSite::serialize_offsets(SerializeClosure* f) {
   CALLSITE_FIELDS_DO(FIELD_SERIALIZE_OFFSET);
+  CALLSITE_INJECTED_FIELDS(INJECTED_FIELD_SERIALIZE_OFFSET);
 }
 #endif
 
-oop java_lang_invoke_CallSite::context_no_keepalive(oop call_site) {
+DependencyContext java_lang_invoke_CallSite::vmdependencies(oop call_site) {
   assert(java_lang_invoke_CallSite::is_instance(call_site), "");
-
-  oop dep_oop = call_site->obj_field_access<AS_NO_KEEPALIVE>(_context_offset);
-  return dep_oop;
+  nmethodBucket* volatile* vmdeps_addr = call_site->field_addr<nmethodBucket* volatile>(_vmdependencies_offset);
+  volatile uint64_t* last_cleanup_addr = call_site->field_addr<volatile uint64_t>(_last_cleanup_offset);
+  DependencyContext dep_ctx(vmdeps_addr, last_cleanup_addr);
+  return dep_ctx;
 }
 
 // Support for java_lang_invoke_ConstantCallSite
@@ -4716,30 +4719,6 @@ void java_lang_invoke_ConstantCallSite::serialize_offsets(SerializeClosure* f) {
 }
 #endif
 
-// Support for java_lang_invoke_MethodHandleNatives_CallSiteContext
-
-int java_lang_invoke_MethodHandleNatives_CallSiteContext::_vmdependencies_offset;
-int java_lang_invoke_MethodHandleNatives_CallSiteContext::_last_cleanup_offset;
-
-void java_lang_invoke_MethodHandleNatives_CallSiteContext::compute_offsets() {
-  InstanceKlass* k = vmClasses::Context_klass();
-  CALLSITECONTEXT_INJECTED_FIELDS(INJECTED_FIELD_COMPUTE_OFFSET);
-}
-
-#if INCLUDE_CDS
-void java_lang_invoke_MethodHandleNatives_CallSiteContext::serialize_offsets(SerializeClosure* f) {
-  CALLSITECONTEXT_INJECTED_FIELDS(INJECTED_FIELD_SERIALIZE_OFFSET);
-}
-#endif
-
-DependencyContext java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(oop call_site) {
-  assert(java_lang_invoke_MethodHandleNatives_CallSiteContext::is_instance(call_site), "");
-  nmethodBucket* volatile* vmdeps_addr = call_site->field_addr<nmethodBucket* volatile>(_vmdependencies_offset);
-  volatile uint64_t* last_cleanup_addr = call_site->field_addr<volatile uint64_t>(_last_cleanup_offset);
-  DependencyContext dep_ctx(vmdeps_addr, last_cleanup_addr);
-  return dep_ctx;
-}
-
 // Support for java_lang_ClassLoader
 
 int  java_lang_ClassLoader::_loader_data_offset;
@@ -5389,7 +5368,6 @@ void java_lang_InternalError::serialize_offsets(SerializeClosure* f) {
   f(java_lang_invoke_MethodType) \
   f(java_lang_invoke_CallSite) \
   f(java_lang_invoke_ConstantCallSite) \
-  f(java_lang_invoke_MethodHandleNatives_CallSiteContext) \
   f(java_lang_reflect_AccessibleObject) \
   f(java_lang_reflect_Method) \
   f(java_lang_reflect_Constructor) \
@@ -5455,8 +5433,7 @@ bool JavaClasses::is_supported_for_archiving(oop obj) {
   if (!CDSConfig::is_dumping_invokedynamic()) {
     // These are supported by CDS only when CDSConfig::is_dumping_invokedynamic() is enabled.
     if (klass == vmClasses::ResolvedMethodName_klass() ||
-        klass == vmClasses::MemberName_klass() ||
-        klass == vmClasses::Context_klass()) {
+        klass == vmClasses::MemberName_klass()) {
       return false;
     }
   }
diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp
@@ -1409,13 +1409,17 @@ class java_lang_invoke_MethodType: AllStatic {
 
 
 // Interface to java.lang.invoke.CallSite objects
+#define CALLSITE_INJECTED_FIELDS(macro) \
+  macro(java_lang_invoke_CallSite, vmdependencies, intptr_signature, false) \
+  macro(java_lang_invoke_CallSite, last_cleanup, long_signature, false)
 
 class java_lang_invoke_CallSite: AllStatic {
   friend class JavaClasses;
 
 private:
   static int _target_offset;
-  static int _context_offset;
+  static int _vmdependencies_offset;
+  static int _last_cleanup_offset;
 
   static void compute_offsets();
 
@@ -1426,7 +1430,7 @@ class java_lang_invoke_CallSite: AllStatic {
   static void         set_target(          oop site, oop target);
   static void         set_target_volatile( oop site, oop target);
 
-  static oop context_no_keepalive(oop site);
+  static DependencyContext vmdependencies(oop call_site);
 
   // Testers
   static bool is_subclass(Klass* klass) {
@@ -1436,7 +1440,6 @@ class java_lang_invoke_CallSite: AllStatic {
 
   // Accessors for code generation:
   static int target_offset()  { CHECK_INIT(_target_offset); }
-  static int context_offset() { CHECK_INIT(_context_offset); }
 };
 
 // Interface to java.lang.invoke.ConstantCallSite objects
@@ -1461,35 +1464,6 @@ class java_lang_invoke_ConstantCallSite: AllStatic {
   static bool is_instance(oop obj);
 };
 
-// Interface to java.lang.invoke.MethodHandleNatives$CallSiteContext objects
-
-#define CALLSITECONTEXT_INJECTED_FIELDS(macro) \
-  macro(java_lang_invoke_MethodHandleNatives_CallSiteContext, vmdependencies, intptr_signature, false) \
-  macro(java_lang_invoke_MethodHandleNatives_CallSiteContext, last_cleanup, long_signature, false)
-
-class DependencyContext;
-
-class java_lang_invoke_MethodHandleNatives_CallSiteContext : AllStatic {
-  friend class JavaClasses;
-
-private:
-  static int _vmdependencies_offset;
-  static int _last_cleanup_offset;
-
-  static void compute_offsets();
-
-public:
-  static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;
-  // Accessors
-  static DependencyContext vmdependencies(oop context);
-
-  // Testers
-  static bool is_subclass(Klass* klass) {
-    return klass->is_subclass_of(vmClasses::Context_klass());
-  }
-  static bool is_instance(oop obj);
-};
-
 // Interface to java.lang.ClassLoader objects
 
 #define CLASSLOADER_INJECTED_FIELDS(macro)                            \
diff --git a/src/hotspot/share/classfile/javaClasses.inline.hpp b/src/hotspot/share/classfile/javaClasses.inline.hpp
@@ -263,10 +263,6 @@ inline bool java_lang_invoke_ConstantCallSite::is_instance(oop obj) {
   return obj != nullptr && is_subclass(obj->klass());
 }
 
-inline bool java_lang_invoke_MethodHandleNatives_CallSiteContext::is_instance(oop obj) {
-  return obj != nullptr && is_subclass(obj->klass());
-}
-
 inline bool java_lang_invoke_MemberName::is_instance(oop obj) {
   return obj != nullptr && obj->klass() == vmClasses::MemberName_klass();
 }
diff --git a/src/hotspot/share/classfile/javaClassesImpl.hpp b/src/hotspot/share/classfile/javaClassesImpl.hpp
@@ -36,7 +36,7 @@
   CLASSLOADER_INJECTED_FIELDS(macro)        \
   RESOLVEDMETHOD_INJECTED_FIELDS(macro)     \
   MEMBERNAME_INJECTED_FIELDS(macro)         \
-  CALLSITECONTEXT_INJECTED_FIELDS(macro)    \
+  CALLSITE_INJECTED_FIELDS(macro)           \
   STACKFRAMEINFO_INJECTED_FIELDS(macro)     \
   MODULE_INJECTED_FIELDS(macro)             \
   THREAD_INJECTED_FIELDS(macro)             \
diff --git a/src/hotspot/share/classfile/vmClassMacros.hpp b/src/hotspot/share/classfile/vmClassMacros.hpp
@@ -123,7 +123,6 @@
   do_klass(ABIDescriptor_klass,                         jdk_internal_foreign_abi_ABIDescriptor                ) \
   do_klass(VMStorage_klass,                             jdk_internal_foreign_abi_VMStorage                    ) \
   do_klass(CallConv_klass,                              jdk_internal_foreign_abi_CallConv                     ) \
-  do_klass(Context_klass,                               java_lang_invoke_MethodHandleNatives_CallSiteContext  ) \
   do_klass(ConstantCallSite_klass,                      java_lang_invoke_ConstantCallSite                     ) \
   do_klass(MutableCallSite_klass,                       java_lang_invoke_MutableCallSite                      ) \
   do_klass(VolatileCallSite_klass,                      java_lang_invoke_VolatileCallSite                     ) \
diff --git a/src/hotspot/share/classfile/vmSymbols.hpp b/src/hotspot/share/classfile/vmSymbols.hpp
@@ -326,11 +326,9 @@ class SerializeClosure;
   template(java_lang_invoke_MemberName,               "java/lang/invoke/MemberName")              \
   template(java_lang_invoke_ResolvedMethodName,       "java/lang/invoke/ResolvedMethodName")      \
   template(java_lang_invoke_MethodHandleNatives,      "java/lang/invoke/MethodHandleNatives")     \
-  template(java_lang_invoke_MethodHandleNatives_CallSiteContext, "java/lang/invoke/MethodHandleNatives$CallSiteContext") \
   template(java_lang_invoke_LambdaForm,               "java/lang/invoke/LambdaForm")              \
   template(java_lang_invoke_InjectedProfile_signature, "Ljava/lang/invoke/InjectedProfile;")      \
   template(java_lang_invoke_LambdaForm_Compiled_signature, "Ljava/lang/invoke/LambdaForm$Compiled;") \
-  template(java_lang_invoke_MethodHandleNatives_CallSiteContext_signature, "Ljava/lang/invoke/MethodHandleNatives$CallSiteContext;") \
   /* internal up-calls made only by the JVM, via class sun.invoke.MethodHandleNatives: */         \
   template(findMethodHandleType_name,                 "findMethodHandleType")                     \
   template(findMethodHandleType_signature,       "(Ljava/lang/Class;[Ljava/lang/Class;)Ljava/lang/invoke/MethodType;") \
diff --git a/src/hotspot/share/code/dependencyContext.cpp b/src/hotspot/share/code/dependencyContext.cpp
@@ -168,12 +168,6 @@ void DependencyContext::clean_unloading_dependents() {
   }
 }
 
-nmethodBucket* DependencyContext::release_and_get_next_not_unloading(nmethodBucket* b) {
-  nmethodBucket* next = b->next_not_unloading();
-  release(b);
-  return next;
- }
-
 //
 // Invalidate all dependencies in the context
 void DependencyContext::remove_all_dependents() {
@@ -214,18 +208,6 @@ void DependencyContext::remove_all_dependents() {
   set_dependencies(nullptr);
 }
 
-void DependencyContext::remove_and_mark_for_deoptimization_all_dependents(DeoptimizationScope* deopt_scope) {
-  nmethodBucket* b = dependencies_not_unloading();
-  set_dependencies(nullptr);
-  while (b != nullptr) {
-    nmethod* nm = b->get_nmethod();
-    // Also count already (concurrently) marked nmethods to make sure
-    // deoptimization is triggered before execution in this thread continues.
-    deopt_scope->mark(nm);
-    b = release_and_get_next_not_unloading(b);
-  }
-}
-
 #ifndef PRODUCT
 bool DependencyContext::is_empty() {
   return dependencies() == nullptr;
diff --git a/src/hotspot/share/code/dependencyContext.hpp b/src/hotspot/share/code/dependencyContext.hpp
@@ -63,7 +63,7 @@ class nmethodBucket: public CHeapObj<mtClass> {
 //
 // Utility class to manipulate nmethod dependency context.
 // Dependency context can be attached either to an InstanceKlass (_dep_context field)
-// or CallSiteContext oop for call_site_target dependencies (see javaClasses.hpp).
+// or CallSite oop for call_site_target dependencies (see javaClasses.hpp).
 // DependencyContext class operates on some location which holds a nmethodBucket* value
 // and uint64_t integer recording the safepoint counter at the last cleanup.
 //
@@ -92,7 +92,6 @@ class DependencyContext : public StackObj {
 #ifdef ASSERT
   // Safepoints are forbidden during DC lifetime. GC can invalidate
   // _dependency_context_addr if it relocates the holder
-  // (e.g. CallSiteContext Java object).
   SafepointStateTracker _safepoint_tracker;
 
   DependencyContext(nmethodBucket* volatile* bucket_addr, volatile uint64_t* last_cleanup_addr)
@@ -114,9 +113,7 @@ class DependencyContext : public StackObj {
   void mark_dependent_nmethods(DeoptimizationScope* deopt_scope, DepChange& changes);
   void add_dependent_nmethod(nmethod* nm);
   void remove_all_dependents();
-  void remove_and_mark_for_deoptimization_all_dependents(DeoptimizationScope* deopt_scope);
   void clean_unloading_dependents();
-  static nmethodBucket* release_and_get_next_not_unloading(nmethodBucket* b);
   static void purge_dependency_contexts();
   static void release(nmethodBucket* b);
   static void cleaning_start();
diff --git a/src/hotspot/share/prims/methodHandles.cpp b/src/hotspot/share/prims/methodHandles.cpp
@@ -932,19 +932,12 @@ void MethodHandles::expand_MemberName(Handle mname, int suppress, TRAPS) {
 void MethodHandles::add_dependent_nmethod(oop call_site, nmethod* nm) {
   assert_locked_or_safepoint(CodeCache_lock);
 
-  oop context = java_lang_invoke_CallSite::context_no_keepalive(call_site);
-  DependencyContext deps = java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(context);
-  // Try to purge stale entries on updates.
-  // Since GC doesn't clean dependency contexts rooted at CallSiteContext objects,
-  // in order to avoid memory leak, stale entries are purged whenever a dependency list
-  // is changed (both on addition and removal). Though memory reclamation is delayed,
-  // it avoids indefinite memory usage growth.
+  DependencyContext deps = java_lang_invoke_CallSite::vmdependencies(call_site);
   deps.add_dependent_nmethod(nm);
 }
 
 void MethodHandles::clean_dependency_context(oop call_site) {
-  oop context = java_lang_invoke_CallSite::context_no_keepalive(call_site);
-  DependencyContext deps = java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(context);
+  DependencyContext deps = java_lang_invoke_CallSite::vmdependencies(call_site);
   deps.clean_unloading_dependents();
 }
 
@@ -956,8 +949,7 @@ void MethodHandles::mark_dependent_nmethods(DeoptimizationScope* deopt_scope, Ha
     NoSafepointVerifier nsv;
     MutexLocker ml(CodeCache_lock, Mutex::_no_safepoint_check_flag);
 
-    oop context = java_lang_invoke_CallSite::context_no_keepalive(call_site());
-    DependencyContext deps = java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(context);
+    DependencyContext deps = java_lang_invoke_CallSite::vmdependencies(call_site());
     deps.mark_dependent_nmethods(deopt_scope, changes);
   }
 }
@@ -1322,23 +1314,6 @@ JVM_ENTRY(void, MHN_copyOutBootstrapArguments(JNIEnv* env, jobject igcls,
 }
 JVM_END
 
-// It is called by a Cleaner object which ensures that dropped CallSites properly
-// deallocate their dependency information.
-JVM_ENTRY(void, MHN_clearCallSiteContext(JNIEnv* env, jobject igcls, jobject context_jh)) {
-  Handle context(THREAD, JNIHandles::resolve_non_null(context_jh));
-  DeoptimizationScope deopt_scope;
-  {
-    NoSafepointVerifier nsv;
-    MutexLocker ml(THREAD, CodeCache_lock, Mutex::_no_safepoint_check_flag);
-    DependencyContext deps = java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(context());
-    deps.remove_and_mark_for_deoptimization_all_dependents(&deopt_scope);
-    // This is assumed to be an 'atomic' operation by verification.
-    // So keep it under lock for now.
-    deopt_scope.deoptimize_marked();
-  }
-}
-JVM_END
-
 /**
  * Throws a java/lang/UnsupportedOperationException unconditionally.
  * This is required by the specification of MethodHandle.invoke if
@@ -1385,7 +1360,6 @@ JVM_END
 #define MT    JLINV "MethodType;"
 #define MH    JLINV "MethodHandle;"
 #define MEM   JLINV "MemberName;"
-#define CTX   JLINV "MethodHandleNatives$CallSiteContext;"
 
 #define CC (char*)  /*cast a literal from (const char*)*/
 #define FN_PTR(f) CAST_FROM_FN_PTR(void*, &f)
@@ -1401,7 +1375,6 @@ static JNINativeMethod MHN_methods[] = {
   {CC "setCallSiteTargetNormal",   CC "(" CS "" MH ")V",                     FN_PTR(MHN_setCallSiteTargetNormal)},
   {CC "setCallSiteTargetVolatile", CC "(" CS "" MH ")V",                     FN_PTR(MHN_setCallSiteTargetVolatile)},
   {CC "copyOutBootstrapArguments", CC "(" CLS "[III[" OBJ "IZ" OBJ ")V",     FN_PTR(MHN_copyOutBootstrapArguments)},
-  {CC "clearCallSiteContext",      CC "(" CTX ")V",                          FN_PTR(MHN_clearCallSiteContext)},
   {CC "staticFieldOffset",         CC "(" MEM ")J",                          FN_PTR(MHN_staticFieldOffset)},
   {CC "staticFieldBase",           CC "(" MEM ")" OBJ,                        FN_PTR(MHN_staticFieldBase)},
   {CC "getMemberVMInfo",           CC "(" MEM ")" OBJ,                       FN_PTR(MHN_getMemberVMInfo)}
diff --git a/src/java.base/share/classes/java/lang/invoke/CallSite.java b/src/java.base/share/classes/java/lang/invoke/CallSite.java
@@ -138,12 +138,6 @@ abstract sealed class CallSite permits ConstantCallSite, MutableCallSite, Volati
         UNSAFE.storeStoreFence(); // barrier between target and isFrozen updates
     }
 
-    /**
-     * {@code CallSite} dependency context.
-     * JVM uses CallSite.context to store nmethod dependencies on the call site target.
-     */
-    private final MethodHandleNatives.CallSiteContext context = MethodHandleNatives.CallSiteContext.make(this);
-
     /**
      * Returns the type of this call site's target.
      * Although targets may change, any call site's type is permanent, and can never change to an unequal type.
diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java
@@ -71,31 +71,6 @@ static native void copyOutBootstrapArguments(Class<?> caller, int[] indexInfo,
                                                  boolean resolve,
                                                  Object ifNotAvailable);
 
-    /** Represents a context to track nmethod dependencies on CallSite instance target. */
-    static class CallSiteContext implements Runnable {
-        //@Injected JVM_nmethodBucket* vmdependencies;
-        //@Injected jlong last_cleanup;
-
-        static CallSiteContext make(CallSite cs) {
-            final CallSiteContext newContext = new CallSiteContext();
-            // CallSite instance is tracked by a Cleanable which clears native
-            // structures allocated for CallSite context. Though the CallSite can
-            // become unreachable, its Context is retained by the Cleanable instance
-            // (which is referenced from Cleaner instance which is referenced from
-            // CleanerFactory class) until cleanup is performed.
-            CleanerFactory.cleaner().register(cs, newContext);
-            return newContext;
-        }
-
-        @Override
-        public void run() {
-            MethodHandleNatives.clearCallSiteContext(this);
-        }
-    }
-
-    /** Invalidate all recorded nmethods. */
-    private static native void clearCallSiteContext(CallSiteContext context);
-
     private static native void registerNatives();
     static {
         registerNatives();
diff --git a/test/hotspot/jtreg/compiler/jsr292/CallSiteDepContextTest.java b/test/hotspot/jtreg/compiler/jsr292/CallSiteDepContextTest.java
@@ -123,8 +123,8 @@ private static void execute(int expected, MethodHandle... mhs) throws Throwable
 
     public static void testHiddenDepField() {
         try {
-            Field f = MethodHandleHelper.MHN_CALL_SITE_CONTEXT_CLASS.getDeclaredField("vmdependencies");
-            throw new AssertionError("Context.dependencies field should be hidden");
+            Field f = MethodHandleHelper.JLI_CALL_SITE_CLASS.getDeclaredField("vmdependencies");
+            throw new AssertionError("CallSite.dependencies field should be hidden");
         } catch(NoSuchFieldException e) { /* expected */ }
     }
 
diff --git a/test/hotspot/jtreg/compiler/jsr292/patches/java.base/java/lang/invoke/MethodHandleHelper.java b/test/hotspot/jtreg/compiler/jsr292/patches/java.base/java/lang/invoke/MethodHandleHelper.java
@@ -36,8 +36,8 @@ public class MethodHandleHelper {
     private MethodHandleHelper() { }
 
     public static final Lookup IMPL_LOOKUP = Lookup.IMPL_LOOKUP;
-    public static final Class<?> MHN_CALL_SITE_CONTEXT_CLASS
-            = MethodHandleNatives.CallSiteContext.class;
+    public static final Class<?> JLI_CALL_SITE_CLASS
+            = java.lang.invoke.CallSite.class;
 
     public static void customize(MethodHandle mh) {
         mh.customize();

Original file line number	Diff line number	Diff line change
`@@ -263,10 +263,6 @@ inline bool java_lang_invoke_ConstantCallSite::is_instance(oop obj) {`
`263`	`263`	`return obj != nullptr && is_subclass(obj->klass());`
`264`	`264`	`}`
`265`	`265`
`266`		`-inline bool java_lang_invoke_MethodHandleNatives_CallSiteContext::is_instance(oop obj) {`
`267`		`- return obj != nullptr && is_subclass(obj->klass());`
`268`		`-}`
`269`		`-`
`270`	`266`	`inline bool java_lang_invoke_MemberName::is_instance(oop obj) {`
`271`	`267`	`return obj != nullptr && obj->klass() == vmClasses::MemberName_klass();`
`272`	`268`	`}`