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

[Calvin Cheung]

Hi Jiangli,

Thanks for filing and fixing the bug.

Just couple of suggestions:

classLoader.cpp

Currently, os::file_separator() contains only 1 character. Consider 
using strrchr instead of a while loop?
You can keep the while loop in case in the len of os::file_separator is > 1.

Something like the following:
void ClassLoader::init_modules_image_name(const char* modules_path) {
   // skip directory names
   const char *p1, *p2;
   p1 = modules_path;
   int len = (int)strlen(os::file_separator());
   if (len == 1) {
     p2 = strrchr(modules_path, os::file_separator()[0]);
     if (p2 != NULL) {
       p1 = p2 + len;
     }
   } else {
     while ((p2 = strstr(p1, os::file_separator())) != NULL) {
       p1 = p2 + len;
     }
   }
   _modules_image_name = os::strdup(p1);
}

ModulesSymLink.java

29  * @requires (os.family == "linux")

I think the test should be applicable to the Macosx and the Solaris 
platforms too.
So perhaps just excluding windows:
         @requires (os.family != "windows")

72         String modules_prefix ...

This variable is unused.

thanks,
Calvin

On 3/6/19, 3: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