8379819: Creating AOT configuration crashes in MethodTrainingData::prepare

Reviewed-by: iklam, kvn

Author: Igor Veresov

src/hotspot/share/oops/methodData.cpp

@@ -329,7 +329,7 @@ static bool is_excluded(Klass* k) {
       log_debug(aot, training)("Purged %s from MDO: unloaded class", k->name()->as_C_string());
       return true;
     } else {
-      bool excluded = SystemDictionaryShared::should_be_excluded(k);
+      bool excluded = SystemDictionaryShared::should_be_excluded(k) || !SystemDictionaryShared::is_builtin_loader(k->class_loader_data());
       if (excluded) {
         log_debug(aot, training)("Purged %s from MDO: excluded class", k->name()->as_C_string());
       }

src/hotspot/share/oops/trainingData.cpp

@@ -118,10 +118,23 @@ void TrainingData::verify() {
   }
 }
 
+static bool is_excluded(InstanceKlass* k) {
+  if (!k->is_loaded() || k->has_been_redefined()) {
+    return true;
+  }
+  if (CDSConfig::is_at_aot_safepoint()) {
+    // Check for AOT exclusion only at AOT safe point.
+    return SystemDictionaryShared::should_be_excluded(k) || !SystemDictionaryShared::is_builtin_loader(k->class_loader_data());
+  }
+  return false;
+}
+
 MethodTrainingData* MethodTrainingData::make(const methodHandle& method, bool null_if_not_found, bool use_cache) {
-  MethodTrainingData* mtd = nullptr;
   if (!have_data() && !need_data()) {
-    return mtd;
+    return nullptr;
+  }
+  if (is_excluded(method->method_holder())) {
+    return nullptr;
   }
   // Try grabbing the cached value first.
   // Cache value is stored in MethodCounters and the following are the
@@ -133,6 +146,7 @@ MethodTrainingData* MethodTrainingData::make(const methodHandle& method, bool nu
   //    i.e. null_if_no_found == true, then just return a null.
   // 3. Cache value is not null.
   //    Return it, the value of training_data_lookup_failed doesn't matter.
+  MethodTrainingData* mtd = nullptr;
   MethodCounters* mcs = method->method_counters();
   if (mcs != nullptr) {
     mtd = mcs->method_training_data();
@@ -175,6 +189,7 @@ MethodTrainingData* MethodTrainingData::make(const methodHandle& method, bool nu
           return nullptr; // allocation failure
         }
         td = training_data_set()->install(mtd);
+        assert(!is_excluded(method->method_holder()), "Should not be excluded");
         assert(td == mtd, "");
       } else {
         mtd = nullptr;
@@ -376,6 +391,9 @@ void CompileTrainingData::prepare(Visitor& visitor) {
 }
 
 KlassTrainingData* KlassTrainingData::make(InstanceKlass* holder, bool null_if_not_found) {
+  if (is_excluded(holder)) {
+    return nullptr;
+  }
   Key key(holder);
   TrainingData* td = CDS_ONLY(have_data() ? lookup_archived_training_data(&key) :) nullptr;
   KlassTrainingData* ktd = nullptr;
@@ -401,6 +419,7 @@ KlassTrainingData* KlassTrainingData::make(InstanceKlass* holder, bool null_if_n
       }
       td = training_data_set()->install(ktd);
       assert(ktd == td, "");
+      assert(!is_excluded(holder), "Should not be excluded");
     } else {
       ktd = td->as_KlassTrainingData();
       guarantee(ktd->holder() != nullptr, "null holder");
@@ -543,18 +562,24 @@ void TrainingData::cleanup_training_data() {
   }
 }
 
+void TrainingData::cleanup_after_redefinition() {
+  if (need_data()) {
+    TrainingDataLocker l;
+    ResourceMark rm;
+    Visitor visitor(training_data_set()->size());
+    training_data_set()->iterate([&](TrainingData* td) {
+      td->cleanup(visitor);
+    });
+  }
+}
+
 void KlassTrainingData::cleanup(Visitor& visitor) {
   if (visitor.is_visited(this)) {
     return;
   }
   visitor.visit(this);
   if (has_holder()) {
-    bool is_excluded = !holder()->is_loaded();
-    if (CDSConfig::is_at_aot_safepoint()) {
-      // Check for AOT exclusion only at AOT safe point.
-      is_excluded |= SystemDictionaryShared::should_be_excluded(holder());
-    }
-    if (is_excluded) {
+    if (is_excluded(holder())) {
       ResourceMark rm;
       log_debug(aot, training)("Cleanup KTD %s", name()->as_klass_external_name());
       _holder = nullptr;
@@ -572,12 +597,8 @@ void MethodTrainingData::cleanup(Visitor& visitor) {
   }
   visitor.visit(this);
   if (has_holder()) {
-    if (CDSConfig::is_at_aot_safepoint() && SystemDictionaryShared::should_be_excluded(holder()->method_holder())) {
-      // Check for AOT exclusion only at AOT safe point.
+    if (is_excluded(holder()->method_holder())) {
       log_debug(aot, training)("Cleanup MTD %s::%s", name()->as_klass_external_name(), signature()->as_utf8());
-      if (_final_profile != nullptr && _final_profile->method() != _holder) {
-        log_warning(aot, training)("Stale MDO for  %s::%s", name()->as_klass_external_name(), signature()->as_utf8());
-      }
       _final_profile = nullptr;
       _final_counters = nullptr;
       _holder = nullptr;
@@ -593,6 +614,7 @@ void MethodTrainingData::cleanup(Visitor& visitor) {
 }
 
 void KlassTrainingData::verify() {
+  guarantee(!has_holder() || !is_excluded(holder()), "Bad holder");
   for (int i = 0; i < comp_dep_count(); i++) {
     CompileTrainingData* ctd = comp_dep(i);
     if (!ctd->_init_deps.contains(this)) {
@@ -604,6 +626,7 @@ void KlassTrainingData::verify() {
 }
 
 void MethodTrainingData::verify(bool verify_dep_counter) {
+  guarantee(!has_holder() || !is_excluded(holder()->method_holder()), "Bad holder");
   iterate_compiles([&](CompileTrainingData* ctd) {
     ctd->verify(verify_dep_counter);
   });

src/hotspot/share/oops/trainingData.hpp

@@ -310,7 +310,7 @@ class TrainingData : public Metadata {
   static void iterate(Function& fn) { // lambda enabled API
     TrainingDataLocker l;
     if (have_data()) {
-      archived_training_data_dictionary()->iterate(fn);
+      archived_training_data_dictionary()->iterate_all(fn);
     }
     if (need_data()) {
       training_data_set()->iterate(fn);
@@ -431,6 +431,8 @@ class TrainingData : public Metadata {
     }
     return nullptr;
   }
+
+  static void cleanup_after_redefinition();
 };
 
 // Training data that is associated with an InstanceKlass

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -53,6 +53,7 @@
 #include "oops/method.hpp"
 #include "oops/oop.inline.hpp"
 #include "oops/recordComponent.hpp"
+#include "oops/trainingData.hpp"
 #include "prims/jvmtiImpl.hpp"
 #include "prims/jvmtiRedefineClasses.hpp"
 #include "prims/jvmtiThreadState.inline.hpp"
@@ -274,6 +275,10 @@ void VM_RedefineClasses::doit() {
     redefine_single_class(current, _class_defs[i].klass, _scratch_classes[i]);
   }
 
+#if INCLUDE_CDS
+  TrainingData::cleanup_after_redefinition();
+#endif
+
   // Flush all compiled code that depends on the classes redefined.
   flush_dependent_code();
 

test/hotspot/jtreg/runtime/cds/appcds/aotCache/RedefineClassesInProfile.java

@@ -41,14 +41,19 @@
  * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar redef1.jar
  *             RedefFoo RedefBar
  *             RedefTaz0 RedefTaz1 RedefTaz2 RedefTaz3 RedefTaz4
-
+ *
+ * @compile test-classes/CustomLoadee.java
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar cust.jar CustomLoadee
+ *
  * @run driver RedefineClassHelper
  * @build RedefineClassesInProfile
  * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar RedefineClassesInProfileApp Util
  * @run driver RedefineClassesInProfile
  */
 
 import java.io.File;
+import java.net.URL;
+import java.net.URLClassLoader;
 import jdk.test.lib.cds.SimpleCDSAppTester;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
@@ -89,6 +94,7 @@ public static void main(String... args) throws Exception {
                     out.shouldNotMatch(prefix + "RedefFoo");
                     out.shouldNotMatch(prefix + "RedefBar");
                     out.shouldNotMatch(prefix + "RedefTaz");
+                    out.shouldNotMatch(prefix + "CustomLoadee");
                 })
             .setProductionChecker((OutputAnalyzer out) -> {
                     out.shouldContain("Redefined: class RedefBar");
@@ -104,6 +110,7 @@ class RedefineClassesInProfileApp {
 
     public static void main(String[] args) throws Exception {
         test1();
+        test2();
     }
 
     // test1
@@ -175,4 +182,30 @@ static void redefine(String jarFile, Class c) {
             throw new RuntimeException("Unexpected failure", t);
         }
     }
+
+    // Test 2 -- TrainingData interaction with custom class loaders.
+    static void test2() throws Exception {
+        // Do this several times. The AOT cache should contain only one
+        // copy of CustomLoadee as an "unregistered" class, which will be
+        // used in the first iteration of this loop.
+        //
+        // The JVM should work well even if the cached version of CustomLoadee
+        // has been unloaded.
+        for (int i = 0; i < 4; i++) {
+            test2_inner();
+            System.gc(); // trigger unloading of CustomLoadee.
+        }
+    }
+
+    static void test2_inner() throws Exception {
+        // Load a class and run a loop to make sure it's compiled, but
+        // TrainingData should not record any class/method that are loaded
+        // by custom class loaders
+        File custJar = new File("cust.jar");
+        URL[] urls = new URL[] {custJar.toURI().toURL()};
+        URLClassLoader loader = new URLClassLoader(urls, RedefineClassesInProfileApp.class.getClassLoader());
+        Class<?> c = loader.loadClass("CustomLoadee");
+        System.out.println(c.newInstance());
+    }
 }
+

test/hotspot/jtreg/runtime/cds/appcds/aotCache/test-classes/CustomLoadee.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2026, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+public class CustomLoadee {
+    public CustomLoadee() {
+        hotspot2();
+    }
+
+    static volatile int x;
+    static void hotspot2() {
+        long start = System.currentTimeMillis();
+        // run this loop long enough (400ms) for it to be JIT compiled.
+        while (System.currentTimeMillis() - start < 400) {
+            x += 1;
+        }
+    }
+}