From df4a25b41c7f339cd077e072aa0fd3604ed809f5 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Bempel Date: Thu, 21 Sep 2023 05:16:07 +0000 Subject: [PATCH] 8308762: Metaspace leak with Instrumentation.retransform Reviewed-by: dholmes, coleenp --- src/hotspot/share/oops/constantPool.cpp | 19 +++--- .../share/prims/jvmtiRedefineClasses.cpp | 44 ------------ .../share/prims/jvmtiRedefineClasses.hpp | 2 - test/hotspot/jtreg/TEST.groups | 1 + .../RedefineLeakThrowable.java | 67 +++++++++++++++++++ 5 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java diff --git a/src/hotspot/share/oops/constantPool.cpp b/src/hotspot/share/oops/constantPool.cpp index 97b863a42dc..94908fb36b2 100644 --- a/src/hotspot/share/oops/constantPool.cpp +++ b/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); diff --git a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp index fad6b2901f0..c03c4b48538 100644 --- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp +++ b/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); diff --git a/src/hotspot/share/prims/jvmtiRedefineClasses.hpp b/src/hotspot/share/prims/jvmtiRedefineClasses.hpp index 2c8c21ced26..5b8ad06d2d6 100644 --- a/src/hotspot/share/prims/jvmtiRedefineClasses.hpp +++ b/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, diff --git a/test/hotspot/jtreg/TEST.groups b/test/hotspot/jtreg/TEST.groups index be6d9fdeddf..6ee8e99a188 100644 --- a/test/hotspot/jtreg/TEST.groups +++ b/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 \ diff --git a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java new file mode 100644 index 00000000000..47e1cd736eb --- /dev/null +++ b/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); + } + } +}