8218755: Refix Symbol leak in prepend_host_package_name
Summary: fix Symbol refcounting again, add comment and a test.
Reviewed-by: kbarrett, dholmes
--- a/src/hotspot/share/classfile/classFileParser.cpp Wed Feb 13 06:48:34 2019 -0500
+++ b/src/hotspot/share/classfile/classFileParser.cpp Wed Feb 13 07:22:09 2019 -0500
@@ -5720,9 +5720,8 @@
void ClassFileParser::update_class_name(Symbol* new_class_name) {
// Decrement the refcount in the old name, since we're clobbering it.
- if (_class_name != NULL) {
- _class_name->decrement_refcount();
- }
+ _class_name->decrement_refcount();
+
_class_name = new_class_name;
// Increment the refcount of the new name.
// Now the ClassFileParser owns this name and will decrement in
@@ -5753,11 +5752,16 @@
// characters. So, do a strncpy instead of using sprintf("%s...").
strncpy(new_anon_name + host_pkg_len + 1, (char *)_class_name->base(), class_name_len);
+ // Decrement old _class_name to avoid leaking.
+ _class_name->decrement_refcount();
+
// Create a symbol and update the anonymous class name.
- Symbol* new_name = SymbolTable::new_symbol(new_anon_name,
+ // The new class name is created with a refcount of one. When installed into the InstanceKlass,
+ // it'll be two and when the ClassFileParser destructor runs, it'll go back to one and get deleted
+ // when the class is unloaded.
+ _class_name = SymbolTable::new_symbol(new_anon_name,
(int)host_pkg_len + 1 + class_name_len,
CHECK);
- update_class_name(new_name);
}
}
@@ -5861,7 +5865,8 @@
_has_vanilla_constructor(false),
_max_bootstrap_specifier_index(-1) {
- update_class_name(name != NULL ? name : vmSymbols::unknown_class_name());
+ _class_name = name != NULL ? name : vmSymbols::unknown_class_name();
+ _class_name->increment_refcount();
assert(THREAD->is_Java_thread(), "invariant");
assert(_loader_data != NULL, "invariant");
@@ -6086,8 +6091,7 @@
Symbol* const class_name_in_cp = cp->klass_name_at(_this_class_index);
assert(class_name_in_cp != NULL, "class_name can't be null");
- // Update _class_name which could be null previously
- // to reflect the name in the constant pool
+ // Update _class_name to reflect the name in the constant pool
update_class_name(class_name_in_cp);
// Don't need to check whether this class name is legal or not.
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/defineAnonClass/AnonSymbolLeak.java Wed Feb 13 07:22:09 2019 -0500
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+// This is copied from DefineAnon to test Symbol Refcounting for the package prepended name.
+
+package p1;
+
+import jdk.internal.org.objectweb.asm.ClassWriter;
+import jdk.internal.org.objectweb.asm.MethodVisitor;
+import jdk.internal.org.objectweb.asm.Opcodes;
+import jdk.internal.misc.Unsafe;
+
+
+class T {
+ static protected void test0() { System.out.println("test0 (public)"); }
+ static protected void test1() { System.out.println("test1 (protected)"); }
+ static /*package-private*/ void test2() { System.out.println("test2 (package)"); }
+ static private void test3() { System.out.println("test3 (private)"); }
+}
+
+public class AnonSymbolLeak {
+
+ private static final Unsafe UNSAFE = Unsafe.getUnsafe();
+
+ static Class<?> getAnonClass(Class<?> hostClass, final String className) {
+ final String superName = "java/lang/Object";
+ ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS + ClassWriter.COMPUTE_FRAMES);
+ cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL + Opcodes.ACC_SUPER, className, null, superName, null);
+
+ MethodVisitor mv = cw.visitMethod(Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC, "test", "()V", null, null);
+ mv.visitMethodInsn(Opcodes.INVOKESTATIC, "p1/T", "test0", "()V", false);
+ mv.visitMethodInsn(Opcodes.INVOKESTATIC, "p1/T", "test1", "()V", false);
+ mv.visitMethodInsn(Opcodes.INVOKESTATIC, "p1/T", "test2", "()V", false);
+ mv.visitMethodInsn(Opcodes.INVOKESTATIC, "p1/T", "test3", "()V", false);
+ mv.visitInsn(Opcodes.RETURN);
+ mv.visitMaxs(0, 0);
+ mv.visitEnd();
+
+ final byte[] classBytes = cw.toByteArray();
+ Class<?> invokerClass = UNSAFE.defineAnonymousClass(hostClass, classBytes, new Object[0]);
+ UNSAFE.ensureClassInitialized(invokerClass);
+ return invokerClass;
+ }
+
+ public static void test() throws Throwable {
+ // AnonClass is injected into package p1.
+ System.out.println("Injecting from the same package (p1):");
+ Class<?> p1cls = getAnonClass(T.class, "AnonClass");
+ p1cls.getMethod("test").invoke(null);
+ }
+
+ public static void main(java.lang.String[] unused) throws Throwable {
+ test();
+ }
+}
--- a/test/hotspot/jtreg/runtime/defineAnonClass/DefineAnon.java Wed Feb 13 06:48:34 2019 -0500
+++ b/test/hotspot/jtreg/runtime/defineAnonClass/DefineAnon.java Wed Feb 13 07:22:09 2019 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -27,6 +27,7 @@
* @library /testlibrary
* @modules java.base/jdk.internal.org.objectweb.asm
* java.management
+ * java.base/jdk.internal.misc
* @compile -XDignore.symbol.file=true DefineAnon.java
* @run main/othervm p1.DefineAnon
*/
@@ -36,7 +37,7 @@
import jdk.internal.org.objectweb.asm.ClassWriter;
import jdk.internal.org.objectweb.asm.MethodVisitor;
import jdk.internal.org.objectweb.asm.Opcodes;
-import sun.misc.Unsafe;
+import jdk.internal.misc.Unsafe;
class T {
@@ -48,17 +49,7 @@
public class DefineAnon {
- private static Unsafe getUnsafe() {
- try {
- java.lang.reflect.Field singleoneInstanceField = Unsafe.class.getDeclaredField("theUnsafe");
- singleoneInstanceField.setAccessible(true);
- return (Unsafe) singleoneInstanceField.get(null);
- } catch (Throwable ex) {
- throw new RuntimeException("Was unable to get Unsafe instance.");
- }
- }
-
- static Unsafe UNSAFE = DefineAnon.getUnsafe();
+ private static final Unsafe UNSAFE = Unsafe.getUnsafe();
static Class<?> getAnonClass(Class<?> hostClass, final String className) {
final String superName = "java/lang/Object";
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/defineAnonClass/TestAnonSymbolLeak.java Wed Feb 13 07:22:09 2019 -0500
@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2019, 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 TestAnonSymbolLeak
+ * @bug 8218755
+ * @requires vm.opt.final.ClassUnloading
+ * @modules java.base/jdk.internal.misc
+ * java.base/jdk.internal.org.objectweb.asm
+ * java.management
+ * @library /test/lib /runtime/testlibrary
+ * @build p1.AnonSymbolLeak
+ * @build sun.hotspot.WhiteBox
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ * sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -Xmn8m -XX:+UnlockDiagnosticVMOptions -Xlog:class+unload -XX:+WhiteBoxAPI TestAnonSymbolLeak
+ */
+
+import java.lang.reflect.Method;
+import sun.hotspot.WhiteBox;
+import jdk.test.lib.Asserts;
+
+public class TestAnonSymbolLeak {
+ static String className = "p1.AnonSymbolLeak";
+
+ private static class ClassUnloadTestMain {
+ public static void test() throws Exception {
+ // Load the AnonSymbolLeak class in a new class loader, run it, which loads
+ // an unsafe anonymous class in the same package p1. Then unload it.
+ // Then test that the refcount of the symbol created for the prepended name doesn't leak.
+ ClassLoader cl = ClassUnloadCommon.newClassLoader();
+ Class<?> c = cl.loadClass(className);
+ c.getMethod("test").invoke(null);
+ cl = null; c = null;
+ ClassUnloadCommon.triggerUnloading();
+ }
+ }
+
+ public static WhiteBox wb = WhiteBox.getWhiteBox();
+
+ public static void main(String... args) throws Exception {
+ String oldName = "AnonClass";
+ String prependedName = "p1/AnonClass";
+ ClassUnloadCommon.failIf(wb.isClassAlive(className), "should not be loaded");
+ int countBeforeOld = wb.getSymbolRefcount(oldName);
+ int countBefore = wb.getSymbolRefcount(prependedName);
+ ClassUnloadTestMain.test();
+ ClassUnloadCommon.failIf(wb.isClassAlive(className), "should be unloaded");
+ int countAfterOld = wb.getSymbolRefcount(oldName);
+ int countAfter = wb.getSymbolRefcount(prependedName);
+ Asserts.assertEquals(countBeforeOld, countAfterOld); // no leaks to the old name
+ System.out.println("count before and after " + countBeforeOld + " " + countAfterOld);
+ Asserts.assertEquals(countBefore, countAfter); // no leaks to the prepended name
+ System.out.println("count before and after " + countBefore + " " + countAfter);
+ }
+}
--- a/test/hotspot/jtreg/runtime/defineAnonClass/UnsafeDefMeths.java Wed Feb 13 06:48:34 2019 -0500
+++ b/test/hotspot/jtreg/runtime/defineAnonClass/UnsafeDefMeths.java Wed Feb 13 07:22:09 2019 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -28,6 +28,7 @@
* @library /testlibrary
* @modules java.base/jdk.internal.org.objectweb.asm
* java.management
+ * java.base/jdk.internal.misc
* @compile -XDignore.symbol.file=true UnsafeDefMeths.java
* @run main UnsafeDefMeths
*/
@@ -35,7 +36,7 @@
import jdk.internal.org.objectweb.asm.ClassWriter;
import jdk.internal.org.objectweb.asm.MethodVisitor;
import jdk.internal.org.objectweb.asm.Type;
-import sun.misc.Unsafe;
+import jdk.internal.misc.Unsafe;
import java.lang.invoke.MethodType;
import java.lang.reflect.Field;
@@ -57,18 +58,7 @@
public class UnsafeDefMeths {
- static final Unsafe UNSAFE;
-
- static {
- try {
- Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe");
- unsafeField.setAccessible(true);
- UNSAFE = (Unsafe) unsafeField.get(null);
- }
- catch (Exception e) {
- throw new InternalError(e);
- }
- }
+ private static final Unsafe UNSAFE = Unsafe.getUnsafe();
interface Resource {
Pointer ptr();