RFR 8020675: invalid jar file in the bootclasspath could lead to jvm fatal error
Ioi Lam
ioi.lam at oracle.com
Mon Aug 26 11:09:15 PDT 2013
Calvin,
Looks good.
Thanks
- Ioi
On 08/23/2013 01:23 PM, Calvin Cheung wrote:
> Hi Misha,
>
> Thanks for reviewing the test case.
>
> All,
>
> I've updated the webrev with the one-line change in classLoader.cpp.
> http://cr.openjdk.java.net/~ccheung/8020675/webrev/
>
> thanks,
> Calvin
>
> On 8/23/2013 11:45 AM, Mikhailo Seledtsov wrote:
>> Hi Calvin,
>>
>> I only reviewed the test portion. It looks good to me.
>>
>> Misha
>>
>> On 8/23/2013 1:53 PM, Calvin Cheung wrote:
>>> On 8/22/2013 11:54 PM, David Holmes wrote:
>>>> On 23/08/2013 4:05 PM, Calvin Cheung wrote:
>>>>> On 8/22/2013 5:21 PM, David Holmes wrote:
>>>>>> Hi Calvin,
>>>>>>
>>>>>> I'm having trouble keeping track of this one ...
>>>>>>
>>>>>> On 23/08/2013 8:55 AM, Calvin Cheung wrote:
>>>>>>> Note that the synopsis of the bug has been changed from:
>>>>>>> "Trying to print to a pdf file with PDFill PDF&Image Writer 10.0
>>>>>>> Build 4"
>>>>>>>
>>>>>>> bug link: https://bugs.openjdk.java.net/browse/JDK-8020675
>>>>>>>
>>>>>>> I've included the suggestions from Coleen and Ioi in this
>>>>>>> version of
>>>>>>> webrev:
>>>>>>> http://cr.openjdk.java.net/~ccheung/8020675/webrev/
>>>>>>
>>>>>> I don't understand your _has_error logic:
>>>>>>
>>>>>> 325 ClassPathEntry* cpe = resolve_entry(CHECK_NULL);
>>>>>> 326 if (cpe == NULL) {
>>>>>> 327 _has_error = true;
>>>>>> 328 return NULL;
>>>>>>
>>>>>> we will only hit line 327 if resolve_entry returns NULL _without_
>>>>>> there being an exception pending. How does that occur? What is
>>>>>> different about the two cases regardless?
>>>>> The _has_error flag was suggested by Ioi and is to avoid opening the
>>>>> invalid jar file again.
>>>>
>>>> Can't we just remove it from the classpath representation?
>>> I think it can be done if the error happens during vm initialization.
>>> But for this test scenario, the error happens after vm init.
>>> Perhaps this should be addressed as a separate bug fix?
>>>>
>>>>> The call sequence is as follows:
>>>>> ClassLoader::load_classfile()
>>>>> -> LazyClassPathEntry::open_stream()
>>>>> -> LazyClassPathEntry::resolve_entry()
>>>>> -> ClassLoader::create_class_path_entry()
>>>>>
>>>>> For second time, it'll return NULL without calling resolve_entry()
>>>>> again.
>>>>>
>>>>> The LazyClassPathEntry instance is instantiated by the following
>>>>> calls:
>>>>> ClassLoader::setup_bootstrap_search_path()
>>>>> ->ClassLoader::update_class_path_entry_list()
>>>>> ->ClassLoader::create_class_path_entry(path, &st,
>>>>> LazyBootClassLoader, CHECK)
>>>>>
>>>>> LazyBootClassLoader is set to true by default.
>>>>
>>>> Sorry but I don't see how that answers my question. Based on the
>>>> below there isn't a second time because the first time will cause
>>>> the pending exception which will terminate the VM. Even if we dont
>>>> terminate and somehow call this again, we didn't set _has_error the
>>>> first time anyway! ???
>>> Let's not confused with the classes we're preloading during vm
>>> initialization and the applications classes after vm initialization.
>>>
>>> Consider the following test case:
>>> public class test3 {
>>> public static void main(String[] args) {
>>> try {
>>> //Class cls = Class.forName("javax.crypto.KeyGenerator");
>>> //System.out.println("Class = " + cls.getName());
>>> Class cls = Class.forName("xxx");
>>> System.out.println("Class = " + cls.getName());
>>> } catch (ClassNotFoundException cnfe) {
>>> cnfe.printStackTrace();
>>> }
>>> try {
>>> Class cls2 = Class.forName("yyy");
>>> System.out.println("Class = " + cls2.getName());
>>> } catch (ClassNotFoundException cnfe) {
>>> cnfe.printStackTrace();
>>> }
>>> }
>>> }
>>>
>>> Let's say we have a dummy.jar with 0-byte and test3.class in the
>>> same directory.
>>> And command line as follows:
>>> java -Xbootclasspath/a:dummy.jar -cp . test3
>>>
>>> During vm init. (as in the above
>>> ClassLoader::setup_bootstrap_search_path() call sequence), there's
>>> no problem because vm doesn't need to read dummy.jar as all other
>>> preload classes can be found in jdk's jar files (rt.jar, jce.jar,
>>> etc.) (Note: this isn't the case with 7u25, we were trying to
>>> preload a non-existing class, please refer to my comment in the bug
>>> report.)
>>>
>>> When vm tries to load the class for the test case (test3.class),
>>> it'll call into LazyClassPathEntry::open_stream(). This is the first
>>> time resolve_entry() returns NULL and the _has_error flag will be
>>> set to true. The next time vm will try to load xxx.class. Since
>>> _has_error was set to NULL, it will return NULL without calling
>>> resolve_entry(). Same story for the third time for the yyy.class.
>>>
>>> call stack as follows:
>>> jvm.dll!ClassLoader::create_class_path_entry(char * path, const
>>> stat * st, bool lazy, Thread * __the_thread__) Line 515 C++
>>> jvm.dll!LazyClassPathEntry::resolve_entry(Thread *
>>> __the_thread__) Line 304 + 0x19 bytes C++
>>> jvm.dll!LazyClassPathEntry::open_stream(const char * name,
>>> Thread * __the_thread__) Line 325 + 0xc bytes C++
>>> jvm.dll!ClassLoader::load_classfile(Symbol * h_name, Thread *
>>> __the_thread__) Line 909 + 0x1b bytes C++
>>> jvm.dll!SystemDictionary::load_instance_class(Symbol *
>>> class_name, Handle class_loader, Thread * __the_thread__) Line 1301
>>> + 0x14 bytes C++
>>> jvm.dll!SystemDictionary::resolve_instance_class_or_null(Symbol *
>>> name, Handle class_loader, Handle protection_domain, Thread *
>>> __the_thread__) Line 779 + 0x18 bytes C++
>>> jvm.dll!SystemDictionary::resolve_or_null(Symbol * class_name,
>>> Handle class_loader, Handle protection_domain, Thread *
>>> __the_thread__) Line 232 + 0x15 bytes C++
>>> jvm.dll!SystemDictionary::resolve_or_null(Symbol * class_name,
>>> Thread * __the_thread__) Line 237 + 0x23 bytes C++
>>> > jvm.dll!JVM_FindClassFromBootLoader(JNIEnv_ * env, const char *
>>> name) Line 770 + 0x12 bytes C++
>>> java.dll!70671e8c()
>>>
>>> I've just noticed that there's a slight error in my change in line
>>> #325 of classLoader.cpp:
>>>
>>> ClassPathEntry* cpe = resolve_entry(CHECK_NULL);
>>>
>>> should be
>>>
>>> ClassPathEntry* cpe = resolve_entry(THREAD);
>>>
>>> Otherwise, it'll skip the rest of the statements.
>>> I'll update webrev shortly.
>>>
>>>>
>>>>>>
>>>>>> And we still have this sequence:
>>>>>>
>>>>>> void ClassLoader::initialize() {
>>>>>> assert(_package_hash_table == NULL, "should have been
>>>>>> initialized by
>>>>>> now.");
>>>>>> EXCEPTION_MARK;
>>>>>> ...
>>>>>> setup_bootstrap_search_path();
>>>>>> -> update_class_path_entry_list(path, false);
>>>>>> -> create_class_path_entry((char *)path, st, &new_entry,
>>>>>> LazyBootClassLoader, CHECK);
>>>>> As mentioned above, this call sequence is for instantiating the
>>>>> LazyClassPathEntry instances.
>>>>>>
>>>>>> So if we return after the call to create_class_path_entry with an
>>>>>> exception pending we will crash when we hit the EXCEPTION_MARK. Why
>>>>>> doesn't this happen?
>>>>> During vm initilization, it's ok to have exception pending. The
>>>>> destructor of ExceptionMark is as follows:
>>>>> ExceptionMark::~ExceptionMark() {
>>>>> if (_thread->has_pending_exception()) {
>>>>> Handle exception(_thread, _thread->pending_exception());
>>>>> _thread->clear_pending_exception(); // Needed to avoid infinite
>>>>> recursion
>>>>> if (is_init_completed()) {
>>>>> exception->print();
>>>>> fatal("ExceptionMark destructor expects no pending
>>>>> exceptions");
>>>>> } else {
>>>>> vm_exit_during_initialization(exception);
>>>>> }
>>>>> }
>>>>> }
>>>>>
>>>>> So if is_init_completed() is false, it'll just print an error and
>>>>> exit.
>>>>
>>>> So we do hit it - ok. So this yet another place where the VM should
>>>> not abort during initialization.
>>> Yes and this is for preloading class scenario during vm init.
>>> IMHO, I don't think we should change the behavior now.
>>>
>>> thanks,
>>> Calvin
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> thanks,
>>>>> Calvin
>>>>>
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>
>>>>>>> Tests:
>>>>>>> jprt
>>>>>>> (in progress - only about 30 tests left on the windows
>>>>>>> platforms, no failure so far;
>>>>>>> a previous run with only Coleen's suggestions was
>>>>>>> successful)
>>>>>>>
>>>>>>> vm.quick.testlist on linux_x64
>>>>>>>
>>>>>>> Please review.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Calvin
>>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130826/7ee7ac5f/attachment.html
More information about the hotspot-runtime-dev
mailing list