8028497: SIGSEGV at ClassLoaderData::oops_do(OopClosure*, KlassClosure*, bool)
Summary: Keep class in CLD::_klasses list and mirror created for CDS classes if OOM during restore_shareable_info(). This keeps pointers consistent for CMS.
Reviewed-by: ehelin, stefank, jmasa, iklam
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -4180,8 +4180,12 @@
clear_class_metadata();
- // deallocate the klass if already created.
- MetadataFactory::free_metadata(_loader_data, _klass);
+ // deallocate the klass if already created. Don't directly deallocate, but add
+ // to the deallocate list so that the klass is removed from the CLD::_klasses list
+ // at a safepoint.
+ if (_klass != NULL) {
+ _loader_data->add_to_deallocate_list(_klass);
+ }
_klass = NULL;
}
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/classfile/javaClasses.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2014, 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
@@ -482,8 +482,8 @@
st->print("\"");
}
-static void initialize_static_field(fieldDescriptor* fd, TRAPS) {
- Handle mirror (THREAD, fd->field_holder()->java_mirror());
+
+static void initialize_static_field(fieldDescriptor* fd, Handle mirror, TRAPS) {
assert(mirror.not_null() && fd->is_static(), "just checking");
if (fd->has_initial_value()) {
BasicType t = fd->field_type();
@@ -550,21 +550,45 @@
create_mirror(k, Handle(NULL), CHECK);
}
-oop java_lang_Class::create_mirror(KlassHandle k, Handle protection_domain, TRAPS) {
+void java_lang_Class::initialize_mirror_fields(KlassHandle k,
+ Handle mirror,
+ Handle protection_domain,
+ TRAPS) {
+ // Allocate a simple java object for a lock.
+ // This needs to be a java object because during class initialization
+ // it can be held across a java call.
+ typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK);
+ set_init_lock(mirror(), r);
+
+ // Set protection domain also
+ set_protection_domain(mirror(), protection_domain());
+
+ // Initialize static fields
+ InstanceKlass::cast(k())->do_local_static_fields(&initialize_static_field, mirror, CHECK);
+}
+
+void java_lang_Class::create_mirror(KlassHandle k, Handle protection_domain, TRAPS) {
assert(k->java_mirror() == NULL, "should only assign mirror once");
// Use this moment of initialization to cache modifier_flags also,
// to support Class.getModifiers(). Instance classes recalculate
// the cached flags after the class file is parsed, but before the
// class is put into the system dictionary.
- int computed_modifiers = k->compute_modifier_flags(CHECK_0);
+ int computed_modifiers = k->compute_modifier_flags(CHECK);
k->set_modifier_flags(computed_modifiers);
// Class_klass has to be loaded because it is used to allocate
// the mirror.
if (SystemDictionary::Class_klass_loaded()) {
// Allocate mirror (java.lang.Class instance)
- Handle mirror = InstanceMirrorKlass::cast(SystemDictionary::Class_klass())->allocate_instance(k, CHECK_0);
+ Handle mirror = InstanceMirrorKlass::cast(SystemDictionary::Class_klass())->allocate_instance(k, CHECK);
+
+ // Setup indirection from mirror->klass
+ if (!k.is_null()) {
+ java_lang_Class::set_klass(mirror(), k());
+ }
InstanceMirrorKlass* mk = InstanceMirrorKlass::cast(mirror->klass());
+ assert(oop_size(mirror()) == mk->instance_size(k), "should have been set");
+
java_lang_Class::set_static_oop_field_count(mirror(), mk->compute_static_oop_field_count(mirror()));
// It might also have a component mirror. This mirror must already exist.
@@ -577,29 +601,32 @@
assert(k->oop_is_objArray(), "Must be");
Klass* element_klass = ObjArrayKlass::cast(k())->element_klass();
assert(element_klass != NULL, "Must have an element klass");
- comp_mirror = element_klass->java_mirror();
+ comp_mirror = element_klass->java_mirror();
}
assert(comp_mirror.not_null(), "must have a mirror");
- // Two-way link between the array klass and its component mirror:
+ // Two-way link between the array klass and its component mirror:
ArrayKlass::cast(k())->set_component_mirror(comp_mirror());
set_array_klass(comp_mirror(), k());
} else {
assert(k->oop_is_instance(), "Must be");
- // Allocate a simple java object for a lock.
- // This needs to be a java object because during class initialization
- // it can be held across a java call.
- typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK_NULL);
- set_init_lock(mirror(), r);
-
- // Set protection domain also
- set_protection_domain(mirror(), protection_domain());
-
- // Initialize static fields
- InstanceKlass::cast(k())->do_local_static_fields(&initialize_static_field, CHECK_NULL);
+ initialize_mirror_fields(k, mirror, protection_domain, THREAD);
+ if (HAS_PENDING_EXCEPTION) {
+ // If any of the fields throws an exception like OOM remove the klass field
+ // from the mirror so GC doesn't follow it after the klass has been deallocated.
+ // This mirror looks like a primitive type, which logically it is because it
+ // it represents no class.
+ java_lang_Class::set_klass(mirror(), NULL);
+ return;
+ }
}
- return mirror();
+
+ // Setup indirection from klass->mirror last
+ // after any exceptions can happen during allocations.
+ if (!k.is_null()) {
+ k->set_java_mirror(mirror());
+ }
} else {
if (fixup_mirror_list() == NULL) {
GrowableArray<Klass*>* list =
@@ -607,12 +634,10 @@
set_fixup_mirror_list(list);
}
fixup_mirror_list()->push(k());
- return NULL;
}
}
-
int java_lang_Class::oop_size(oop java_class) {
assert(_oop_size_offset != 0, "must be set");
return java_class->int_field(_oop_size_offset);
--- a/hotspot/src/share/vm/classfile/javaClasses.hpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/classfile/javaClasses.hpp Tue Apr 08 13:58:38 2014 -0400
@@ -246,11 +246,12 @@
static void set_init_lock(oop java_class, oop init_lock);
static void set_protection_domain(oop java_class, oop protection_domain);
+ static void initialize_mirror_fields(KlassHandle k, Handle mirror, Handle protection_domain, TRAPS);
public:
static void compute_offsets();
// Instance creation
- static oop create_mirror(KlassHandle k, Handle protection_domain, TRAPS);
+ static void create_mirror(KlassHandle k, Handle protection_domain, TRAPS);
static void fixup_mirror(KlassHandle k, TRAPS);
static oop create_basic_type_mirror(const char* basic_type_name, BasicType type, TRAPS);
// Conversion
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -826,47 +826,6 @@
}
} // load_instance_class loop
- if (HAS_PENDING_EXCEPTION) {
- // An exception, such as OOM could have happened at various places inside
- // load_instance_class. We might have partially initialized a shared class
- // and need to clean it up.
- if (class_loader.is_null()) {
- // In some cases k may be null. Let's find the shared class again.
- instanceKlassHandle ik(THREAD, find_shared_class(name));
- if (ik.not_null()) {
- if (ik->class_loader_data() == NULL) {
- // We didn't go as far as Klass::restore_unshareable_info(),
- // so nothing to clean up.
- } else {
- Klass *kk;
- {
- MutexLocker mu(SystemDictionary_lock, THREAD);
- kk = find_class(d_index, d_hash, name, ik->class_loader_data());
- }
- if (kk != NULL) {
- // No clean up is needed if the shared class has been entered
- // into system dictionary, as load_shared_class() won't be called
- // again.
- } else {
- // This must be done outside of the SystemDictionary_lock to
- // avoid deadlock.
- //
- // Note that Klass::restore_unshareable_info (called via
- // load_instance_class above) is also called outside
- // of SystemDictionary_lock. Other threads are blocked from
- // loading this class because they are waiting on the
- // SystemDictionary_lock until this thread removes
- // the placeholder below.
- //
- // This need to be re-thought when parallel-capable non-boot
- // classloaders are supported by CDS (today they're not).
- clean_up_shared_class(ik, class_loader, THREAD);
- }
- }
- }
- }
- }
-
if (load_instance_added == true) {
// clean up placeholder entries for LOAD_INSTANCE success or error
// This brackets the SystemDictionary updates for both defining
@@ -1272,19 +1231,6 @@
return ik;
}
-void SystemDictionary::clean_up_shared_class(instanceKlassHandle ik, Handle class_loader, TRAPS) {
- // Updating methods must be done under a lock so multiple
- // threads don't update these in parallel
- // Shared classes are all currently loaded by the bootstrap
- // classloader, so this will never cause a deadlock on
- // a custom class loader lock.
- {
- Handle lockObject = compute_loader_lock_object(class_loader, THREAD);
- check_loader_lock_contention(lockObject, THREAD);
- ObjectLocker ol(lockObject, THREAD, true);
- ik->remove_unshareable_info();
- }
-}
instanceKlassHandle SystemDictionary::load_instance_class(Symbol* class_name, Handle class_loader, TRAPS) {
instanceKlassHandle nh = instanceKlassHandle(); // null Handle
--- a/hotspot/src/share/vm/classfile/systemDictionary.hpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/classfile/systemDictionary.hpp Tue Apr 08 13:58:38 2014 -0400
@@ -617,7 +617,6 @@
Handle class_loader, TRAPS);
static instanceKlassHandle load_shared_class(instanceKlassHandle ik,
Handle class_loader, TRAPS);
- static void clean_up_shared_class(instanceKlassHandle ik, Handle class_loader, TRAPS);
static instanceKlassHandle load_instance_class(Symbol* class_name, Handle class_loader, TRAPS);
static Handle compute_loader_lock_object(Handle class_loader, TRAPS);
static void check_loader_lock_contention(Handle loader_lock, TRAPS);
--- a/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2014, 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
@@ -582,36 +582,6 @@
}
}
-oop CollectedHeap::Class_obj_allocate(KlassHandle klass, int size, KlassHandle real_klass, TRAPS) {
- debug_only(check_for_valid_allocation_state());
- assert(!Universe::heap()->is_gc_active(), "Allocation during gc not allowed");
- assert(size >= 0, "int won't convert to size_t");
- HeapWord* obj;
- assert(ScavengeRootsInCode > 0, "must be");
- obj = common_mem_allocate_init(real_klass, size, CHECK_NULL);
- post_allocation_setup_common(klass, obj);
- assert(Universe::is_bootstrapping() ||
- !((oop)obj)->is_array(), "must not be an array");
- NOT_PRODUCT(Universe::heap()->check_for_bad_heap_word_value(obj, size));
- oop mirror = (oop)obj;
-
- java_lang_Class::set_oop_size(mirror, size);
-
- // Setup indirections
- if (!real_klass.is_null()) {
- java_lang_Class::set_klass(mirror, real_klass());
- real_klass->set_java_mirror(mirror);
- }
-
- InstanceMirrorKlass* mk = InstanceMirrorKlass::cast(mirror->klass());
- assert(size == mk->instance_size(real_klass), "should have been set");
-
- // notify jvmti and dtrace
- post_allocation_notify(klass, (oop)obj);
-
- return mirror;
-}
-
/////////////// Unit tests ///////////////
#ifndef PRODUCT
--- a/hotspot/src/share/vm/gc_interface/collectedHeap.hpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/gc_interface/collectedHeap.hpp Tue Apr 08 13:58:38 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2014, 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
@@ -312,9 +312,6 @@
// May be overridden to set additional parallelism.
virtual void set_par_threads(uint t) { _n_par_threads = t; };
- // Allocate and initialize instances of Class
- static oop Class_obj_allocate(KlassHandle klass, int size, KlassHandle real_klass, TRAPS);
-
// General obj/array allocation facilities.
inline static oop obj_allocate(KlassHandle klass, int size, TRAPS);
inline static oop array_allocate(KlassHandle klass, int size, int length, TRAPS);
--- a/hotspot/src/share/vm/oops/constantPool.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/constantPool.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -144,6 +144,10 @@
// CDS support. Create a new resolved_references array.
void ConstantPool::restore_unshareable_info(TRAPS) {
+ // Only create the new resolved references array and lock if it hasn't been
+ // attempted before
+ if (resolved_references() != NULL) return;
+
// restore the C++ vtable from the shared archive
restore_vtable();
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -1289,17 +1289,18 @@
}
-void InstanceKlass::do_local_static_fields(void f(fieldDescriptor*, TRAPS), TRAPS) {
+void InstanceKlass::do_local_static_fields(void f(fieldDescriptor*, Handle, TRAPS), Handle mirror, TRAPS) {
instanceKlassHandle h_this(THREAD, this);
- do_local_static_fields_impl(h_this, f, CHECK);
+ do_local_static_fields_impl(h_this, f, mirror, CHECK);
}
-void InstanceKlass::do_local_static_fields_impl(instanceKlassHandle this_k, void f(fieldDescriptor* fd, TRAPS), TRAPS) {
+void InstanceKlass::do_local_static_fields_impl(instanceKlassHandle this_k,
+ void f(fieldDescriptor* fd, Handle, TRAPS), Handle mirror, TRAPS) {
for (JavaFieldStream fs(this_k()); !fs.done(); fs.next()) {
if (fs.access_flags().is_static()) {
fieldDescriptor& fd = fs.field_descriptor();
- f(&fd, CHECK);
+ f(&fd, mirror, CHECK);
}
}
}
@@ -2240,9 +2241,7 @@
int num_methods = methods->length();
for (int index2 = 0; index2 < num_methods; ++index2) {
methodHandle m(THREAD, methods->at(index2));
- m()->link_method(m, CHECK);
- // restore method's vtable by calling a virtual function
- m->restore_vtable();
+ m->restore_unshareable_info(CHECK);
}
if (JvmtiExport::has_redefined_a_class()) {
// Reinitialize vtable because RedefineClasses may have changed some
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Tue Apr 08 13:58:38 2014 -0400
@@ -802,7 +802,7 @@
// Iterators
void do_local_static_fields(FieldClosure* cl);
void do_nonstatic_fields(FieldClosure* cl); // including inherited fields
- void do_local_static_fields(void f(fieldDescriptor*, TRAPS), TRAPS);
+ void do_local_static_fields(void f(fieldDescriptor*, Handle, TRAPS), Handle, TRAPS);
void methods_do(void f(Method* method));
void array_klasses_do(void f(Klass* k));
@@ -1010,7 +1010,7 @@
static void set_initialization_state_and_notify_impl (instanceKlassHandle this_k, ClassState state, TRAPS);
static void call_class_initializer_impl (instanceKlassHandle this_k, TRAPS);
static Klass* array_klass_impl (instanceKlassHandle this_k, bool or_null, int n, TRAPS);
- static void do_local_static_fields_impl (instanceKlassHandle this_k, void f(fieldDescriptor* fd, TRAPS), TRAPS);
+ static void do_local_static_fields_impl (instanceKlassHandle this_k, void f(fieldDescriptor* fd, Handle, TRAPS), Handle, TRAPS);
/* jni_id_for_impl for jfieldID only */
static JNIid* jni_id_for_impl (instanceKlassHandle this_k, int offset);
--- a/hotspot/src/share/vm/oops/instanceMirrorKlass.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/instanceMirrorKlass.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -367,7 +367,12 @@
// Query before forming handle.
int size = instance_size(k);
KlassHandle h_k(THREAD, this);
- instanceOop i = (instanceOop) CollectedHeap::Class_obj_allocate(h_k, size, k, CHECK_NULL);
+ instanceOop i = (instanceOop)CollectedHeap::obj_allocate(h_k, size, CHECK_NULL);
+
+ // Since mirrors can be variable sized because of the static fields, store
+ // the size in the mirror itself.
+ java_lang_Class::set_oop_size(i, size);
+
return i;
}
--- a/hotspot/src/share/vm/oops/klass.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/klass.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -475,12 +475,8 @@
}
void Klass::remove_unshareable_info() {
- if (!DumpSharedSpaces) {
- // Clean up after OOM during class loading
- if (class_loader_data() != NULL) {
- class_loader_data()->remove_class(this);
- }
- }
+ assert (DumpSharedSpaces, "only called for DumpSharedSpaces");
+
set_subklass(NULL);
set_next_sibling(NULL);
// Clear the java mirror
@@ -492,17 +488,26 @@
}
void Klass::restore_unshareable_info(TRAPS) {
- ClassLoaderData* loader_data = ClassLoaderData::the_null_class_loader_data();
- // Restore class_loader_data to the null class loader data
- set_class_loader_data(loader_data);
+ // If an exception happened during CDS restore, some of these fields may already be
+ // set. We leave the class on the CLD list, even if incomplete so that we don't
+ // modify the CLD list outside a safepoint.
+ if (class_loader_data() == NULL) {
+ ClassLoaderData* loader_data = ClassLoaderData::the_null_class_loader_data();
+ // Restore class_loader_data to the null class loader data
+ set_class_loader_data(loader_data);
- // Add to null class loader list first before creating the mirror
- // (same order as class file parsing)
- loader_data->add_class(this);
+ // Add to null class loader list first before creating the mirror
+ // (same order as class file parsing)
+ loader_data->add_class(this);
+ }
// Recreate the class mirror. The protection_domain is always null for
// boot loader, for now.
- java_lang_Class::create_mirror(this, Handle(NULL), CHECK);
+ // Only recreate it if not present. A previous attempt to restore may have
+ // gotten an OOM later but keep the mirror if it was created.
+ if (java_mirror() == NULL) {
+ java_lang_Class::create_mirror(this, Handle(NULL), CHECK);
+ }
}
Klass* Klass::array_klass_or_null(int rank) {
--- a/hotspot/src/share/vm/oops/method.cpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/method.cpp Tue Apr 08 13:58:38 2014 -0400
@@ -903,6 +903,19 @@
return adapter->get_c2i_entry();
}
+void Method::restore_unshareable_info(TRAPS) {
+ // Since restore_unshareable_info can be called more than once for a method, don't
+ // redo any work. If this field is restored, there is nothing to do.
+ if (_from_compiled_entry == NULL) {
+ // restore method's vtable by calling a virtual function
+ restore_vtable();
+
+ methodHandle mh(THREAD, this);
+ link_method(mh, CHECK);
+ }
+}
+
+
// The verified_code_entry() must be called when a invoke is resolved
// on this method.
--- a/hotspot/src/share/vm/oops/method.hpp Tue Apr 08 11:50:01 2014 +0200
+++ b/hotspot/src/share/vm/oops/method.hpp Tue Apr 08 13:58:38 2014 -0400
@@ -123,6 +123,8 @@
void restore_vtable() { guarantee(is_method(), "vtable restored by this call"); }
bool is_method() const volatile { return true; }
+ void restore_unshareable_info(TRAPS);
+
// accessors for instance variables
ConstMethod* constMethod() const { return _constMethod; }