RFR(S): 8232980: Cleanup initialization of function pointers into java.base from classloader.cpp

[Ioi Lam]

Hi Chtistoph,

Thanks for fixing this.

I think is OK to leave the 3 functions separate, as each of them are 
non-trivial.

In fact, we can probably delay calling of 
ClassLoader::load_zip_library() in ClassPathZipEntry::open_entry, so 
apps that don't use JAR files can start up a little faster. I'll file an 
REF for this: JDK-8232989.

For repeated calls like this:


992 ZipOpen = CAST_TO_FN_PTR(ZipOpen_t, os::dll_lookup(handle, "ZIP_Open"));
993 if (ZipOpen == NULL) {
994 vm_exit_during_initialization("Corrupted ZIP library: ZIP_Open 
missing", path);
995 }
996 ZipClose = CAST_TO_FN_PTR(ZipClose_t, os::dll_lookup(handle, 
"ZIP_Close"));
997 if (ZipClose == NULL) {
998 vm_exit_during_initialization("Corrupted ZIP library: ZIP_Close 
missing", path);
999 }


I think it's better to use a utility function to do the check and exiting.

Thanks
- Ioi

On 10/24/19 8:41 AM, Langer, Christoph wrote:
> Hi,
>
> please help reviewing a cleanup patch to classLoader.cpp.
>
> The methods load_zip_library() and load_jimage_library() can be cleaned up a little bit. In my patch, I also extracted the initialization of the one symbol coming from libjava to a new method load_java_library(). However, I'm not fully sure on whether it would be nicer to have all these 3 methods consolidated into one. What do you think?
>
> In my patch I check for all needed symbols since it's all coming from the JDK and we can assume consistency. Should there be a problem in resolving some symbol, then VM initialization should fail.
>
> Furthermore, I'm wondering, whether to use guarantee or vm_exit_during_initialization for the NULL checks of the resolved symbols. Currently we have both but I think we should use one consistent approach. I  think vm_exit_during_initialization would be the best fit. Opinions?
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232980
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8232980.0/
>
> Thanks
> Christoph
>