RFR(S): 8203354: assert in ClassLoader::update_module_path_entry_list() could have incorrect message

[Thomas Stüfe]

Hi Calvin,

some small nits:

- could you use vm_exit_during_initialization instead of raw exit?
- "Bad path %s" - that is an assumption, stat() may fail for a number
of reasons. So I would make the message more neutral. I also always
enclose such an output in quotes, to catch if the caller had
leading/trailing spaces. So, maybe this: "CDS dump aborted (path was
\"%s\")." ?

Side note: I looked at the various os::stat implementations and see a
number of issues. We copy the input path around, which is on Posix
platforms unnecessary since os::native_path() returns just the path;
and since we use a fixed sized buffer we artificially limit the path
os::stat() recognizes. That is bad. Also, on windows, we return the
Win32 error code (GetLastError) as errno, which is just plain wrong.

But these are issues beyond your patch.

Kind Regards, Thomas


On Tue, May 22, 2018 at 9:08 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8203354
>
> webrev: http://cr.openjdk.java.net/~ccheung/8203354/webrev.00/
>
> Converting the assert in ClassLoader::update_module_path_entry_list() to a
> meaningful error message before aborting the CDS dump.
>
> With a module path with very long length (> 2K), the error message will be
> like the following:
>
> os::stat error 36 (ENAMETOOLONG). CDS dump aborted. Bad path
> /scratch/myDir/JTwork/scratch/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx <snip>
>
> Note that tty->print_cr() has a buffer size limit of 2000, so the full path
> won't be shown entirely. That's why I'd like to put the error code up front
> before the path.
>
> Ran the modified tests on Oracle platforms.
>
> thanks,
> Calvin