8193332: MetaspaceShared::check_shared_class_loader_type is not used during archive creation
authoriklam
Mon, 21 May 2018 21:27:12 -0700
changeset 50206 adec398d9051
parent 50205 95ba3a1dc2b2
child 50207 24b5f2e635f6
8193332: MetaspaceShared::check_shared_class_loader_type is not used during archive creation Reviewed-by: lfoltan, jiangli
src/hotspot/share/classfile/classListParser.cpp
src/hotspot/share/classfile/classLoader.cpp
src/hotspot/share/memory/metaspaceShared.cpp
src/hotspot/share/memory/metaspaceShared.hpp
src/hotspot/share/oops/instanceKlass.hpp
test/hotspot/jtreg/runtime/SharedArchiveFile/BootAppendTests.java
--- a/src/hotspot/share/classfile/classListParser.cpp	Mon May 21 18:44:09 2018 -0700
+++ b/src/hotspot/share/classfile/classListParser.cpp	Mon May 21 21:27:12 2018 -0700
@@ -311,6 +311,7 @@
 
     // This tells JVM_FindLoadedClass to not find this class.
     k->set_shared_classpath_index(UNREGISTERED_INDEX);
+    k->clear_class_loader_type();
   }
 
   return k;
--- a/src/hotspot/share/classfile/classLoader.cpp	Mon May 21 18:44:09 2018 -0700
+++ b/src/hotspot/share/classfile/classLoader.cpp	Mon May 21 21:27:12 2018 -0700
@@ -1410,16 +1410,12 @@
   s2 classpath_index = 0;
   ClassPathEntry* e = NULL;
 
-  // If DumpSharedSpaces is true boot loader visibility boundaries are set to:
-  //   - [jimage] + [_first_append_entry to _last_append_entry] (all path entries).
-  //
   // If search_append_only is true, boot loader visibility boundaries are
   // set to be _first_append_entry to the end. This includes:
   //   [-Xbootclasspath/a]; [jvmti appended entries]
   //
-  // If both DumpSharedSpaces and search_append_only are false, boot loader
-  // visibility boundaries are set to be the --patch-module entries plus the base piece.
-  // This would include:
+  // If search_append_only is false, boot loader visibility boundaries are
+  // set to be the --patch-module entries plus the base piece. This includes:
   //   [--patch-module=<module>=<file>(<pathsep><file>)*]; [jimage | exploded module build]
   //
 
@@ -1455,7 +1451,7 @@
   }
 
   // Load Attempt #3: [-Xbootclasspath/a]; [jvmti appended entries]
-  if ((search_append_only || DumpSharedSpaces) && (NULL == stream)) {
+  if (search_append_only && (NULL == stream)) {
     // For the boot loader append path search, the starting classpath_index
     // for the appended piece is always 1 to account for either the
     // _jrt_entry or the _exploded_entries.
--- a/src/hotspot/share/memory/metaspaceShared.cpp	Mon May 21 18:44:09 2018 -0700
+++ b/src/hotspot/share/memory/metaspaceShared.cpp	Mon May 21 21:27:12 2018 -0700
@@ -1411,6 +1411,9 @@
   // Move classes from platform/system dictionaries into the boot dictionary
   SystemDictionary::combine_shared_dictionaries();
 
+  // Make sure all classes have a correct loader type.
+  ClassLoaderData::the_null_class_loader_data()->dictionary()->classes_do(MetaspaceShared::check_shared_class_loader_type);
+
   // Remove all references outside the metadata
   tty->print("Removing unshareable information ... ");
   remove_unshareable_in_classes();
@@ -1610,13 +1613,14 @@
   }
 };
 
-void MetaspaceShared::check_shared_class_loader_type(Klass* k) {
-  if (k->is_instance_klass()) {
-    InstanceKlass* ik = InstanceKlass::cast(k);
-    u2 loader_type = ik->loader_type();
-    ResourceMark rm;
-    guarantee(loader_type != 0,
-              "Class loader type is not set for this class %s", ik->name()->as_C_string());
+void MetaspaceShared::check_shared_class_loader_type(InstanceKlass* ik) {
+  ResourceMark rm;
+  if (ik->shared_classpath_index() == UNREGISTERED_INDEX) {
+    guarantee(ik->loader_type() == 0,
+            "Class loader type must not be set for this class %s", ik->name()->as_C_string());
+  } else {
+    guarantee(ik->loader_type() != 0,
+            "Class loader type must be set for this class %s", ik->name()->as_C_string());
   }
 }
 
--- a/src/hotspot/share/memory/metaspaceShared.hpp	Mon May 21 18:44:09 2018 -0700
+++ b/src/hotspot/share/memory/metaspaceShared.hpp	Mon May 21 21:27:12 2018 -0700
@@ -225,7 +225,7 @@
 
   static bool try_link_class(InstanceKlass* ik, TRAPS);
   static void link_and_cleanup_shared_classes(TRAPS);
-  static void check_shared_class_loader_type(Klass* obj);
+  static void check_shared_class_loader_type(InstanceKlass* ik);
 
   // Allocate a block of memory from the "mc", "ro", or "rw" regions.
   static char* misc_code_space_alloc(size_t num_bytes);
--- a/src/hotspot/share/oops/instanceKlass.hpp	Mon May 21 18:44:09 2018 -0700
+++ b/src/hotspot/share/oops/instanceKlass.hpp	Mon May 21 21:27:12 2018 -0700
@@ -326,6 +326,10 @@
     return (_misc_flags & _misc_is_shared_app_class) != 0;
   }
 
+  void clear_class_loader_type() {
+    _misc_flags &= ~loader_type_bits();
+  }
+
   void set_class_loader_type(s2 loader_type) {
     switch (loader_type) {
     case ClassLoader::BOOT_LOADER:
--- a/test/hotspot/jtreg/runtime/SharedArchiveFile/BootAppendTests.java	Mon May 21 18:44:09 2018 -0700
+++ b/test/hotspot/jtreg/runtime/SharedArchiveFile/BootAppendTests.java	Mon May 21 21:27:12 2018 -0700
@@ -115,7 +115,15 @@
                                  "-XX:SharedClassListFile=" + classlist.getPath());
         // Make sure all the classes were successfully archived.
         for (String archiveClass : ARCHIVE_CLASSES) {
-            out.shouldNotContain("Preload Warning: Cannot find " + archiveClass);
+            String msg = "Preload Warning: Cannot find " + archiveClass;
+            if (archiveClass.equals(BOOT_APPEND_MODULE_CLASS)) {
+                // We shouldn't archive a class in the appended boot class path that
+                // are the java.desktop module. Such a class cannot be loaded
+                // at runtime anyway.
+                out.shouldContain(msg);
+            } else {
+                out.shouldNotContain(msg);
+            }
         }
     }