8010196: NPG: Internal Error: Metaspace allocation lock -- possible deadlock
Summary: Refactor the CLD dependency list into a separate class. Use an ObjectLocker to synchronize additions to the CLD dependency list.
Reviewed-by: stefank, coleenp
--- a/hotspot/src/share/vm/classfile/classLoaderData.cpp Tue Apr 09 15:32:45 2013 +0200
+++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp Wed Apr 10 13:27:35 2013 +0200
@@ -70,15 +70,19 @@
_is_anonymous(is_anonymous), _keep_alive(is_anonymous), // initially
_metaspace(NULL), _unloading(false), _klasses(NULL),
_claimed(0), _jmethod_ids(NULL), _handles(NULL), _deallocate_list(NULL),
- _next(NULL), _dependencies(NULL),
+ _next(NULL), _dependencies(),
_metaspace_lock(new Mutex(Monitor::leaf+1, "Metaspace allocation lock", true)) {
// empty
}
void ClassLoaderData::init_dependencies(TRAPS) {
+ _dependencies.init(CHECK);
+}
+
+void ClassLoaderData::Dependencies::init(TRAPS) {
// Create empty dependencies array to add to. CMS requires this to be
// an oop so that it can track additions via card marks. We think.
- _dependencies = (oop)oopFactory::new_objectArray(2, CHECK);
+ _list_head = oopFactory::new_objectArray(2, CHECK);
}
bool ClassLoaderData::claim() {
@@ -95,13 +99,17 @@
}
f->do_oop(&_class_loader);
- f->do_oop(&_dependencies);
+ _dependencies.oops_do(f);
_handles->oops_do(f);
if (klass_closure != NULL) {
classes_do(klass_closure);
}
}
+void ClassLoaderData::Dependencies::oops_do(OopClosure* f) {
+ f->do_oop((oop*)&_list_head);
+}
+
void ClassLoaderData::classes_do(KlassClosure* klass_closure) {
for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
klass_closure->do_klass(k);
@@ -154,14 +162,14 @@
// It's a dependency we won't find through GC, add it. This is relatively rare
// Must handle over GC point.
Handle dependency(THREAD, to);
- from_cld->add_dependency(dependency, CHECK);
+ from_cld->_dependencies.add(dependency, CHECK);
}
-void ClassLoaderData::add_dependency(Handle dependency, TRAPS) {
+void ClassLoaderData::Dependencies::add(Handle dependency, TRAPS) {
// Check first if this dependency is already in the list.
// Save a pointer to the last to add to under the lock.
- objArrayOop ok = (objArrayOop)_dependencies;
+ objArrayOop ok = _list_head;
objArrayOop last = NULL;
while (ok != NULL) {
last = ok;
@@ -184,16 +192,17 @@
objArrayHandle new_dependency(THREAD, deps);
// Add the dependency under lock
- locked_add_dependency(last_handle, new_dependency);
+ locked_add(last_handle, new_dependency, THREAD);
}
-void ClassLoaderData::locked_add_dependency(objArrayHandle last_handle,
- objArrayHandle new_dependency) {
+void ClassLoaderData::Dependencies::locked_add(objArrayHandle last_handle,
+ objArrayHandle new_dependency,
+ Thread* THREAD) {
// Have to lock and put the new dependency on the end of the dependency
// array so the card mark for CMS sees that this dependency is new.
// Can probably do this lock free with some effort.
- MutexLockerEx ml(metaspace_lock(), Mutex::_no_safepoint_check_flag);
+ ObjectLocker ol(Handle(THREAD, _list_head), THREAD);
oop loader_or_mirror = new_dependency->obj_at(0);
--- a/hotspot/src/share/vm/classfile/classLoaderData.hpp Tue Apr 09 15:32:45 2013 +0200
+++ b/hotspot/src/share/vm/classfile/classLoaderData.hpp Wed Apr 10 13:27:35 2013 +0200
@@ -93,6 +93,18 @@
class ClassLoaderData : public CHeapObj<mtClass> {
friend class VMStructs;
private:
+ class Dependencies VALUE_OBJ_CLASS_SPEC {
+ objArrayOop _list_head;
+ void locked_add(objArrayHandle last,
+ objArrayHandle new_dependency,
+ Thread* THREAD);
+ public:
+ Dependencies() : _list_head(NULL) {}
+ void add(Handle dependency, TRAPS);
+ void init(TRAPS);
+ void oops_do(OopClosure* f);
+ };
+
friend class ClassLoaderDataGraph;
friend class ClassLoaderDataGraphMetaspaceIterator;
friend class MetaDataFactory;
@@ -100,10 +112,11 @@
static ClassLoaderData * _the_null_class_loader_data;
- oop _class_loader; // oop used to uniquely identify a class loader
- // class loader or a canonical class path
- oop _dependencies; // oop to hold dependencies from this class loader
- // data to others.
+ oop _class_loader; // oop used to uniquely identify a class loader
+ // class loader or a canonical class path
+ Dependencies _dependencies; // holds dependencies from this class loader
+ // data to others.
+
Metaspace * _metaspace; // Meta-space where meta-data defined by the
// classes in the class loader are allocated.
Mutex* _metaspace_lock; // Locks the metaspace for allocations and setup.
@@ -134,9 +147,6 @@
static Metaspace* _ro_metaspace;
static Metaspace* _rw_metaspace;
- void add_dependency(Handle dependency, TRAPS);
- void locked_add_dependency(objArrayHandle last, objArrayHandle new_dependency);
-
void set_next(ClassLoaderData* next) { _next = next; }
ClassLoaderData* next() const { return _next; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/metaspace/G1AddMetaspaceDependency.java Wed Apr 10 13:27:35 2013 +0200
@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2013, 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 G1AddMetaspaceDependency
+ * @bug 8010196
+ * @summary Checks that we don't get locking problems when adding metaspace dependencies with the G1 update buffer monitor
+ * @run main/othervm -XX:+UseG1GC -XX:G1UpdateBufferSize=1 G1AddMetaspaceDependency
+ */
+
+import java.io.InputStream;
+
+public class G1AddMetaspaceDependency {
+
+ static byte[] getClassBytes(String name) {
+ byte[] b = null;
+ try (InputStream is = ClassLoader.getSystemResourceAsStream(name)) {
+ byte[] tmp = new byte[is.available()];
+ is.read(tmp);
+ b = tmp;
+ } finally {
+ if (b == null) {
+ throw new RuntimeException("Unable to load class file");
+ }
+ return b;
+ }
+ }
+
+ static final String a_name = G1AddMetaspaceDependency.class.getName() + "$A";
+ static final String b_name = G1AddMetaspaceDependency.class.getName() + "$B";
+
+ public static void main(String... args) throws Exception {
+ final byte[] a_bytes = getClassBytes(a_name + ".class");
+ final byte[] b_bytes = getClassBytes(b_name + ".class");
+
+ for (int i = 0; i < 1000; i += 1) {
+ runTest(a_bytes, b_bytes);
+ }
+ }
+
+ static class Loader extends ClassLoader {
+ private final String myClass;
+ private final byte[] myBytes;
+ private final String friendClass;
+ private final ClassLoader friendLoader;
+
+ Loader(String myClass, byte[] myBytes,
+ String friendClass, ClassLoader friendLoader) {
+ this.myClass = myClass;
+ this.myBytes = myBytes;
+ this.friendClass = friendClass;
+ this.friendLoader = friendLoader;
+ }
+
+ Loader(String myClass, byte[] myBytes) {
+ this(myClass, myBytes, null, null);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ Class<?> c = findLoadedClass(name);
+ if (c != null) {
+ return c;
+ }
+
+ if (name.equals(friendClass)) {
+ return friendLoader.loadClass(name);
+ }
+
+ if (name.equals(myClass)) {
+ c = defineClass(name, myBytes, 0, myBytes.length);
+ resolveClass(c);
+ return c;
+ }
+
+ return findSystemClass(name);
+ }
+
+ }
+
+ private static void runTest(final byte[] a_bytes, final byte[] b_bytes) throws Exception {
+ Loader a_loader = new Loader(a_name, a_bytes);
+ Loader b_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+ Loader c_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+ Loader d_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+ Loader e_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+ Loader f_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+ Loader g_loader = new Loader(b_name, b_bytes, a_name, a_loader);
+
+ byte[] b = new byte[20 * 2 << 20];
+ Class<?> c;
+ c = b_loader.loadClass(b_name);
+ c = c_loader.loadClass(b_name);
+ c = d_loader.loadClass(b_name);
+ c = e_loader.loadClass(b_name);
+ c = f_loader.loadClass(b_name);
+ c = g_loader.loadClass(b_name);
+ }
+ public class A {
+ }
+ class B extends A {
+ }
+}