RFR 8020675: invalid jar file in the bootclasspath could lead to jvm fatal error
Calvin Cheung
calvin.cheung at oracle.com
Fri Aug 23 13:23:04 PDT 2013
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/20130823/a2692cbf/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list