8308762: Metaspace leak with Instrumentation.retransform

Reviewed-by: dholmes, coleenp

src/hotspot/share/oops/constantPool.cpp

@@ -1294,6 +1294,16 @@ bool ConstantPool::compare_entry_to(int index1, const constantPoolHandle& cp2,
   jbyte t1 = tag_at(index1).non_error_value();
   jbyte t2 = cp2->tag_at(index2).non_error_value();
 
+  // Some classes are pre-resolved (like Throwable) which may lead to
+  // consider it as a different entry. We then revert them back temporarily
+  // to ensure proper comparison.
+  if (t1 == JVM_CONSTANT_Class) {
+    t1 = JVM_CONSTANT_UnresolvedClass;
+  }
+  if (t2 == JVM_CONSTANT_Class) {
+    t2 = JVM_CONSTANT_UnresolvedClass;
+  }
+
   if (t1 != t2) {
     // Not the same entry type so there is nothing else to check. Note
     // that this style of checking will consider resolved/unresolved
@@ -1305,15 +1315,6 @@ bool ConstantPool::compare_entry_to(int index1, const constantPoolHandle& cp2,
   }
 
   switch (t1) {
-  case JVM_CONSTANT_Class:
-  {
-    Klass* k1 = resolved_klass_at(index1);
-    Klass* k2 = cp2->resolved_klass_at(index2);
-    if (k1 == k2) {
-      return true;
-    }
-  } break;
-
   case JVM_CONSTANT_ClassIndex:
   {
     int recur1 = klass_index_at(index1);

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -1287,35 +1287,6 @@ int VM_RedefineClasses::find_new_operand_index(int old_index) {
 } // end find_new_operand_index()
 
 
-// Returns true if the current mismatch is due to a resolved/unresolved
-// class pair. Otherwise, returns false.
-bool VM_RedefineClasses::is_unresolved_class_mismatch(const constantPoolHandle& cp1,
-       int index1, const constantPoolHandle& cp2, int index2) {
-
-  jbyte t1 = cp1->tag_at(index1).value();
-  if (t1 != JVM_CONSTANT_Class && t1 != JVM_CONSTANT_UnresolvedClass) {
-    return false;  // wrong entry type; not our special case
-  }
-
-  jbyte t2 = cp2->tag_at(index2).value();
-  if (t2 != JVM_CONSTANT_Class && t2 != JVM_CONSTANT_UnresolvedClass) {
-    return false;  // wrong entry type; not our special case
-  }
-
-  if (t1 == t2) {
-    return false;  // not a mismatch; not our special case
-  }
-
-  char *s1 = cp1->klass_name_at(index1)->as_C_string();
-  char *s2 = cp2->klass_name_at(index2)->as_C_string();
-  if (strcmp(s1, s2) != 0) {
-    return false;  // strings don't match; not our special case
-  }
-
-  return true;  // made it through the gauntlet; this is our special case
-} // end is_unresolved_class_mismatch()
-
-
 // The bug 6214132 caused the verification to fail.
 // 1. What's done in RedefineClasses() before verification:
 //  a) A reference to the class being redefined (_the_class) and a
@@ -1705,14 +1676,6 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
       if (match) {
         // found a match at the same index so nothing more to do
         continue;
-      } else if (is_unresolved_class_mismatch(scratch_cp, scratch_i,
-                                              *merge_cp_p, scratch_i)) {
-        // The mismatch in compare_entry_to() above is because of a
-        // resolved versus unresolved class entry at the same index
-        // with the same string value. Since Pass 0 reverted any
-        // class entries to unresolved class entries in *merge_cp_p,
-        // we go with the unresolved class entry.
-        continue;
       }
 
       int found_i = scratch_cp->find_matching_entry(scratch_i, *merge_cp_p);
@@ -1726,13 +1689,6 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
         continue;
       }
 
-      // The find_matching_entry() call above could fail to find a match
-      // due to a resolved versus unresolved class or string entry situation
-      // like we solved above with the is_unresolved_*_mismatch() calls.
-      // However, we would have to call is_unresolved_*_mismatch() over
-      // all of *merge_cp_p (potentially) and that doesn't seem to be
-      // worth the time.
-
       // No match found so we have to append this entry and any unique
       // referenced entries to *merge_cp_p.
       append_entry(scratch_cp, scratch_i, merge_cp_p, merge_cp_length_p);

src/hotspot/share/prims/jvmtiRedefineClasses.hpp

@@ -438,8 +438,6 @@ class VM_RedefineClasses: public VM_Operation {
     constantPoolHandle *merge_cp_p, int *merge_cp_length_p);
   u2 find_new_index(int old_index);
   int find_new_operand_index(int old_bootstrap_spec_index);
-  bool is_unresolved_class_mismatch(const constantPoolHandle& cp1, int index1,
-    const constantPoolHandle& cp2, int index2);
   void map_index(const constantPoolHandle& scratch_cp, int old_index, int new_index);
   void map_operand_index(int old_bootstrap_spec_index, int new_bootstrap_spec_index);
   bool merge_constant_pools(const constantPoolHandle& old_cp,

test/hotspot/jtreg/TEST.groups

@@ -537,6 +537,7 @@ tier1_serviceability = \
   serviceability/ \
   -serviceability/dcmd/compiler/CompilerQueueTest.java \
   -serviceability/jvmti/RedefineClasses/RedefineLeak.java \
+  -serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java \
   -serviceability/jvmti/RedefineClasses/RedefinePreviousVersions.java \
   -serviceability/jvmti/RedefineClasses/RedefineRunningMethods.java \
   -serviceability/jvmti/RedefineClasses/RedefineRunningMethodsWithBacktrace.java \

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2023, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ # @bug 8308762
+ * @library /test/lib
+ * @summary Test that redefinition of class containing Throwable refs does not leak constant pool
+ * @requires vm.jvmti
+ * @requires vm.flagless
+ * @modules java.base/jdk.internal.misc
+ * @modules java.instrument
+ *          java.compiler
+ * @run main RedefineClassHelper
+ * @run main/othervm/timeout=6000 -javaagent:redefineagent.jar -XX:MetaspaceSize=12m  -XX:MaxMetaspaceSize=12m RedefineLeakThrowable
+ */
+
+class Tester {
+    void test() {
+        try {
+            int i = 42;
+        } catch (Throwable t) {
+            t.printStackTrace();
+        }
+    }
+}
+
+public class RedefineLeakThrowable {
+
+    static final String NEW_TESTER =
+        "class Tester {" +
+        "   void test() {" +
+        "        try {" +
+        "            int i = 42;" +
+        "        } catch (Throwable t) {" +
+        "            t.printStackTrace();" +
+        "        }" +
+        "    }" +
+        "}";
+
+
+    public static void main(String argv[]) throws Exception {
+        for (int i = 0; i < 500; i++) {
+            RedefineClassHelper.redefineClass(Tester.class, NEW_TESTER);
+        }
+    }
+}