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

Mikhailo Seledtsov mikhailo.seledtsov at oracle.com
Fri Aug 23 11:45:49 PDT 2013


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/20130823/d33aa125/attachment-0001.html 


More information about the hotspot-runtime-dev mailing list