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

Calvin Cheung calvin.cheung at oracle.com
Mon Aug 26 11:28:29 PDT 2013


Ioi,

Thanks for your review.

Calvin

On 8/26/2013 11:09 AM, Ioi Lam wrote:
> 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/6e8551e3/attachment.html 


More information about the hotspot-runtime-dev mailing list