RFR (S) 8024667: VM crashes with "assert(method() != NULL) failed: must have set method"

[harold seigel]

Hi Dan,

Thanks for the review.  I like parenthesis, but I'll clean them up a bit 
along with the NULL checks.

Harold

On 10/11/2013 1:10 PM, Daniel D. Daugherty wrote:
> > http://cr.openjdk.java.net/~hseigel/bug_8024667/
>
> src/share/vm/memory/metadataFactory.hpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.cpp
>     nit line 348: if ((ti != sti) && (ti != NULL) && !ti->is_shared()) {
>         A little over parenthesized here.
>
>     line 354: if ((local_interfaces != 
> Universe::the_empty_klass_array()) &&
>     line 355:     (local_interfaces != NULL) && 
> !local_interfaces->is_shared()) {
>         Moving the NULL check to be first would be faster.
>         Also a little over parenthesized here.
>
>     line 385: if ((method_ordering() != 
> Universe::the_empty_int_array()) &&
>     line 386:     (method_ordering() != NULL) && 
> !method_ordering()->is_shared()) {
>         Moving the NULL check to be first would be faster.
>         Also a little over parenthesized here.
>
>     line 411: if (secondary_supers() != 
> Universe::the_empty_klass_array() &&
>     line 412:     secondary_supers() != transitive_interfaces() &&
>     line 413:     secondary_supers() != NULL && 
> !secondary_supers()->is_shared()) {
>         Moving the NULL check to be first would be faster.
>
>     line 422:   if ((fields() != NULL) && !fields()->is_shared()) {
>         A little over parenthesized here.
>
>     line 437: if ((inner_classes() != 
> Universe::the_empty_short_array()) &&
>     line 438:     (inner_classes() != NULL) && 
> !inner_classes()->is_shared()) {
>         Moving the NULL check to be first would be faster.
>         Also a little over parenthesized here.
>
>     line 444: if ((annotations() != NULL) && 
> !annotations()->is_shared()) {
>         A little over parenthesized here.
>
> None of my comments are critical so thumbs up if you're
> pushed for time.
>
> Dan
>
>
> On 10/10/13 7:59 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this fix for bug 8024667.  The problem occurred because 
>> function InstanceKlass::deallocate_contents() was erroneously 
>> deallocating memory that was in the CDS shared region. This change 
>> fixes the problem by checking if the memory is in the shared region 
>> before allowing the deallocation.  The change also added asserts as 
>> an additional check.
>>
>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8024667/ 
>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8024667/>
>>
>> Bug Link: https://bugs.openjdk.java.net/browse/JDK-8024667
>>
>> The fix was tested with JPRT and with UTE and JTREG tests with both 
>> -client and -server.  It was also tested using the failed tests that 
>> were reported in bugs 8024667 and 8026055.
>>
>> Thanks, Harold
>>
>