Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8263632: Improve exception handling of APIs in classLoader.cpp #3203

Closed
wants to merge 4 commits into from
Closed

8263632: Improve exception handling of APIs in classLoader.cpp #3203

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52632b6
8263632: Improve exception handling of APIs in classLoader.cpp
calvinccheung Mar 25, 2021
c2d1cde
get rid of throw_exception in ClassLoader::create_class_path_entry
calvinccheung Mar 26, 2021
9bf7c0b
@coleenp review comment
calvinccheung Mar 26, 2021
76932cc
@iklam review comment
calvinccheung Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 34 additions & 69 deletions src/hotspot/share/classfile/classLoader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ void ClassLoader::trace_class_path(const char* msg, const char* name) {
}
}

void ClassLoader::setup_bootstrap_search_path(TRAPS) {
void ClassLoader::setup_bootstrap_search_path(Thread* current) {
const char* sys_class_path = Arguments::get_sysclasspath();
assert(sys_class_path != NULL, "System boot class path must not be NULL");
if (PrintSharedArchiveAndExit) {
Expand All @@ -509,19 +509,19 @@ void ClassLoader::setup_bootstrap_search_path(TRAPS) {
} else {
trace_class_path("bootstrap loader class path=", sys_class_path);
}
setup_bootstrap_search_path_impl(sys_class_path, CHECK);
setup_bootstrap_search_path_impl(current, sys_class_path);
}

#if INCLUDE_CDS
void ClassLoader::setup_app_search_path(const char *class_path, TRAPS) {
void ClassLoader::setup_app_search_path(Thread* current, const char *class_path) {
Arguments::assert_is_dumping_archive();

ResourceMark rm;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add 'current' as a parameter to ResourceMark since you have it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Coleen.
I've fixed it.

ClasspathStream cp_stream(class_path);

while (cp_stream.has_next()) {
const char* path = cp_stream.get_next();
update_class_path_entry_list(path, false, false, false, CHECK);
update_class_path_entry_list(current, path, false, false, false);
}
}

Expand All @@ -541,7 +541,7 @@ void ClassLoader::add_to_module_path_entries(const char* path,
}

// Add a module path to the _module_path_entries list.
void ClassLoader::setup_module_search_path(const char* path, TRAPS) {
void ClassLoader::setup_module_search_path(Thread* current, const char* path) {
Arguments::assert_is_dumping_archive();
struct stat st;
if (os::stat(path, &st) != 0) {
Expand All @@ -551,13 +551,11 @@ void ClassLoader::setup_module_search_path(const char* path, TRAPS) {
}
// File or directory found
ClassPathEntry* new_entry = NULL;
new_entry = create_class_path_entry_or_fail(path, &st,
false /*is_boot_append */, false /* from_class_path_attr */, CHECK);
if (new_entry == NULL) {
return;
new_entry = create_class_path_entry(current, path, &st,
false /*is_boot_append */, false /* from_class_path_attr */);
if (new_entry != NULL) {
add_to_module_path_entries(path, new_entry);
}

add_to_module_path_entries(path, new_entry);
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return is no longer needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ioi.
I've fixed it.

}

Expand Down Expand Up @@ -595,7 +593,7 @@ void ClassLoader::setup_patch_mod_entries() {
struct stat st;
if (os::stat(path, &st) == 0) {
// File or directory found
ClassPathEntry* new_entry = create_class_path_entry_or_null(THREAD, path, &st, false, false);
ClassPathEntry* new_entry = create_class_path_entry(THREAD, path, &st, false, false);
// If the path specification is valid, enter it into this module's list
if (new_entry != NULL) {
module_cpl->add_to_list(new_entry);
Expand Down Expand Up @@ -627,8 +625,8 @@ bool ClassLoader::is_in_patch_mod_entries(Symbol* module_name) {
}

// Set up the _jrt_entry if present and boot append path
void ClassLoader::setup_bootstrap_search_path_impl(const char *class_path, TRAPS) {
ResourceMark rm(THREAD);
void ClassLoader::setup_bootstrap_search_path_impl(Thread* current, const char *class_path) {
ResourceMark rm(current);
ClasspathStream cp_stream(class_path);
bool set_base_piece = true;

Expand All @@ -652,7 +650,7 @@ void ClassLoader::setup_bootstrap_search_path_impl(const char *class_path, TRAPS
struct stat st;
if (os::stat(path, &st) == 0) {
// Directory found
ClassPathEntry* new_entry = create_class_path_entry_or_null(THREAD, path, &st, false, false);
ClassPathEntry* new_entry = create_class_path_entry(current, path, &st, false, false);

// Check for a jimage
if (Arguments::has_jimage()) {
Expand All @@ -669,7 +667,7 @@ void ClassLoader::setup_bootstrap_search_path_impl(const char *class_path, TRAPS
} else {
// Every entry on the system 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(path, false, true, false, CHECK);
update_class_path_entry_list(current, path, false, true, false);
}
}
}
Expand All @@ -693,7 +691,7 @@ void ClassLoader::add_to_exploded_build_list(Thread* current, Symbol* module_sym
struct stat st;
if (os::stat(path, &st) == 0) {
// Directory found
ClassPathEntry* new_entry = create_class_path_entry_or_null(current, path, &st, false, false);
ClassPathEntry* new_entry = create_class_path_entry(current, path, &st, false, false);

// If the path specification is valid, enter it into this module's list.
// There is no need to check for duplicate modules in the exploded entry list,
Expand All @@ -719,38 +717,19 @@ jzfile* ClassLoader::open_zip_file(const char* canonical_path, char** error_msg,
return (*ZipOpen)(canonical_path, error_msg);
}

ClassPathEntry* ClassLoader::create_class_path_entry_or_fail(const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr, TRAPS) {
return create_class_path_entry(path, st, true /*throw_exception*/, is_boot_append, from_class_path_attr, CHECK_NULL);
}

ClassPathEntry* ClassLoader::create_class_path_entry_or_null(Thread* current,
const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr) {
return create_class_path_entry(path, st, false /*throw_exception*/, is_boot_append, from_class_path_attr, current);
}

ClassPathEntry* ClassLoader::create_class_path_entry(const char *path, const struct stat* st,
bool throw_exception,
ClassPathEntry* ClassLoader::create_class_path_entry(Thread* current,
const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr,
TRAPS) {
JavaThread* thread = THREAD->as_Java_thread();
bool from_class_path_attr) {
JavaThread* thread = current->as_Java_thread();
ClassPathEntry* new_entry = NULL;
if ((st->st_mode & S_IFMT) == S_IFREG) {
ResourceMark rm(thread);
// Regular file, should be a zip or jimage file
// Canonicalized filename
const char* canonical_path = get_canonical_path(path, thread);
if (canonical_path == NULL) {
// This matches the classic VM
if (throw_exception) {
THROW_MSG_(vmSymbols::java_io_IOException(), "Bad pathname", NULL);
} else {
return NULL;
}
return NULL;
}
jint error;
JImageFile* jimage =(*JImageOpen)(canonical_path, &error);
Expand All @@ -762,21 +741,7 @@ ClassPathEntry* ClassLoader::create_class_path_entry(const char *path, const str
if (zip != NULL && error_msg == NULL) {
new_entry = new ClassPathZipEntry(zip, path, is_boot_append, from_class_path_attr);
} else {
char *msg;
if (error_msg == NULL) {
msg = NEW_RESOURCE_ARRAY_IN_THREAD(thread, char, strlen(path) + 128); ;
jio_snprintf(msg, strlen(path) + 127, "error in opening JAR file %s", path);
} else {
int len = (int)(strlen(path) + strlen(error_msg) + 128);
msg = NEW_RESOURCE_ARRAY_IN_THREAD(thread, char, len); ;
jio_snprintf(msg, len - 1, "error in opening JAR file <%s> %s", error_msg, path);
}
// 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;
}
return NULL;
}
}
log_info(class, path)("opened: %s", path);
Expand Down Expand Up @@ -847,10 +812,10 @@ 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(const char* path,
void ClassLoader::add_to_app_classpath_entries(Thread* current,
const char* path,
ClassPathEntry* entry,
bool check_for_duplicates,
TRAPS) {
bool check_for_duplicates) {
#if INCLUDE_CDS
assert(entry != NULL, "ClassPathEntry should not be NULL");
ClassPathEntry* e = _app_classpath_entries;
Expand All @@ -874,22 +839,22 @@ void ClassLoader::add_to_app_classpath_entries(const char* path,
}

if (entry->is_jar_file()) {
ClassLoaderExt::process_jar_manifest(entry, check_for_duplicates, CHECK);
ClassLoaderExt::process_jar_manifest(current, entry, check_for_duplicates);
}
#endif
}

// Returns true IFF the file/dir exists and the entry was successfully created.
bool ClassLoader::update_class_path_entry_list(const char *path,
bool ClassLoader::update_class_path_entry_list(Thread* current,
const char *path,
bool check_for_duplicates,
bool is_boot_append,
bool from_class_path_attr,
TRAPS) {
bool from_class_path_attr) {
struct stat st;
if (os::stat(path, &st) == 0) {
// File or directory found
ClassPathEntry* new_entry = NULL;
new_entry = create_class_path_entry_or_fail(path, &st, is_boot_append, from_class_path_attr, CHECK_false);
new_entry = create_class_path_entry(current, path, &st, is_boot_append, from_class_path_attr);
if (new_entry == NULL) {
return false;
}
Expand All @@ -899,7 +864,7 @@ bool ClassLoader::update_class_path_entry_list(const char *path,
if (is_boot_append) {
add_to_boot_append_entries(new_entry);
} else {
add_to_app_classpath_entries(path, new_entry, check_for_duplicates, CHECK_false);
add_to_app_classpath_entries(current, path, new_entry, check_for_duplicates);
}
return true;
} else {
Expand Down Expand Up @@ -1454,7 +1419,7 @@ void ClassLoader::initialize(TRAPS) {
// lookup java library entry points
load_java_library();
// jimage library entry points are loaded below, in lookup_vm_options
setup_bootstrap_search_path(CHECK);
setup_bootstrap_search_path(THREAD);
}

char* lookup_vm_resource(JImageFile *jimage, const char *jimage_version, const char *path) {
Expand Down Expand Up @@ -1491,15 +1456,15 @@ char* ClassLoader::lookup_vm_options() {
}

#if INCLUDE_CDS
void ClassLoader::initialize_shared_path(TRAPS) {
void ClassLoader::initialize_shared_path(Thread* current) {
if (Arguments::is_dumping_archive()) {
ClassLoaderExt::setup_search_paths(CHECK);
ClassLoaderExt::setup_search_paths(current);
}
}

void ClassLoader::initialize_module_path(TRAPS) {
if (Arguments::is_dumping_archive()) {
ClassLoaderExt::setup_module_paths(CHECK);
ClassLoaderExt::setup_module_paths(THREAD);
FileMapInfo::allocate_shared_path_table(CHECK);
}
}
Expand Down
37 changes: 15 additions & 22 deletions src/hotspot/share/classfile/classLoader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ class ClassLoader: AllStatic {
CDS_ONLY(static ClassPathEntry* _last_app_classpath_entry;)
CDS_ONLY(static ClassPathEntry* _module_path_entries;)
CDS_ONLY(static ClassPathEntry* _last_module_path_entry;)
CDS_ONLY(static void setup_app_search_path(const char* class_path, TRAPS);)
CDS_ONLY(static void setup_module_search_path(const char* path, TRAPS);)
static void add_to_app_classpath_entries(const char* path,
CDS_ONLY(static void setup_app_search_path(Thread* current, const char* class_path);)
CDS_ONLY(static void setup_module_search_path(Thread* current, const char* path);)
static void add_to_app_classpath_entries(Thread* current,
const char* path,
ClassPathEntry* entry,
bool check_for_duplicates,
TRAPS);
bool check_for_duplicates);
CDS_ONLY(static void add_to_module_path_entries(const char* path,
ClassPathEntry* entry);)
public:
Expand All @@ -241,8 +241,8 @@ class ClassLoader: AllStatic {
// - setup the boot loader's system class path
// - setup the boot loader's patch mod entries, if present
// - create the ModuleEntry for java.base
static void setup_bootstrap_search_path(TRAPS);
static void setup_bootstrap_search_path_impl(const char *class_path, TRAPS);
static void setup_bootstrap_search_path(Thread* current);
static void setup_bootstrap_search_path_impl(Thread* current, const char *class_path);
static void setup_patch_mod_entries();
static void create_javabase();

Expand All @@ -256,19 +256,12 @@ class ClassLoader: AllStatic {
static void release_load_zip_library();
static inline void load_zip_library_if_needed();
static jzfile* open_zip_file(const char* canonical_path, char** error_msg, JavaThread* thread);
static ClassPathEntry* create_class_path_entry(const char *path, const struct stat* st,
bool throw_exception,
bool is_boot_append,
bool from_class_path_attr, TRAPS);

public:
static ClassPathEntry* create_class_path_entry_or_fail(const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr, TRAPS);
static ClassPathEntry* create_class_path_entry_or_null(Thread* current,
const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr);
static ClassPathEntry* create_class_path_entry(Thread* current,
const char *path, const struct stat* st,
bool is_boot_append,
bool from_class_path_attr);

// Canonicalizes path names, so strcmp will work properly. This is mainly
// to avoid confusing the zip library
Expand All @@ -277,11 +270,11 @@ class ClassLoader: AllStatic {
int class_name_len);
static PackageEntry* get_package_entry(Symbol* pkg_name, ClassLoaderData* loader_data);
static int crc32(int crc, const char* buf, int len);
static bool update_class_path_entry_list(const char *path,
static bool update_class_path_entry_list(Thread* current,
const char *path,
bool check_for_duplicates,
bool is_boot_append,
bool from_class_path_attr,
TRAPS);
bool from_class_path_attr);
static void print_bootclasspath();

// Timing
Expand Down Expand Up @@ -346,7 +339,7 @@ class ClassLoader: AllStatic {
// Initialization
static void initialize(TRAPS);
static void classLoader_init2(Thread* current);
CDS_ONLY(static void initialize_shared_path(TRAPS);)
CDS_ONLY(static void initialize_shared_path(Thread* current);)
CDS_ONLY(static void initialize_module_path(TRAPS);)

static int compute_Object_vtable();
Expand Down
Loading