8368727: CDS custom loader support causes asserts during class unloading

Reviewed-by: coleenp, dholmes

Author: iklam

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -174,7 +174,6 @@ InstanceKlass* SystemDictionaryShared::acquire_class_for_current_thread(
 
   // No longer holding SharedDictionary_lock
   // No need to lock, as <ik> can be held only by a single thread.
-  loader_data->add_class(ik);
 
   // Get the package entry.
   PackageEntry* pkg_entry = CDSProtectionDomain::get_package_entry_from_class(ik, class_loader);

src/hotspot/share/oops/klass.cpp

@@ -872,11 +872,10 @@ void Klass::restore_unshareable_info(ClassLoaderData* loader_data, Handle protec
   // modify the CLD list outside a safepoint.
   if (class_loader_data() == nullptr) {
     set_class_loader_data(loader_data);
-
-    // Add to class loader list first before creating the mirror
-    // (same order as class file parsing)
-    loader_data->add_class(this);
   }
+  // Add to class loader list first before creating the mirror
+  // (same order as class file parsing)
+  loader_data->add_class(this);
 
   Handle loader(THREAD, loader_data->class_loader());
   ModuleEntry* module_entry = nullptr;

test/hotspot/jtreg/runtime/cds/appcds/customLoader/UnloadUnregisteredLoaderTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 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
@@ -29,9 +29,10 @@
  * @requires vm.cds
  * @requires vm.cds.custom.loaders
  * @requires vm.opt.final.ClassUnloading
- * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds /test/hotspot/jtreg/runtime/cds/appcds/test-classes
  * @build jdk.test.whitebox.WhiteBox jdk.test.lib.classloader.ClassUnloadCommon
  * @compile test-classes/UnloadUnregisteredLoader.java test-classes/CustomLoadee.java
+ *          test-classes/CustomLoadee5.java test-classes/CustomLoadee5Child.java
  * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
  *                 jdk.test.lib.classloader.ClassUnloadCommon
  *                 jdk.test.lib.classloader.ClassUnloadCommon$1
@@ -49,12 +50,19 @@ public class UnloadUnregisteredLoaderTest {
         CDSOptions.disableRuntimePrefixForEpsilonGC();
     }
     public static void main(String[] args) throws Exception {
-        String appJar1 = JarBuilder.build("UnloadUnregisteredLoader_app1", "UnloadUnregisteredLoader");
+        String appJar1 = JarBuilder.build("UnloadUnregisteredLoader_app1",
+                                          "UnloadUnregisteredLoader",
+                                          "UnloadUnregisteredLoader$CustomLoader",
+                                          "CustomLoadee5",
+                                          "Util");
         String appJar2 = JarBuilder.build(true, "UnloadUnregisteredLoader_app2",
                                           "jdk/test/lib/classloader/ClassUnloadCommon",
                                           "jdk/test/lib/classloader/ClassUnloadCommon$1",
                                           "jdk/test/lib/classloader/ClassUnloadCommon$TestFailure");
-        String customJarPath = JarBuilder.build("UnloadUnregisteredLoader_custom", "CustomLoadee");
+        String customJarPath = JarBuilder.build("UnloadUnregisteredLoader_custom",
+                                                "CustomLoadee",
+                                                "CustomLoadee5",
+                                                "CustomLoadee5Child");
         String wbJar = JarBuilder.build(true, "WhiteBox", "jdk/test/whitebox/WhiteBox");
         String use_whitebox_jar = "-Xbootclasspath/a:" + wbJar;
 
@@ -66,6 +74,8 @@ public static void main(String[] args) throws Exception {
             "jdk/test/lib/classloader/ClassUnloadCommon$TestFailure",
             "java/lang/Object id: 1",
             "CustomLoadee id: 2 super: 1 source: " + customJarPath,
+            "CustomLoadee5 id: 3 super: 1 source: " + customJarPath,
+            "CustomLoadee5Child id: 4 super: 3 source: " + customJarPath,
         };
 
         OutputAnalyzer output;

test/hotspot/jtreg/runtime/cds/appcds/customLoader/test-classes/CustomLoadee5.java

@@ -21,7 +21,7 @@
  * questions.
  */
 
-class CustomLoadee5 {
+public class CustomLoadee5 {
     public String toString() {
         return "this is CustomLoadee5";
     }

test/hotspot/jtreg/runtime/cds/appcds/customLoader/test-classes/UnloadUnregisteredLoader.java

@@ -23,6 +23,7 @@
  */
 
 import java.io.File;
+import java.io.InputStream;
 import java.net.URL;
 import java.net.URLClassLoader;
 import jdk.test.whitebox.WhiteBox;
@@ -46,9 +47,11 @@ public static void main(String args[]) throws Exception {
         }
     }
 
-  public static void doit(URL urls[], String className, boolean isFirstTime) throws Exception {
+    public static void doit(URL urls[], String className, boolean isFirstTime) throws Exception {
         ClassLoader appLoader = UnloadUnregisteredLoader.class.getClassLoader();
-        URLClassLoader custLoader = new URLClassLoader(urls, appLoader);
+        CustomLoader custLoader = new CustomLoader(urls, appLoader);
+
+        // Part 1 -- load CustomLoadee. It should be loaded from archive when isFirstTime==true
 
         Class klass = custLoader.loadClass(className);
         WhiteBox wb = WhiteBox.getWhiteBox();
@@ -68,5 +71,43 @@ public static void doit(URL urls[], String className, boolean isFirstTime) throw
                 }
             }
         }
+
+        // Part 2
+        //
+        // CustomLoadee5 is never loaded from the archive, because the classfile bytes don't match
+        // CustomLoadee5Child is never loaded from the archive, its super is not loaded from the archive
+        try (InputStream in = appLoader.getResourceAsStream("CustomLoadee5.class")) {
+            byte[] b = in.readAllBytes();
+            Util.replace(b, "this is", "DAS IST"); // Modify the bytecodes
+            Class<?> c = custLoader.myDefineClass(b, 0, b.length);
+            System.out.println(c.newInstance());
+            if (!"DAS IST CustomLoadee5".equals(c.newInstance().toString())) {
+                throw new RuntimeException("Bytecode modification not successful");
+            }
+            if (wb.isSharedClass(c)) {
+                throw new RuntimeException(c + "should not be loaded from CDS");
+            }
+        }
+
+        // When isFirstTime==true, the VM will try to load the archived copy of CustomLoadee5Child,
+        // but it will fail (because CustomLoadee5 was not loaded from the archive) and will recover
+        // by decoding the class from its classfile data.
+        // This failure should not leave the JVM in an inconsistent state.
+        Class<?> child = custLoader.loadClass("CustomLoadee5Child");
+        if (wb.isSharedClass(child)) {
+            throw new RuntimeException(child + "should not be loaded from CDS");
+        }
+    }
+
+    static class CustomLoader extends URLClassLoader {
+        public CustomLoader(URL[] urls, ClassLoader parent) {
+            super(urls, parent);
+        }
+
+        public Class<?> myDefineClass(byte[] b, int off, int len)
+            throws ClassFormatError
+        {
+            return super.defineClass(b, off, len);
+        }
     }
 }