RFR(S): 8205946: JVM crash after call to ClassLoader::setup_bootstrap_search_path()
Lois Foltan
lois.foltan at oracle.com
Tue Jul 10 17:42:33 UTC 2018
On 7/10/2018 1:31 PM, Calvin Cheung wrote:
> Updated webrev with the changes mentioned below:
> http://cr.openjdk.java.net/~ccheung/8205946/webrev.01/
Hi Calvin,
Looks good, thanks for making the change. It does seem a bit odd that
the vm_exit_during_initialization is not just called within the method
os::set_boot_path(), however, it makes sense for each platform to decide
how to handle the failure.
Thanks,
Lois
>
> 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
>>>
More information about the hotspot-runtime-dev
mailing list