8358680: AOT cache creation fails: no strings should have been added

Co-authored-by: Aleksey Shipilev <shade@openjdk.org>
Reviewed-by: coleenp, shade

Author: iklam

src/hotspot/share/classfile/stringTable.cpp

@@ -32,6 +32,7 @@
 #include "classfile/javaClasses.inline.hpp"
 #include "classfile/stringTable.hpp"
 #include "classfile/vmClasses.hpp"
+#include "compiler/compileBroker.hpp"
 #include "gc/shared/collectedHeap.hpp"
 #include "gc/shared/oopStorage.inline.hpp"
 #include "gc/shared/oopStorageSet.hpp"
@@ -115,6 +116,7 @@ OopStorage*   StringTable::_oop_storage;
 
 static size_t _current_size = 0;
 static volatile size_t _items_count = 0;
+DEBUG_ONLY(static bool _disable_interning_during_cds_dump = false);
 
 volatile bool _alt_hash = false;
 
@@ -346,6 +348,10 @@ bool StringTable::has_work() {
   return Atomic::load_acquire(&_has_work);
 }
 
+size_t StringTable::items_count_acquire() {
+  return Atomic::load_acquire(&_items_count);
+}
+
 void StringTable::trigger_concurrent_work() {
   // Avoid churn on ServiceThread
   if (!has_work()) {
@@ -504,6 +510,9 @@ oop StringTable::intern(const char* utf8_string, TRAPS) {
 }
 
 oop StringTable::intern(const StringWrapper& name, TRAPS) {
+  assert(!Atomic::load_acquire(&_disable_interning_during_cds_dump),
+         "All threads that may intern strings should have been stopped before CDS starts copying the interned string table");
+
   // shared table always uses java_lang_String::hash_code
   unsigned int hash = hash_wrapped_string(name);
   oop found_string = lookup_shared(name, hash);
@@ -793,7 +802,7 @@ void StringTable::verify() {
 }
 
 // Verification and comp
-class VerifyCompStrings : StackObj {
+class StringTable::VerifyCompStrings : StackObj {
   static unsigned string_hash(oop const& str) {
     return java_lang_String::hash_code_noupdate(str);
   }
@@ -805,7 +814,7 @@ class VerifyCompStrings : StackObj {
                               string_hash, string_equals> _table;
  public:
   size_t _errors;
-  VerifyCompStrings() : _table(unsigned(_items_count / 8) + 1, 0 /* do not resize */), _errors(0) {}
+  VerifyCompStrings() : _table(unsigned(items_count_acquire() / 8) + 1, 0 /* do not resize */), _errors(0) {}
   bool operator()(WeakHandle* val) {
     oop s = val->resolve();
     if (s == nullptr) {
@@ -939,20 +948,31 @@ oop StringTable::lookup_shared(const jchar* name, int len) {
   return _shared_table.lookup(wrapped_name, java_lang_String::hash_code(name, len), 0);
 }
 
-// This is called BEFORE we enter the CDS safepoint. We can allocate heap objects.
-// This should be called when we know no more strings will be added (which will be easy
-// to guarantee because CDS runs with a single Java thread. See JDK-8253495.)
+// This is called BEFORE we enter the CDS safepoint. We can still allocate Java object arrays to
+// be used by the shared strings table.
 void StringTable::allocate_shared_strings_array(TRAPS) {
   if (!CDSConfig::is_dumping_heap()) {
     return;
   }
-  assert(CDSConfig::allow_only_single_java_thread(), "No more interned strings can be added");
 
-  if (_items_count > (size_t)max_jint) {
-    fatal("Too many strings to be archived: %zu", _items_count);
+  CompileBroker::wait_for_no_active_tasks();
+
+  precond(CDSConfig::allow_only_single_java_thread());
+
+  // At this point, no more strings will be added:
+  // - There's only a single Java thread (this thread). It no longer executes Java bytecodes
+  //   so JIT compilation will eventually stop.
+  // - CompileBroker has no more active tasks, so all JIT requests have been processed.
+
+  // This flag will be cleared after intern table dumping has completed, so we can run the
+  // compiler again (for future AOT method compilation, etc).
+  DEBUG_ONLY(Atomic::release_store(&_disable_interning_during_cds_dump, true));
+
+  if (items_count_acquire() > (size_t)max_jint) {
+    fatal("Too many strings to be archived: %zu", items_count_acquire());
   }
 
-  int total = (int)_items_count;
+  int total = (int)items_count_acquire();
   size_t single_array_size = objArrayOopDesc::object_size(total);
 
   log_info(aot)("allocated string table for %d strings", total);
@@ -972,7 +992,7 @@ void StringTable::allocate_shared_strings_array(TRAPS) {
       // This can only happen if you have an extremely large number of classes that
       // refer to more than 16384 * 16384 = 26M interned strings! Not a practical concern
       // but bail out for safety.
-      log_error(aot)("Too many strings to be archived: %zu", _items_count);
+      log_error(aot)("Too many strings to be archived: %zu", items_count_acquire());
       MetaspaceShared::unrecoverable_writing_error();
     }
 
@@ -1070,7 +1090,7 @@ oop StringTable::init_shared_strings_array() {
 
 void StringTable::write_shared_table() {
   _shared_table.reset();
-  CompactHashtableWriter writer((int)_items_count, ArchiveBuilder::string_stats());
+  CompactHashtableWriter writer((int)items_count_acquire(), ArchiveBuilder::string_stats());
 
   int index = 0;
   auto copy_into_shared_table = [&] (WeakHandle* val) {
@@ -1084,6 +1104,8 @@ void StringTable::write_shared_table() {
   };
   _local_table->do_safepoint_scan(copy_into_shared_table);
   writer.dump(&_shared_table, "string");
+
+  DEBUG_ONLY(Atomic::release_store(&_disable_interning_during_cds_dump, false));
 }
 
 void StringTable::set_shared_strings_array_index(int root_index) {

src/hotspot/share/classfile/stringTable.hpp

@@ -40,7 +40,7 @@ class StringTableConfig;
 
 class StringTable : AllStatic {
   friend class StringTableConfig;
-
+  class VerifyCompStrings;
   static volatile bool _has_work;
 
   // Set if one bucket is out of balance due to hash algorithm deficiency
@@ -74,6 +74,7 @@ class StringTable : AllStatic {
 
   static void item_added();
   static void item_removed();
+  static size_t items_count_acquire();
 
   static oop intern(const StringWrapper& name, TRAPS);
   static oop do_intern(const StringWrapper& name, uintx hash, TRAPS);

src/hotspot/share/compiler/compileBroker.cpp

@@ -1750,6 +1750,10 @@ void CompileBroker::wait_for_completion(CompileTask* task) {
   }
 }
 
+void CompileBroker::wait_for_no_active_tasks() {
+  CompileTask::wait_for_no_active_tasks();
+}
+
 /**
  * Initialize compiler thread(s) + compiler object(s). The postcondition
  * of this function is that the compiler runtimes are initialized and that

src/hotspot/share/compiler/compileBroker.hpp

@@ -383,6 +383,9 @@ class CompileBroker: AllStatic {
   static bool is_compilation_disabled_forever() {
     return _should_compile_new_jobs == shutdown_compilation;
   }
+
+  static void wait_for_no_active_tasks();
+
   static void handle_full_code_cache(CodeBlobType code_blob_type);
   // Ensures that warning is only printed once.
   static bool should_print_compiler_warning() {

src/hotspot/share/compiler/compileTask.cpp

@@ -37,12 +37,13 @@
 #include "runtime/mutexLocker.hpp"
 
 CompileTask*  CompileTask::_task_free_list = nullptr;
+int CompileTask::_active_tasks = 0;
 
 /**
  * Allocate a CompileTask, from the free list if possible.
  */
 CompileTask* CompileTask::allocate() {
-  MutexLocker locker(CompileTaskAlloc_lock);
+  MonitorLocker locker(CompileTaskAlloc_lock);
   CompileTask* task = nullptr;
 
   if (_task_free_list != nullptr) {
@@ -56,14 +57,15 @@ CompileTask* CompileTask::allocate() {
   }
   assert(task->is_free(), "Task must be free.");
   task->set_is_free(false);
+  _active_tasks++;
   return task;
 }
 
 /**
 * Add a task to the free list.
 */
 void CompileTask::free(CompileTask* task) {
-  MutexLocker locker(CompileTaskAlloc_lock);
+  MonitorLocker locker(CompileTaskAlloc_lock);
   if (!task->is_free()) {
     if ((task->_method_holder != nullptr && JNIHandles::is_weak_global_handle(task->_method_holder))) {
       JNIHandles::destroy_weak_global(task->_method_holder);
@@ -79,6 +81,17 @@ void CompileTask::free(CompileTask* task) {
     task->set_is_free(true);
     task->set_next(_task_free_list);
     _task_free_list = task;
+    _active_tasks--;
+    if (_active_tasks == 0) {
+      locker.notify_all();
+    }
+  }
+}
+
+void CompileTask::wait_for_no_active_tasks() {
+  MonitorLocker locker(CompileTaskAlloc_lock);
+  while (_active_tasks > 0) {
+    locker.wait();
   }
 }
 

src/hotspot/share/compiler/compileTask.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2025, 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
@@ -83,6 +83,7 @@ class CompileTask : public CHeapObj<mtCompiler> {
 
  private:
   static CompileTask*  _task_free_list;
+  static int           _active_tasks;
   int                  _compile_id;
   Method*              _method;
   jobject              _method_holder;
@@ -123,6 +124,7 @@ class CompileTask : public CHeapObj<mtCompiler> {
 
   static CompileTask* allocate();
   static void         free(CompileTask* task);
+  static void         wait_for_no_active_tasks();
 
   int          compile_id() const                { return _compile_id; }
   Method*      method() const                    { return _method; }

src/hotspot/share/runtime/mutexLocker.cpp

@@ -84,7 +84,7 @@ Monitor* CompileTaskWait_lock         = nullptr;
 Monitor* MethodCompileQueue_lock      = nullptr;
 Monitor* CompileThread_lock           = nullptr;
 Monitor* Compilation_lock             = nullptr;
-Mutex*   CompileTaskAlloc_lock        = nullptr;
+Monitor* CompileTaskAlloc_lock        = nullptr;
 Mutex*   CompileStatistics_lock       = nullptr;
 Mutex*   DirectivesStack_lock         = nullptr;
 Monitor* Terminator_lock              = nullptr;
@@ -346,7 +346,7 @@ void mutex_init() {
     MUTEX_DEFL(G1RareEvent_lock             , PaddedMutex  , Threads_lock, true);
   }
 
-  MUTEX_DEFL(CompileTaskAlloc_lock          , PaddedMutex  , MethodCompileQueue_lock);
+  MUTEX_DEFL(CompileTaskAlloc_lock          , PaddedMonitor, MethodCompileQueue_lock);
   MUTEX_DEFL(CompileTaskWait_lock           , PaddedMonitor, MethodCompileQueue_lock);
 
 #if INCLUDE_PARALLELGC

src/hotspot/share/runtime/mutexLocker.hpp

@@ -86,7 +86,7 @@ extern Monitor* CompileThread_lock;              // a lock held by compile threa
 extern Monitor* Compilation_lock;                // a lock used to pause compilation
 extern Mutex*   TrainingData_lock;               // a lock used when accessing training records
 extern Monitor* TrainingReplayQueue_lock;        // a lock held when class are added/removed to the training replay queue
-extern Mutex*   CompileTaskAlloc_lock;           // a lock held when CompileTasks are allocated
+extern Monitor* CompileTaskAlloc_lock;           // a lock held when CompileTasks are allocated
 extern Monitor* CompileTaskWait_lock;            // a lock held when CompileTasks are waited/notified
 extern Mutex*   CompileStatistics_lock;          // a lock held when updating compilation statistics
 extern Mutex*   DirectivesStack_lock;            // a lock held when mutating the dirstack and ref counting directives