RFR 8020675: invalid jar file in the bootclasspath could lead to jvm fatal error

[David Holmes]

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?

> 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! ???

>>
>> 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.

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
>