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

hotspot/src/share/vm/classfile/classFileParser.cpp

@@ -4180,8 +4180,12 @@ ClassFileParser::~ClassFileParser() {
 
   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;
 }
 

hotspot/src/share/vm/classfile/javaClasses.cpp

 /*
- * 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 @@ void java_lang_String::print(oop java_string, outputStream* st) {
   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 @@ void java_lang_Class::fixup_mirror(KlassHandle k, TRAPS) {
   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 @@ oop java_lang_Class::create_mirror(KlassHandle k, Handle protection_domain, TRAP
         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_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;
+      }
+    }
 
-      // Initialize static fields
-      InstanceKlass::cast(k())->do_local_static_fields(&initialize_static_field, CHECK_NULL);
+    // Setup indirection from klass->mirror last
+    // after any exceptions can happen during allocations.
+    if (!k.is_null()) {
+      k->set_java_mirror(mirror());
     }
-    return mirror();
   } else {
     if (fixup_mirror_list() == NULL) {
       GrowableArray<Klass*>* list =
@@ -607,12 +634,10 @@ oop java_lang_Class::create_mirror(KlassHandle k, Handle protection_domain, TRAP
       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);

hotspot/src/share/vm/classfile/javaClasses.hpp

@@ -246,11 +246,12 @@ class java_lang_Class : AllStatic {
 
   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

hotspot/src/share/vm/classfile/systemDictionary.cpp

@@ -826,47 +826,6 @@ Klass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
       }
     } // 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 @@ instanceKlassHandle SystemDictionary::load_shared_class(
   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

hotspot/src/share/vm/classfile/systemDictionary.hpp

@@ -617,7 +617,6 @@ private:
                                                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);

hotspot/src/share/vm/gc_interface/collectedHeap.cpp

 /*
- * 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 @@ void CollectedHeap::post_full_gc_dump(GCTimer* timer) {
   }
 }
 
-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

hotspot/src/share/vm/gc_interface/collectedHeap.hpp

 /*
- * 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 @@ class CollectedHeap : public CHeapObj<mtInternal> {
   // 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);

hotspot/src/share/vm/oops/constantPool.cpp

@@ -144,6 +144,10 @@ void ConstantPool::initialize_resolved_references(ClassLoaderData* loader_data,
 // 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();
 

hotspot/src/share/vm/oops/instanceKlass.cpp

@@ -1289,17 +1289,18 @@ void InstanceKlass::do_local_static_fields(FieldClosure* cl) {
 }
 
 
-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 @@ void InstanceKlass::restore_unshareable_info(TRAPS) {
   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

hotspot/src/share/vm/oops/instanceKlass.hpp

@@ -802,7 +802,7 @@ class InstanceKlass: public Klass {
   // 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 @@ private:
   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);
 

hotspot/src/share/vm/oops/instanceMirrorKlass.cpp

 /*
- * 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 @@ instanceOop InstanceMirrorKlass::allocate_instance(KlassHandle k, TRAPS) {
   // 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;
 }
 

hotspot/src/share/vm/oops/klass.cpp

@@ -475,12 +475,8 @@ void Klass::oops_do(OopClosure* cl) {
 }
 
 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::remove_unshareable_info() {
 }
 
 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);
-
-  // Add to null class loader list first before creating the mirror
-  // (same order as class file parsing)
-  loader_data->add_class(this);
+  // 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);
+  }
 
   // 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) {

hotspot/src/share/vm/oops/method.cpp

@@ -903,6 +903,19 @@ address Method::make_adapters(methodHandle mh, TRAPS) {
   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.
 

hotspot/src/share/vm/oops/method.hpp

@@ -123,6 +123,8 @@ class Method : public Metadata {
   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; }