RFR 8027146: Class loading verification failure if GC occurs in Universe::flush_dependents_on

[Coleen Phillimore]

Dan, Thank you for reviewing the code.   Most of the files were easy.

On 2/14/14 10:15 AM, Daniel D. Daugherty wrote:
> On 2/13/14 12:00 PM, Coleen Phillimore wrote:
>> Summary: Remove search in system dictionary and hacks, replace with 
>> verifying in CLD::_klasses list.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8027146/
>
> src/share/vm/classfile/classLoaderData.hpp
>     Nocommentsother than why add a blank line on 273?

No reason.  I'll remove it.  I might have made and removed a different 
change there.

>
> src/share/vm/classfile/classLoaderData.cpp
>     No comments.
>
> src/share/vm/classfile/dictionary.cpp
>     No comments.
>
> src/share/vm/classfile/systemDictionary.hpp
>     No comments.
>
> src/share/vm/classfile/systemDictionary.cpp
>     So SystemDictionary::verify_obj_klass_present() isn't even
>     useful or correct in sanity checking/debug code?

I don't think it's that useful since there are so many exceptions to 
this check and it has to lock the system dictionary for the query.   And 
it used to be called when verifying the classes in the system dictionary.

>
>     Update: So InstanceKlass::verify_on() now does a much simpler
> contains_klass() check. I understand now why the existing
>         check needs to be replaced by the simpler and less strict
>         check.

Yes, when we create the class we link it to it's ClassLoaderData and 
vice versa.  Making sure these links are correct is worthwhile.

>
> src/share/vm/oops/arrayKlass.hpp
>     No comments.
>
> src/share/vm/oops/arrayKlass.cpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.hpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.cpp
>     The old code didn't call verify_obj_klass_present() unless
>     is_loaded() is true. The new code presumes that is_loaded()
>     is true since it expects the class to be present in the
>     class_loader_data.
>
>     Update: To put it another way: The old code assumed when
>     is_loaded() is true that the class was in the system dictionary
>     or in the placeholder table. That's not quite correct in all
>     cases. When is_loaded() is true,all we know for sure is that
> the class must be present inthe class_loader_data(); it might
> still be "on its way" to either the system dictionary or the
> placeholder table, but has not yet arrived there.

Yes. I think this is the better verification.

Thanks!
Coleen
>
> src/share/vm/oops/klass.hpp
>     No comments.
>
> src/share/vm/oops/klass.cpp
>     No comments.
>
> src/share/vm/oops/objArrayKlass.hpp
>     No comments.
>
> src/share/vm/oops/objArrayKlass.cpp
>     No comments.
>
> Thumbs up.
>
> Dan
>
>
>
>> bug link https://bugs.openjdk.java.net/browse/JDK-8027146
>>
>> Tested with AllocClasses.java with VM_Verify op suggested in the bug 
>> report (can't add it's noreg-hard).   Also ran all jtreg and gc jtreg 
>> tests with -XX:+VerifyBeforeGC.
>>
>> Thanks,
>> Coleen
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140214/2ab077c9/attachment-0001.html