RFR 8020675: invalid jar file in the bootclasspath could lead to jvm fatal error
David Holmes
david.holmes at oracle.com
Fri Aug 23 21:02:47 PDT 2013
On 24/08/2013 3:53 AM, Calvin Cheung wrote:
> 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.
Which is exactly the problem I was flagging. :)
David
-----
> 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
>>>
>
More information about the hotspot-runtime-dev
mailing list