RFR: 8263632: Improve exception handling of APIs in classLoader.cpp

[Ioi Lam]

On Thu, 25 Mar 2021 18:42:37 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> Please review this change which includes:
> 
> -  adding `ClassLoader::create_class_path_entry_or_fail()` and `ClassLoader::create_class_path_entry_or_fail()` functions for better readability. They will call the existing `ClassLoader::create_class_path_entry()`.
> - replacing the TRAPS parameter with `Thread* current` for the functions which never throw exception.
> 
> Testing: tiers 1,2 (passed); tiers 3,4 (in progress).

Changes requested by iklam (Reviewer).

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

> 552:   // File or directory found
> 553:   ClassPathEntry* new_entry = NULL;
> 554:   new_entry = create_class_path_entry_or_fail(path, &st,

Ideally, there's should be no need to check for new_entry == NULL when calling  calling create_class_path_entry_or_fail(). This will make the code easier to read.

But currently the create_class_path_entry_or_fail() is still "tri-state":

- success: non-null
- throw vmSymbols::java_io_IOException()
- return NULL

The third case is this:

        // Don't complain about bad jar files added via -Xbootclasspath/a:.
        if (throw_exception && is_init_completed()) {
          THROW_MSG_(vmSymbols::java_lang_ClassNotFoundException(), msg, NULL);
        } else {
          return NULL; <<<<< here
        }

I am wondering if we can get rid of this logic. That way, we can add asserts like:

ClassPathEntry* ClassLoader::create_class_path_entry_or_fail(...) {
  ClassPathEntry* entry = create_class_path_entry(, THREAD);
  assert(entry != NULL || HAS_PENDING_EXCEPTION, "must throw or return valid entry");
  return entry;
}

ClassPathEntry* ClassLoader::create_class_path_entry_or_null(...) {
  ClassPathEntry* entry = create_class_path_entry(, current);
  assert(!HAS_PENDING_EXCEPTION, "must not throw");
  return entry;
}

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

PR: https://git.openjdk.java.net/jdk/pull/3203