RFR: 8220095: Assertion failure when symlink (with different name) is used for lib/modules file

[Lois Foltan]

On 3/6/2019 6:20 PM, Jiangli Zhou wrote:
> Please review the following fix for 8220095.
>
> webrev: http://cr.openjdk.java.net/~jiangli/8220095/webrev.00/
> bug: https://bugs.openjdk.java.net/browse/JDK-8220095
>
> Symbolic links may be used commonly in some cloud environments. The target
> files might have different names than the linked names. The VM crashes when
> 'lib/modules' links to a target file with a different name, for example,
> lib/modules -> lib/0.
>
> The usage of the hard-coded MODULES_IMAGE_NAME (used by
> ClassLoader::is_modules_image(const char*)) can become problematic in this
> case and leads to assertion failures and crashes in this case. The
> is_modules_image()
> checks always fail even the actual file is the runtime image because the
> canonical paths (which might not end with 'modules') are passed in for the
> check. The fix is to obtain the real name from the canonical path early
> during initialization and use that for the is_modules_image() check.
>
> Tested with submit repo testing, tier1, tier2, and hotspot runtime tests
> locally.
>
> Thanks!
> Jiangli

Hi Jiangli,

This change looks good!  A couple of minor comments:

-  classLoader.hpp
     line #39 - I think you should expand on the comment and add why in 
the case of the canonical path name MODULES_IMAGE_NAME can not be used 
as-is but if one is dealing with the system boot path string set up by 
os::set_boot_path, than MODULES_IMAGE_NAME should be used.

- classLoader.cpp
    line #702 - Can you add the same explanation comment as in 
classLoader.hpp ahead of the new ClassLoader::init_modules_image_name() 
method?  That would be helpful.
    line #710 - ahead of setting _modules_image_name please consider 
adding an assert that p1's length is greater than 0?
                        Maybe there should there be an error case test 
where "lib/modules -> lib/"

Thanks,
Lois

-