--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp Fri Feb 10 20:45:39 2017 +0300
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp Fri Feb 10 17:22:17 2017 +0100
@@ -1152,21 +1152,26 @@
Symbol* h_name = k->name();
assert(class_name == NULL || class_name == h_name, "name mismatch");
- bool define_succeeded = false;
// Add class just loaded
// If a class loader supports parallel classloading handle parallel define requests
// find_or_define_instance_class may return a different InstanceKlass
if (is_parallelCapable(class_loader)) {
- instanceKlassHandle defined_k = find_or_define_instance_class(h_name, class_loader, k, CHECK_NULL);
- if (k() == defined_k()) {
- // we have won over other concurrent threads (if any) that are
- // competing to define the same class.
- define_succeeded = true;
+ instanceKlassHandle defined_k = find_or_define_instance_class(h_name, class_loader, k, THREAD);
+ if (!HAS_PENDING_EXCEPTION && defined_k() != k()) {
+ // If a parallel capable class loader already defined this class, register 'k' for cleanup.
+ assert(defined_k.not_null(), "Should have a klass if there's no exception");
+ loader_data->add_to_deallocate_list(k());
+ k = defined_k;
}
- k = defined_k;
} else {
- define_instance_class(k, CHECK_NULL);
- define_succeeded = true;
+ define_instance_class(k, THREAD);
+ }
+
+ // If defining the class throws an exception register 'k' for cleanup.
+ if (HAS_PENDING_EXCEPTION) {
+ assert(k.not_null(), "Must have an instance klass here!");
+ loader_data->add_to_deallocate_list(k());
+ return NULL;
}
// Make sure we have an entry in the SystemDictionary on success
@@ -1518,8 +1523,16 @@
// find_or_define_instance_class may return a different InstanceKlass
if (!k.is_null()) {
instanceKlassHandle defined_k =
- find_or_define_instance_class(class_name, class_loader, k, CHECK_(nh));
- k = defined_k;
+ find_or_define_instance_class(class_name, class_loader, k, THREAD);
+ if (!HAS_PENDING_EXCEPTION && defined_k() != k()) {
+ // If a parallel capable class loader already defined this class, register 'k' for cleanup.
+ assert(defined_k.not_null(), "Should have a klass if there's no exception");
+ loader_data->add_to_deallocate_list(k());
+ k = defined_k;
+ } else if (HAS_PENDING_EXCEPTION) {
+ loader_data->add_to_deallocate_list(k());
+ return nh;
+ }
}
return k;
} else {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/Metaspace/DefineClass.java Fri Feb 10 17:22:17 2017 +0100
@@ -0,0 +1,380 @@
+/*
+ * Copyright (c) 2017 SAP SE. 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 8173743
+ * @summary Failures during class definition can lead to memory leaks in metaspace
+ * @library /test/lib
+ * @run main/othervm test.DefineClass defineClass
+ * @run main/othervm test.DefineClass defineSystemClass
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions
+ -XX:+UnsyncloadClass -XX:+AllowParallelDefineClass
+ test.DefineClass defineClassParallel
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions
+ -XX:+UnsyncloadClass -XX:-AllowParallelDefineClass
+ test.DefineClass defineClassParallel
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions
+ -XX:-UnsyncloadClass -XX:+AllowParallelDefineClass
+ test.DefineClass defineClassParallel
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions
+ -XX:-UnsyncloadClass -XX:-AllowParallelDefineClass
+ test.DefineClass defineClassParallel
+ * @run main/othervm test.DefineClass redefineClass
+ * @run main/othervm test.DefineClass redefineClassWithError
+ * @author volker.simonis@gmail.com
+ */
+
+package test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.InputStream;
+import java.lang.instrument.ClassDefinition;
+import java.lang.instrument.Instrumentation;
+import java.lang.management.ManagementFactory;
+import java.util.Scanner;
+import java.util.concurrent.CountDownLatch;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+
+import com.sun.tools.attach.VirtualMachine;
+
+import jdk.test.lib.process.ProcessTools;
+
+public class DefineClass {
+
+ private static Instrumentation instrumentation;
+
+ public void getID(CountDownLatch start, CountDownLatch stop) {
+ String id = "AAAAAAAA";
+ System.out.println(id);
+ try {
+ // Signal that we've entered the activation..
+ start.countDown();
+ //..and wait until we can leave it.
+ stop.await();
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ System.out.println(id);
+ return;
+ }
+
+ private static class MyThread extends Thread {
+ private DefineClass dc;
+ private CountDownLatch start, stop;
+
+ public MyThread(DefineClass dc, CountDownLatch start, CountDownLatch stop) {
+ this.dc = dc;
+ this.start = start;
+ this.stop = stop;
+ }
+
+ public void run() {
+ dc.getID(start, stop);
+ }
+ }
+
+ private static class ParallelLoadingThread extends Thread {
+ private MyParallelClassLoader pcl;
+ private CountDownLatch stop;
+ private byte[] buf;
+
+ public ParallelLoadingThread(MyParallelClassLoader pcl, byte[] buf, CountDownLatch stop) {
+ this.pcl = pcl;
+ this.stop = stop;
+ this.buf = buf;
+ }
+
+ public void run() {
+ try {
+ stop.await();
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ try {
+ @SuppressWarnings("unchecked")
+ Class<DefineClass> dc = (Class<DefineClass>) pcl.myDefineClass(DefineClass.class.getName(), buf, 0, buf.length);
+ }
+ catch (LinkageError jle) {
+ // Expected with a parallel capable class loader and
+ // -XX:+UnsyncloadClass or -XX:+AllowParallelDefineClass
+ pcl.incrementLinkageErrors();
+ }
+
+ }
+ }
+
+ static private class MyClassLoader extends ClassLoader {
+ public Class<?> myDefineClass(String name, byte[] b, int off, int len) throws ClassFormatError {
+ return defineClass(name, b, off, len, null);
+ }
+ }
+
+ static private class MyParallelClassLoader extends ClassLoader {
+ static {
+ System.out.println("parallelCapable : " + registerAsParallelCapable());
+ }
+ public Class<?> myDefineClass(String name, byte[] b, int off, int len) throws ClassFormatError {
+ return defineClass(name, b, off, len, null);
+ }
+ public synchronized void incrementLinkageErrors() {
+ linkageErrors++;
+ }
+ public int getLinkageErrors() {
+ return linkageErrors;
+ }
+ private volatile int linkageErrors;
+ }
+
+ public static void agentmain(String args, Instrumentation inst) {
+ System.out.println("Loading Java Agent.");
+ instrumentation = inst;
+ }
+
+
+ private static void loadInstrumentationAgent(String myName, byte[] buf) throws Exception {
+ // Create agent jar file on the fly
+ Manifest m = new Manifest();
+ m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+ m.getMainAttributes().put(new Attributes.Name("Agent-Class"), myName);
+ m.getMainAttributes().put(new Attributes.Name("Can-Redefine-Classes"), "true");
+ File jarFile = File.createTempFile("agent", ".jar");
+ jarFile.deleteOnExit();
+ JarOutputStream jar = new JarOutputStream(new FileOutputStream(jarFile), m);
+ jar.putNextEntry(new JarEntry(myName.replace('.', '/') + ".class"));
+ jar.write(buf);
+ jar.close();
+ String pid = Long.toString(ProcessTools.getProcessId());
+ System.out.println("Our pid is = " + pid);
+ VirtualMachine vm = VirtualMachine.attach(pid);
+ vm.loadAgent(jarFile.getAbsolutePath());
+ }
+
+ private static byte[] getBytecodes(String myName) throws Exception {
+ InputStream is = DefineClass.class.getResourceAsStream(myName + ".class");
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ byte[] buf = new byte[4096];
+ int len;
+ while ((len = is.read(buf)) != -1) baos.write(buf, 0, len);
+ buf = baos.toByteArray();
+ System.out.println("sizeof(" + myName + ".class) == " + buf.length);
+ return buf;
+ }
+
+ private static int getStringIndex(String needle, byte[] buf) {
+ return getStringIndex(needle, buf, 0);
+ }
+
+ private static int getStringIndex(String needle, byte[] buf, int offset) {
+ outer:
+ for (int i = offset; i < buf.length - offset - needle.length(); i++) {
+ for (int j = 0; j < needle.length(); j++) {
+ if (buf[i + j] != (byte)needle.charAt(j)) continue outer;
+ }
+ return i;
+ }
+ return 0;
+ }
+
+ private static void replaceString(byte[] buf, String name, int index) {
+ for (int i = index; i < index + name.length(); i++) {
+ buf[i] = (byte)name.charAt(i - index);
+ }
+ }
+
+ private static MBeanServer mbserver = ManagementFactory.getPlatformMBeanServer();
+
+ private static int getClassStats(String pattern) {
+ try {
+ ObjectName diagCmd = new ObjectName("com.sun.management:type=DiagnosticCommand");
+
+ String result = (String)mbserver.invoke(diagCmd , "gcClassStats" , new Object[] { null }, new String[] {String[].class.getName()});
+ int count = 0;
+ try (Scanner s = new Scanner(result)) {
+ if (s.hasNextLine()) {
+ System.out.println(s.nextLine());
+ }
+ while (s.hasNextLine()) {
+ String l = s.nextLine();
+ if (l.endsWith(pattern)) {
+ count++;
+ System.out.println(l);
+ }
+ }
+ }
+ return count;
+ }
+ catch (Exception e) {
+ throw new RuntimeException("Test failed because we can't read the class statistics!", e);
+ }
+ }
+
+ private static void printClassStats(int expectedCount, boolean reportError) {
+ int count = getClassStats("DefineClass");
+ String res = "Should have " + expectedCount +
+ " DefineClass instances and we have: " + count;
+ System.out.println(res);
+ if (reportError && count != expectedCount) {
+ throw new RuntimeException(res);
+ }
+ }
+
+ public static final int ITERATIONS = 10;
+
+ public static void main(String[] args) throws Exception {
+ String myName = DefineClass.class.getName();
+ byte[] buf = getBytecodes(myName.substring(myName.lastIndexOf(".") + 1));
+ int iterations = (args.length > 1 ? Integer.parseInt(args[1]) : ITERATIONS);
+
+ if (args.length == 0 || "defineClass".equals(args[0])) {
+ MyClassLoader cl = new MyClassLoader();
+ for (int i = 0; i < iterations; i++) {
+ try {
+ @SuppressWarnings("unchecked")
+ Class<DefineClass> dc = (Class<DefineClass>) cl.myDefineClass(myName, buf, 0, buf.length);
+ System.out.println(dc);
+ }
+ catch (LinkageError jle) {
+ // Can only define once!
+ if (i == 0) throw new Exception("Should succeed the first time.");
+ }
+ }
+ // We expect to have two instances of DefineClass here: the initial version in which we are
+ // executing and another version which was loaded into our own classloader 'MyClassLoader'.
+ // All the subsequent attempts to reload DefineClass into our 'MyClassLoader' should have failed.
+ printClassStats(2, false);
+ System.gc();
+ System.out.println("System.gc()");
+ // At least after System.gc() the failed loading attempts should leave no instances around!
+ printClassStats(2, true);
+ }
+ else if ("defineSystemClass".equals(args[0])) {
+ MyClassLoader cl = new MyClassLoader();
+ int index = getStringIndex("test/DefineClass", buf);
+ replaceString(buf, "java/DefineClass", index);
+ while ((index = getStringIndex("Ltest/DefineClass;", buf, index + 1)) != 0) {
+ replaceString(buf, "Ljava/DefineClass;", index);
+ }
+ index = getStringIndex("test.DefineClass", buf);
+ replaceString(buf, "java.DefineClass", index);
+
+ for (int i = 0; i < iterations; i++) {
+ try {
+ @SuppressWarnings("unchecked")
+ Class<DefineClass> dc = (Class<DefineClass>) cl.myDefineClass(null, buf, 0, buf.length);
+ throw new RuntimeException("Defining a class in the 'java' package should fail!");
+ }
+ catch (java.lang.SecurityException jlse) {
+ // Expected, because we're not allowed to define a class in the 'java' package
+ }
+ }
+ // We expect to stay with one (the initial) instances of DefineClass.
+ // All the subsequent attempts to reload DefineClass into the 'java' package should have failed.
+ printClassStats(1, false);
+ System.gc();
+ System.out.println("System.gc()");
+ // At least after System.gc() the failed loading attempts should leave no instances around!
+ printClassStats(1, true);
+ }
+ else if ("defineClassParallel".equals(args[0])) {
+ MyParallelClassLoader pcl = new MyParallelClassLoader();
+ CountDownLatch stop = new CountDownLatch(1);
+
+ Thread[] threads = new Thread[iterations];
+ for (int i = 0; i < iterations; i++) {
+ (threads[i] = new ParallelLoadingThread(pcl, buf, stop)).start();
+ }
+ stop.countDown(); // start parallel class loading..
+ // ..and wait until all threads loaded the class
+ for (int i = 0; i < iterations; i++) {
+ threads[i].join();
+ }
+ System.out.print("Counted " + pcl.getLinkageErrors() + " LinkageErrors ");
+ System.out.println(pcl.getLinkageErrors() == 0 ?
+ "" : "(use -XX:+UnsyncloadClass and/or -XX:+AllowParallelDefineClass to avoid this)");
+ System.gc();
+ System.out.println("System.gc()");
+ // After System.gc() we expect to remain with two instances: one is the initial version which is
+ // kept alive by this main method and another one in the parallel class loader.
+ printClassStats(2, true);
+ }
+ else if ("redefineClass".equals(args[0])) {
+ loadInstrumentationAgent(myName, buf);
+ int index = getStringIndex("AAAAAAAA", buf);
+ CountDownLatch stop = new CountDownLatch(1);
+
+ Thread[] threads = new Thread[iterations];
+ for (int i = 0; i < iterations; i++) {
+ buf[index] = (byte) ('A' + i + 1); // Change string constant in getID() which is legal in redefinition
+ instrumentation.redefineClasses(new ClassDefinition(DefineClass.class, buf));
+ DefineClass dc = DefineClass.class.newInstance();
+ CountDownLatch start = new CountDownLatch(1);
+ (threads[i] = new MyThread(dc, start, stop)).start();
+ start.await(); // Wait until the new thread entered the getID() method
+ }
+ // We expect to have one instance for each redefinition because they are all kept alive by an activation
+ // plus the initial version which is kept active by this main method.
+ printClassStats(iterations + 1, false);
+ stop.countDown(); // Let all threads leave the DefineClass.getID() activation..
+ // ..and wait until really all of them returned from DefineClass.getID()
+ for (int i = 0; i < iterations; i++) {
+ threads[i].join();
+ }
+ System.gc();
+ System.out.println("System.gc()");
+ // After System.gc() we expect to remain with two instances: one is the initial version which is
+ // kept alive by this main method and another one which is the latest redefined version.
+ printClassStats(2, true);
+ }
+ else if ("redefineClassWithError".equals(args[0])) {
+ loadInstrumentationAgent(myName, buf);
+ int index = getStringIndex("getID", buf);
+
+ for (int i = 0; i < iterations; i++) {
+ buf[index] = (byte) 'X'; // Change getID() to XetID() which is illegal in redefinition
+ try {
+ instrumentation.redefineClasses(new ClassDefinition(DefineClass.class, buf));
+ throw new RuntimeException("Class redefinition isn't allowed to change method names!");
+ }
+ catch (UnsupportedOperationException uoe) {
+ // Expected because redefinition can't change the name of methods
+ }
+ }
+ // We expect just a single DefineClass instance because failed redefinitions should
+ // leave no garbage around.
+ printClassStats(1, false);
+ System.gc();
+ System.out.println("System.gc()");
+ // At least after a System.gc() we should definitely stay with a single instance!
+ printClassStats(1, true);
+ }
+ }
+}