8152949: Jigsaw crash when Klass in _fixup_module_field_list is unloaded
authorlfoltan
Thu, 21 Apr 2016 09:23:04 -0400
changeset 38014 8731fa11f766
parent 38013 89b93eb018fb
child 38015 8dd94eb5dcfc
child 38057 1eba14626850
8152949: Jigsaw crash when Klass in _fixup_module_field_list is unloaded Summary: During start up anonymous classes must be kept alive until after their mirror's module field is patched with java.base Reviewed-by: coleenp, hseigel Contributed-by: per.liden@oracle.com, stefan.karlsson@oracle.com
hotspot/src/share/vm/classfile/classLoaderData.cpp
hotspot/src/share/vm/classfile/classLoaderData.hpp
hotspot/src/share/vm/classfile/javaClasses.cpp
hotspot/src/share/vm/classfile/moduleEntry.cpp
hotspot/src/share/vm/classfile/modules.cpp
hotspot/src/share/vm/prims/unsafe.cpp
--- a/hotspot/src/share/vm/classfile/classLoaderData.cpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp	Thu Apr 21 09:23:04 2016 -0400
@@ -84,7 +84,7 @@
   // An anonymous class loader data doesn't have anything to keep
   // it from being unloaded during parsing of the anonymous class.
   // The null-class-loader should always be kept alive.
-  _keep_alive(is_anonymous || h_class_loader.is_null()),
+  _keep_alive((is_anonymous || h_class_loader.is_null()) ? 1 : 0),
   _metaspace(NULL), _unloading(false), _klasses(NULL),
   _modules(NULL), _packages(NULL),
   _claimed(0), _jmethod_ids(NULL), _handles(NULL), _deallocate_list(NULL),
@@ -114,6 +114,21 @@
   return (int) Atomic::cmpxchg(1, &_claimed, 0) == 0;
 }
 
+// Anonymous classes have their own ClassLoaderData that is marked to keep alive
+// while the class is being parsed, and if the class appears on the module fixup list.
+// Due to the uniqueness that no other class shares the anonymous class' name or
+// ClassLoaderData, no other non-GC thread has knowledge of the anonymous class while
+// it is being defined, therefore _keep_alive is not volatile or atomic.
+void ClassLoaderData::inc_keep_alive() {
+  assert(_keep_alive >= 0, "Invalid keep alive count");
+  _keep_alive++;
+}
+
+void ClassLoaderData::dec_keep_alive() {
+  assert(_keep_alive > 0, "Invalid keep alive count");
+  _keep_alive--;
+}
+
 void ClassLoaderData::oops_do(OopClosure* f, KlassClosure* klass_closure, bool must_claim) {
   if (must_claim && !claim()) {
     return;
--- a/hotspot/src/share/vm/classfile/classLoaderData.hpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classLoaderData.hpp	Thu Apr 21 09:23:04 2016 -0400
@@ -175,8 +175,11 @@
                            // classes in the class loader are allocated.
   Mutex* _metaspace_lock;  // Locks the metaspace for allocations and setup.
   bool _unloading;         // true if this class loader goes away
-  bool _keep_alive;        // if this CLD is kept alive without a keep_alive_object().
   bool _is_anonymous;      // if this CLD is for an anonymous class
+  int _keep_alive;         // if this CLD is kept alive without a keep_alive_object().
+                           // Currently used solely for anonymous classes.
+                           // _keep_alive does not need to be volatile or
+                           // atomic since there is one unique CLD per anonymous class.
   volatile int _claimed;   // true if claimed, for example during GC traces.
                            // To avoid applying oop closure more than once.
                            // Has to be an int because we cas it.
@@ -224,7 +227,7 @@
   bool claim();
 
   void unload();
-  bool keep_alive() const       { return _keep_alive; }
+  bool keep_alive() const       { return _keep_alive > 0; }
   void classes_do(void f(Klass*));
   void loaded_classes_do(KlassClosure* klass_closure);
   void classes_do(void f(InstanceKlass*));
@@ -286,8 +289,8 @@
     return _unloading;
   }
 
-  // Used to make sure that this CLD is not unloaded.
-  void set_keep_alive(bool value) { _keep_alive = value; }
+  void inc_keep_alive();
+  void dec_keep_alive();
 
   inline unsigned int identity_hash() const;
 
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/classfile/javaClasses.cpp	Thu Apr 21 09:23:04 2016 -0400
@@ -852,6 +852,7 @@
           new (ResourceObj::C_HEAP, mtClass) GrowableArray<Klass*>(500, true);
         set_fixup_module_field_list(list);
       }
+      k->class_loader_data()->inc_keep_alive();
       fixup_module_field_list()->push(k());
     }
   } else {
--- a/hotspot/src/share/vm/classfile/moduleEntry.cpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/classfile/moduleEntry.cpp	Thu Apr 21 09:23:04 2016 -0400
@@ -354,6 +354,7 @@
     Thread* THREAD = Thread::current();
     KlassHandle kh(THREAD, k);
     java_lang_Class::fixup_module_field(kh, module_handle);
+    k->class_loader_data()->dec_keep_alive();
   }
 
   delete java_lang_Class::fixup_module_field_list();
--- a/hotspot/src/share/vm/classfile/modules.cpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/classfile/modules.cpp	Thu Apr 21 09:23:04 2016 -0400
@@ -277,6 +277,11 @@
   {
     MutexLocker m1(Module_lock, THREAD);
 
+    if (ModuleEntryTable::javabase_defined()) {
+      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+                "Module java.base is already defined");
+    }
+
     // Verify that all java.base packages created during bootstrapping are in
     // pkg_list.  If any are not in pkg_list, than a non-java.base class was
     // loaded erroneously pre java.base module definition.
--- a/hotspot/src/share/vm/prims/unsafe.cpp	Tue Apr 19 14:53:33 2016 +0200
+++ b/hotspot/src/share/vm/prims/unsafe.cpp	Thu Apr 21 09:23:04 2016 -0400
@@ -934,7 +934,7 @@
   // this point.   The mirror and any instances of this class have to keep
   // it alive afterwards.
   if (anon_klass() != NULL) {
-    anon_klass->class_loader_data()->set_keep_alive(false);
+    anon_klass->class_loader_data()->dec_keep_alive();
   }
 
   // let caller initialize it as needed...