8384416: Crash in RunTimeClassInfo::klass() during AOT cache assembly

Reviewed-by: kvn, heidinga, coleenp

Author: iklam

src/hotspot/share/cds/cdsConfig.cpp

@@ -110,6 +110,19 @@ void CDSConfig::ergo_initialize() {
   AOTMapLogger::ergo_initialize();
 
   setup_compiler_args();
+
+  if (is_dumping_full_module_graph()) {
+    precond(allow_only_single_java_thread());
+
+    // The AttachListenerThread may execute Java code or load new classes. It might see
+    // unexpected results after HeapShared::prepare_for_archiving().
+    //
+    // We disable all new incoming attach requests, so you can't use jcmd, etc, on this JVM.
+    // Since we are not running any application code in this JVM and only executed a very
+    // limited set of Java code (for module system init, class loading, indy resolution,
+    // etc), there is usually no need to attach to this JVM.
+    FLAG_SET_ERGO(DisableAttachMechanism, true);
+  }
 }
 
 const char* CDSConfig::default_archive_path() {

src/hotspot/share/cds/lambdaProxyClassDictionary.cpp

@@ -92,6 +92,7 @@ void RunTimeLambdaProxyClassInfo::init(LambdaProxyClassKey& key, DumpTimeLambdaP
 }
 
 DumpTimeLambdaProxyClassDictionary* LambdaProxyClassDictionary::_dumptime_table = nullptr;
+LambdaProxyClassDictionary LambdaProxyClassDictionary::_runtime_table_for_dumping;
 LambdaProxyClassDictionary LambdaProxyClassDictionary::_runtime_static_table; // for static CDS archive
 LambdaProxyClassDictionary LambdaProxyClassDictionary::_runtime_dynamic_table; // for dynamic CDS archive
 
@@ -425,7 +426,7 @@ class CopyLambdaProxyClassInfoToArchive : StackObj {
 };
 
 void LambdaProxyClassDictionary::write_dictionary(bool is_static_archive) {
-  LambdaProxyClassDictionary* dictionary = is_static_archive ? &_runtime_static_table : &_runtime_dynamic_table;
+  LambdaProxyClassDictionary* dictionary = &_runtime_table_for_dumping;
   CompactHashtableStats stats;
   dictionary->reset();
   CompactHashtableWriter writer(_dumptime_table->_count, &stats);

src/hotspot/share/cds/lambdaProxyClassDictionary.hpp

@@ -269,6 +269,7 @@ class LambdaProxyClassDictionary : public OffsetCompactHashtable<
 private:
   class CleanupDumpTimeLambdaProxyClassTable;
   static DumpTimeLambdaProxyClassDictionary* _dumptime_table;
+  static LambdaProxyClassDictionary _runtime_table_for_dumping;
   static LambdaProxyClassDictionary _runtime_static_table; // for static CDS archive
   static LambdaProxyClassDictionary _runtime_dynamic_table; // for dynamic CDS archive
 
@@ -319,7 +320,9 @@ class LambdaProxyClassDictionary : public OffsetCompactHashtable<
   }
 
   static void serialize(SerializeClosure* soc, bool is_static_archive) {
-    if (is_static_archive) {
+    if (soc->writing()) {
+      _runtime_table_for_dumping.serialize_header(soc);
+    } else if (is_static_archive) {
       _runtime_static_table.serialize_header(soc);
     } else {
       _runtime_dynamic_table.serialize_header(soc);

src/hotspot/share/classfile/stringTable.cpp

@@ -86,7 +86,7 @@ typedef CompactHashtable<
   StringTable::read_string_from_compact_hashtable,
   StringTable::wrapped_string_equals> SharedStringTable;
 
-static SharedStringTable _shared_table;
+static SharedStringTable _shared_table, _shared_table_for_dumping;
 #endif
 
 // --------------------------------------------------------------------------
@@ -961,7 +961,7 @@ void StringTable::write_shared_table() {
   precond(CDSConfig::is_dumping_heap());
   assert(HeapShared::is_writing_mapping_mode(), "not used for streamed oops");
 
-  _shared_table.reset();
+  _shared_table_for_dumping.reset();
   CompactHashtableWriter writer((int)items_count_acquire(), ArchiveBuilder::string_stats());
 
   auto copy_into_shared_table = [&] (WeakHandle* val) {
@@ -974,17 +974,16 @@ void StringTable::write_shared_table() {
     return true;
   };
   _local_table->do_safepoint_scan(copy_into_shared_table);
-  writer.dump(&_shared_table, "string");
+  writer.dump(&_shared_table_for_dumping, "string");
 }
 
 void StringTable::serialize_shared_table_header(SerializeClosure* soc) {
-  _shared_table.serialize_header(soc);
+  SharedStringTable* table = soc->reading() ? &_shared_table : &_shared_table_for_dumping;
 
-  if (soc->writing()) {
-    // Sanity. Make sure we don't use the shared table at dump time
-    _shared_table.reset();
-  } else if (!AOTMappedHeapLoader::is_in_use()) {
-    _shared_table.reset();
+  table->serialize_header(soc);
+  if (soc->reading() && !AOTMappedHeapLoader::is_in_use()) {
+    // AOTStreamedHeapLoader does not use _shared_table.
+    table->reset();
   }
 }
 

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -79,8 +79,9 @@
 #include "utilities/hashTable.hpp"
 #include "utilities/stringUtils.hpp"
 
-SystemDictionaryShared::ArchiveInfo SystemDictionaryShared::_static_archive;
-SystemDictionaryShared::ArchiveInfo SystemDictionaryShared::_dynamic_archive;
+SystemDictionaryShared::ArchiveInfo SystemDictionaryShared::_info_for_static_archive;
+SystemDictionaryShared::ArchiveInfo SystemDictionaryShared::_info_for_dynamic_archive;
+SystemDictionaryShared::ArchiveInfo SystemDictionaryShared::_info_for_dumping;
 
 DumpTimeSharedClassTable* SystemDictionaryShared::_dumptime_table = nullptr;
 
@@ -132,8 +133,8 @@ InstanceKlass* SystemDictionaryShared::lookup_from_stream(Symbol* class_name,
     return nullptr;
   }
 
-  const RunTimeClassInfo* record = find_record(&_static_archive._unregistered_dictionary,
-                                               &_dynamic_archive._unregistered_dictionary,
+  const RunTimeClassInfo* record = find_record(&_info_for_static_archive._unregistered_dictionary,
+                                               &_info_for_dynamic_archive._unregistered_dictionary,
                                                class_name);
   if (record == nullptr) {
     return nullptr;
@@ -701,7 +702,7 @@ void SystemDictionaryShared::copy_unregistered_class_size_and_crc32(InstanceKlas
   precond(klass->in_aot_cache());
 
   // A shared class must have a RunTimeClassInfo record
-  const RunTimeClassInfo* record = find_record(&_static_archive._unregistered_dictionary,
+  const RunTimeClassInfo* record = find_record(&_info_for_static_archive._unregistered_dictionary,
                                                nullptr, klass->name());
   precond(record != nullptr);
   precond(record->klass() == klass);
@@ -1335,7 +1336,7 @@ void SystemDictionaryShared::write_dictionary(RunTimeSharedDictionary* dictionar
 }
 
 void SystemDictionaryShared::write_to_archive(bool is_static_archive) {
-  ArchiveInfo* archive = get_archive(is_static_archive);
+  ArchiveInfo* archive = get_archive(is_static_archive, /*is_dumping=*/true);
 
   write_dictionary(&archive->_builtin_dictionary, true);
   write_dictionary(&archive->_unregistered_dictionary, false);
@@ -1348,7 +1349,7 @@ void SystemDictionaryShared::write_to_archive(bool is_static_archive) {
 
 void SystemDictionaryShared::serialize_dictionary_headers(SerializeClosure* soc,
                                                           bool is_static_archive) {
-  ArchiveInfo* archive = get_archive(is_static_archive);
+  ArchiveInfo* archive = get_archive(is_static_archive, soc->writing());
 
   archive->_builtin_dictionary.serialize_header(soc);
   archive->_unregistered_dictionary.serialize_header(soc);
@@ -1395,8 +1396,8 @@ SystemDictionaryShared::find_record(RunTimeSharedDictionary* static_dict, RunTim
 }
 
 InstanceKlass* SystemDictionaryShared::find_builtin_class(Symbol* name) {
-  const RunTimeClassInfo* record = find_record(&_static_archive._builtin_dictionary,
-                                               &_dynamic_archive._builtin_dictionary,
+  const RunTimeClassInfo* record = find_record(&_info_for_static_archive._builtin_dictionary,
+                                               &_info_for_dynamic_archive._builtin_dictionary,
                                                name);
   if (record != nullptr) {
     assert(!record->klass()->is_hidden(), "hidden class cannot be looked up by name");
@@ -1437,11 +1438,12 @@ const char* SystemDictionaryShared::loader_type_for_shared_class(Klass* k) {
 }
 
 void SystemDictionaryShared::get_all_archived_classes(bool is_static_archive, GrowableArray<Klass*>* classes) {
-  get_archive(is_static_archive)->_builtin_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
+  ArchiveInfo* archive = get_archive(is_static_archive, /*is_dumping=*/false);
+  archive->_builtin_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
       classes->append(record->klass());
     });
 
-  get_archive(is_static_archive)->_unregistered_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
+  archive->_unregistered_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
       classes->append(record->klass());
     });
 }
@@ -1488,10 +1490,10 @@ void SystemDictionaryShared::ArchiveInfo::print_table_statistics(const char* pre
 void SystemDictionaryShared::print_shared_archive(outputStream* st, bool is_static) {
   if (CDSConfig::is_using_archive()) {
     if (is_static) {
-      _static_archive.print_on("", st, true);
+      _info_for_static_archive.print_on("", st, true);
     } else {
       if (DynamicArchive::is_mapped()) {
-        _dynamic_archive.print_on("Dynamic ", st, false);
+        _info_for_dynamic_archive.print_on("Dynamic ", st, false);
       }
     }
   }
@@ -1504,9 +1506,9 @@ void SystemDictionaryShared::print_on(outputStream* st) {
 
 void SystemDictionaryShared::print_table_statistics(outputStream* st) {
   if (CDSConfig::is_using_archive()) {
-    _static_archive.print_table_statistics("Static ", st, true);
+    _info_for_static_archive.print_table_statistics("Static ", st, true);
     if (DynamicArchive::is_mapped()) {
-      _dynamic_archive.print_table_statistics("Dynamic ", st, false);
+      _info_for_dynamic_archive.print_table_statistics("Dynamic ", st, false);
     }
   }
 }

src/hotspot/share/classfile/systemDictionaryShared.hpp

@@ -150,11 +150,16 @@ class SystemDictionaryShared: public SystemDictionary {
   class ExclusionCheckCandidates;
   static DumpTimeSharedClassTable* _dumptime_table;
 
-  static ArchiveInfo _static_archive;
-  static ArchiveInfo _dynamic_archive;
-
-  static ArchiveInfo* get_archive(bool is_static_archive) {
-    return is_static_archive ? &_static_archive : &_dynamic_archive;
+  static ArchiveInfo _info_for_static_archive;
+  static ArchiveInfo _info_for_dynamic_archive;
+  static ArchiveInfo _info_for_dumping;
+
+  static ArchiveInfo* get_archive(bool is_static_archive, bool is_dumping) {
+    if (is_dumping) {
+      return &_info_for_dumping;
+    } else {
+      return is_static_archive ? &_info_for_static_archive : &_info_for_dynamic_archive;
+    }
   }
 
   static InstanceKlass* load_shared_class_for_builtin_loader(