RFR: 8304292: Memory leak related to ClassLoader::update_class_path_entry_list [v2]

[Ioi Lam]

On Thu, 3 Aug 2023 22:09:48 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Please review this small fix for `ClassPathEntry` leak when there's a duplicate entry in the app class path (`-cp`) and the `check_for_duplicate` argument is set to `true`, the `ClassLoader::add_to_app_classpath_entries` function should delete the entry before returning. This fix also corrects the call to `update_class_path_entry_list` from `setup_app_search_path`; the `check_for_duplicate` argument should be set to `true`. Two test cases are included to exercise the code path.
>> 
>> Passed tiers 1 - 4 testing.
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add comments to boolean args

src/hotspot/share/classfile/classLoader.cpp line 816:

> 814:       if (strcmp(e->name(), entry->name()) == 0) {
> 815:         // entry already exists
> 816:         delete entry;

I think the API contract between `update_class_path_entry_list()` and `add_to_app_classpath_entries()` is too complicated:

- `add_to_app_classpath_entries()` will delete the entry if it decides that the entry shouldn't be saved

It's better for the lifetime of the entry to be maintained by `update_class_path_entry_list()`. It should do something like:


 new_entry = create_class_path_entry(current, path, &st, is_boot_append, from_class_path_attr);

 if (add_to_app_classpath_entries(current, new_entry, check_for_duplicates) == false) {
  // new_entry is not saved, free it now
  delete new_entry;
 }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15132#discussion_r1289219802