RFR(S): 8205946: JVM crash after call to ClassLoader::setup_bootstrap_search_path()

[Calvin Cheung]

Updated webrev with the changes mentioned below:
     http://cr.openjdk.java.net/~ccheung/8205946/webrev.01/

I've rerun hs-tier{1,2,3} tests.

thanks,
Calvin

On 7/9/18, 1:43 PM, Calvin Cheung wrote:
> Hi Lois,
>
> Thanks for your review.
>
> On 7/9/18, 11:58 AM, Lois Foltan wrote:
>> On 7/9/2018 1:29 PM, Calvin Cheung wrote:
>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8205946
>>>
>>> webrev: http://cr.openjdk.java.net/~ccheung/8205946/webrev.00/
>>>
>>> The JVM crash could be simulated by renaming/removing the modules 
>>> file under the jdk/lib directory.
>>> The proposed simple fix is to perform a 
>>> vm_exit_during_initialization().
>>
>> Hi Calvin,
>>
>> Some clarifying questions.  Is this just an issue for exploded builds?
> I don't think so. As mentioned above, I could reproduce the crash with 
> a regular jdk image build by renaming the modules file under the 
> jdk/lib directory.
>>   I would prefer the exit to occur if the os::stat() fails for the 
>> system class path in os::set_boot_path().
> Instead of exiting in os::set_boot_path(), how about checking the 
> return status of os::set_boot_path() in the caller and exiting there 
> like the following:
> bash-4.2$ hg diff os_linux.cpp
> diff --git a/src/hotspot/os/linux/os_linux.cpp 
> b/src/hotspot/os/linux/os_linux.cpp
> --- a/src/hotspot/os/linux/os_linux.cpp
> +++ b/src/hotspot/os/linux/os_linux.cpp
> @@ -367,7 +367,9 @@
>        }
>      }
>      Arguments::set_java_home(buf);
> -    set_boot_path('/', ':');
> +    if (!set_boot_path('/', ':')) {
> +      vm_exit_during_initialization("Failed setting boot class 
> path.", NULL);
> +    }
>    }
>
> Note that before the above change, the return status of 
> set_boot_path() isn't checked.
> The above would involve changing 5 of those os_*.cpp files, one for 
> each O/S.
>
>>   With certainly an added assert later in 
>> ClassLoader::setup_bootstrap_search_path() to ensure that the system 
>> class path is never NULL.
> Sure, I can add an assert there.
> I'll post updated webrev once I've made the change and done testing.
>
> thanks,
> Calvin
>>
>> Thanks,
>> Lois
>>
>>>
>>> Ran hs-tier{1,2,3} tests successfully including the new test case.
>>>
>>> thanks,
>>> Calvin
>>