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