8298084: Memory leak in Method::build_profiling_method_data

Co-authored-by: Justin King <jcking@openjdk.org>
Reviewed-by: kbarrett, eosterlund, dholmes, jcking, thartmann

src/hotspot/share/memory/metadataFactory.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,6 +30,7 @@
 #include "oops/array.inline.hpp"
 #include "utilities/exceptions.hpp"
 #include "utilities/globalDefinitions.hpp"
+#include <type_traits>
 
 class MetadataFactory : AllStatic {
  public:
@@ -61,14 +62,21 @@ class MetadataFactory : AllStatic {
 
   // Deallocation method for metadata
   template <class T>
-  static void free_metadata(ClassLoaderData* loader_data, T md) {
+  static void free_metadata(ClassLoaderData* loader_data, T* md) {
     if (md != NULL) {
       assert(loader_data != NULL, "shouldn't pass null");
       int size = md->size();
-      // Call metadata's deallocate function which will call deallocate fields
+      // Call metadata's deallocate function which will deallocate fields and release_C_heap_structures
       assert(!md->on_stack(), "can't deallocate things on stack");
       assert(!md->is_shared(), "cannot deallocate if in shared spaces");
       md->deallocate_contents(loader_data);
+      // Call the destructor. This is currently used for MethodData which has a member
+      // that needs to be destructed to release resources. Most Metadata derived classes have noop
+      // destructors and/or cleanup using deallocate_contents.
+      // T is a potentially const or volatile qualified pointer. Remove any const
+      // or volatile so we can call the destructor of the type T points to.
+      using U = std::remove_cv_t<T>;
+      md->~U();
       loader_data->metaspace_non_null()->deallocate((MetaWord*)md, size, md->is_klass());
     }
   }

src/hotspot/share/oops/instanceKlass.cpp

@@ -595,10 +595,10 @@ void InstanceKlass::deallocate_contents(ClassLoaderData* loader_data) {
 
   // Release C heap allocated data that this points to, which includes
   // reference counting symbol names.
-  // Can't release the constant pool here because the constant pool can be
-  // deallocated separately from the InstanceKlass for default methods and
-  // redefine classes.
-  release_C_heap_structures(/* release_constant_pool */ false);
+  // Can't release the constant pool or MethodData C heap data here because the constant
+  // pool can be deallocated separately from the InstanceKlass for default methods and
+  // redefine classes.  MethodData can also be released separately.
+  release_C_heap_structures(/* release_sub_metadata */ false);
 
   deallocate_methods(loader_data, methods());
   set_methods(NULL);
@@ -2650,13 +2650,15 @@ static void method_release_C_heap_structures(Method* m) {
   m->release_C_heap_structures();
 }
 
-// Called also by InstanceKlass::deallocate_contents, with false for release_constant_pool.
-void InstanceKlass::release_C_heap_structures(bool release_constant_pool) {
+// Called also by InstanceKlass::deallocate_contents, with false for release_sub_metadata.
+void InstanceKlass::release_C_heap_structures(bool release_sub_metadata) {
   // Clean up C heap
   Klass::release_C_heap_structures();
 
   // Deallocate and call destructors for MDO mutexes
-  methods_do(method_release_C_heap_structures);
+  if (release_sub_metadata) {
+    methods_do(method_release_C_heap_structures);
+  }
 
   // Destroy the init_monitor
   delete _init_monitor;
@@ -2696,7 +2698,7 @@ void InstanceKlass::release_C_heap_structures(bool release_constant_pool) {
 
   FREE_C_HEAP_ARRAY(char, _source_debug_extension);
 
-  if (release_constant_pool) {
+  if (release_sub_metadata) {
     constants()->release_C_heap_structures();
   }
 }

src/hotspot/share/oops/instanceKlass.hpp

@@ -1012,7 +1012,7 @@ class InstanceKlass: public Klass {
   // callbacks for actions during class unloading
   static void unload_class(InstanceKlass* ik);
 
-  virtual void release_C_heap_structures(bool release_constant_pool = true);
+  virtual void release_C_heap_structures(bool release_sub_metadata = true);
 
   // Naming
   const char* signature_name() const;

src/hotspot/share/oops/method.cpp

@@ -141,10 +141,9 @@ void Method::deallocate_contents(ClassLoaderData* loader_data) {
 
 void Method::release_C_heap_structures() {
   if (method_data()) {
-#if INCLUDE_JVMCI
-    FailedSpeculation::free_failed_speculations(method_data()->get_failed_speculations_address());
-#endif
-    // Destroy MethodData
+    method_data()->release_C_heap_structures();
+
+    // Destroy MethodData embedded lock
     method_data()->~MethodData();
   }
 }

src/hotspot/share/oops/methodData.cpp

@@ -1821,3 +1821,13 @@ void MethodData::clean_weak_method_links() {
   clean_extra_data(&cl);
   verify_extra_data_clean(&cl);
 }
+
+void MethodData::deallocate_contents(ClassLoaderData* loader_data) {
+  release_C_heap_structures();
+}
+
+void MethodData::release_C_heap_structures() {
+#if INCLUDE_JVMCI
+  FailedSpeculation::free_failed_speculations(get_failed_speculations_address());
+#endif
+}

src/hotspot/share/oops/methodData.hpp

@@ -2449,8 +2449,9 @@ class MethodData : public Metadata {
   virtual void metaspace_pointers_do(MetaspaceClosure* iter);
   virtual MetaspaceObj::Type type() const { return MethodDataType; }
 
-  // Deallocation support - no metaspace pointer fields to deallocate
-  void deallocate_contents(ClassLoaderData* loader_data) {}
+  // Deallocation support
+  void deallocate_contents(ClassLoaderData* loader_data);
+  void release_C_heap_structures();
 
   // GC support
   void set_size(int object_size_in_bytes) { _size = object_size_in_bytes; }