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