8304292: Memory leak related to ClassLoader::update_class_path_entry_list

Reviewed-by: dholmes, iklam

src/hotspot/share/classfile/classLoader.cpp

@@ -523,7 +523,8 @@ void ClassLoader::setup_app_search_path(JavaThread* current, const char *class_p
 
   while (cp_stream.has_next()) {
     const char* path = cp_stream.get_next();
-    update_class_path_entry_list(current, path, false, false, false);
+    update_class_path_entry_list(current, path, /* check_for_duplicates */ true,
+                                 /* is_boot_append */ false, /* from_class_path_attr */ false);
   }
 }
 
@@ -668,7 +669,8 @@ void ClassLoader::setup_bootstrap_search_path_impl(JavaThread* current, const ch
     } else {
       // Every entry on the boot class path after the initial base piece,
       // which is set by os::set_boot_path(), is considered an appended entry.
-      update_class_path_entry_list(current, path, false, true, false);
+      update_class_path_entry_list(current, path, /* check_for_duplicates */ false,
+                                    /* is_boot_append */ true, /* from_class_path_attr */ false);
     }
   }
 }
@@ -803,7 +805,7 @@ void ClassLoader::add_to_boot_append_entries(ClassPathEntry *new_entry) {
 // Note that at dump time, ClassLoader::_app_classpath_entries are NOT used for
 // loading app classes. Instead, the app class are loaded by the
 // jdk/internal/loader/ClassLoaders$AppClassLoader instance.
-void ClassLoader::add_to_app_classpath_entries(JavaThread* current,
+bool ClassLoader::add_to_app_classpath_entries(JavaThread* current,
                                                ClassPathEntry* entry,
                                                bool check_for_duplicates) {
 #if INCLUDE_CDS
@@ -813,7 +815,7 @@ void ClassLoader::add_to_app_classpath_entries(JavaThread* current,
     while (e != nullptr) {
       if (strcmp(e->name(), entry->name()) == 0) {
         // entry already exists
-        return;
+        return false;
       }
       e = e->next();
     }
@@ -832,6 +834,7 @@ void ClassLoader::add_to_app_classpath_entries(JavaThread* current,
     ClassLoaderExt::process_jar_manifest(current, entry);
   }
 #endif
+  return true;
 }
 
 // Returns true IFF the file/dir exists and the entry was successfully created.
@@ -854,7 +857,10 @@ bool ClassLoader::update_class_path_entry_list(JavaThread* current,
     if (is_boot_append) {
       add_to_boot_append_entries(new_entry);
     } else {
-      add_to_app_classpath_entries(current, new_entry, check_for_duplicates);
+      if (!add_to_app_classpath_entries(current, new_entry, check_for_duplicates)) {
+        // new_entry is not saved, free it now
+        delete new_entry;
+      }
     }
     return true;
   } else {

src/hotspot/share/classfile/classLoader.hpp

@@ -222,7 +222,7 @@ class ClassLoader: AllStatic {
   CDS_ONLY(static ClassPathEntry* _last_module_path_entry;)
   CDS_ONLY(static void setup_app_search_path(JavaThread* current, const char* class_path);)
   CDS_ONLY(static void setup_module_search_path(JavaThread* current, const char* path);)
-  static void add_to_app_classpath_entries(JavaThread* current,
+  static bool add_to_app_classpath_entries(JavaThread* current,
                                            ClassPathEntry* entry,
                                            bool check_for_duplicates);
   CDS_ONLY(static void add_to_module_path_entries(const char* path,

test/hotspot/jtreg/runtime/cds/appcds/ClassPathAttr.java

@@ -55,9 +55,10 @@ public class ClassPathAttr {
         "CpAttr2", "CpAttr3", "CpAttr4", "CpAttr5");
     buildCpAttr("cpattr5_123456789_223456789_323456789_423456789_523456789_623456789", "cpattr5_extra_long.mf", "CpAttr5", "CpAttr5");
 
+    String[] classlist = { "CpAttr1", "CpAttr2", "CpAttr3", "CpAttr4", "CpAttr5"};
+    String jar4 = TestCommon.getTestJar("cpattr4.jar");
     for (int i=1; i<=2; i++) {
       String jar1 = TestCommon.getTestJar("cpattr1.jar");
-      String jar4 = TestCommon.getTestJar("cpattr4.jar");
       if (i == 2) {
         // Test case #2 -- same as #1, except we use cpattr1_long.jar, which has a super-long
         // Class-Path: attribute.
@@ -65,11 +66,7 @@ public class ClassPathAttr {
       }
       String cp = jar1 + File.pathSeparator + jar4;
 
-      TestCommon.testDump(cp, TestCommon.list("CpAttr1",
+      TestCommon.testDump(cp, classlist);
-                                                          "CpAttr2",
-                                                          "CpAttr3",
-                                                          "CpAttr4",
-                                                          "CpAttr5"));
 
       TestCommon.run(
           "-cp", cp,
@@ -86,6 +83,16 @@ public class ClassPathAttr {
             output.shouldMatch("checking shared classpath entry: .*cpattr3.jar");
           });
     }
+
+    // test duplicate jars in the "Class-path" attribute in the jar manifest
+    buildCpAttr("cpattr_dup", "cpattr_dup.mf", "CpAttr1", "CpAttr1");
+    String cp = TestCommon.getTestJar("cpattr_dup.jar") + File.pathSeparator + jar4;
+    TestCommon.testDump(cp, classlist);
+
+    TestCommon.run(
+        "-cp", cp,
+        "CpAttr1")
+      .assertNormalExit();
   }
 
   static void testNonExistentJars() throws Exception {

test/hotspot/jtreg/runtime/cds/appcds/DuplicateClassPaths.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2023, 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.
+ *
+ */
+
+/*
+ * @test
+ * @summary duplicate class paths test
+ * @requires vm.cds
+ * @library /test/lib
+ * @compile test-classes/Hello.java
+ * @run driver DuplicateClassPaths
+ */
+
+import java.io.File;
+
+public class DuplicateClassPaths {
+    public static void main(String[] args) throws Exception {
+        String appJar = JarBuilder.getOrCreateHelloJar();
+        String jars = appJar + File.pathSeparator + appJar;
+        TestCommon.test(jars, TestCommon.list("Hello"), "Hello");
+    }
+}

test/hotspot/jtreg/runtime/cds/appcds/test-classes/cpattr_dup.mf

@@ -0,0 +1,3 @@
+Manifest-Version: 1.0
+Class-Path: cpattr2.jar cpattr2.jar
+Created-By: 1.9.0-internal (Oracle Corporation)