RFR: JDK-8131600: heapdump/JMapHeap EXCEPTION_ACCESS_VIOLATION

[Bengt Rutisson]

Hi everyone,

Could I have a couple of reviews for this patch to fix one of the 
critical nightly issues we are currently having?

http://cr.openjdk.java.net/~brutisso/8131600/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8131600

Background

When we load a class we first create the InstanceKlass and then allocate 
the Java mirror on the heap. The allocation of the Java mirror is just 
like any other allocation, so it can be blocked by a safepoint since it 
will in some cases try to take the heap lock.

This means that when we hit a safepoint there can potentially exist 
InstanceKlasses where ik->java_mirror() == NULL.

The DumperSupport::dump_class_and_array_classes() currently does not 
handle that case. It blindly calls ik->signers() on the klass it is 
supposed to dump. signers() looks up the mirror (java_class()) and 
passes it on:


objArrayOop InstanceKlass::signers() const {
   // return the signers from the mirror
   return java_lang_Class::signers(java_mirror());
}

...to java_lang_Class::signers(), which will crash if it gets NULL as 
java_class:

objArrayOop java_lang_Class::signers(oop java_class) {
   assert(_signers_offset != 0, "must be set");
   return (objArrayOop)java_class->obj_field(_signers_offset);
}

The dump_class_and_array_classes() is called as part of dumping the heap 
from a jmap call. This is obviously done at a safepoint. So, if the jmap 
call comes in between the creation of the InstanceKlass and the 
allocation of the mirror we end up calling 
dump_class_and_array_classes() with an InstanceKlass that has NULL as 
its mirror. This is the reason for the crash in JDK-8131600.


Except for DumperSupport::dump_class_and_array_classes() there is only 
one user of InstanceKlass::signers(). That is 
VM_HeapWalkOperation::iterate_over_class(). This method has an early 
exit if the class has not been properly initialized yet:


     // ignore the class if it's has been initialized yet
     if (!ik->is_linked()) {
       return true;
     }


So, I think the proper fix for JDK-8131600 is to make sure that 
DumperSupport::dump_class_and_array_classes() has the same check.

Now, DumperSupport::dump_class_and_array_classes() actually calls 
InstanceKlass::signers() twice. The second time around we don't have an 
InstanceKlass but just a Klass. The Klass does not have the is_linked() 
method. I haven't tried to provoke that code to fail, but I assume it 
has the same problem. So, I added a java_class() != NULL check to that 
code to make it handle this case as well.

Thanks,
Bengt