RFR: JDK-8134030: test/serviceability/dcmd/gc/HeapDumpTest fails to verify the dump

[Andreas Eriksson]

On 2015-11-03 07:27, David Holmes wrote:
> Hi Andreas,
>
> On 31/10/2015 12:03 AM, Andreas Eriksson wrote:
>> Hi,
>>
>> Please review this change to heap dump logic.
>> Including runtime list, since this is related to class loading.
>>
>> Bug:
>> JDK-8134030: test/serviceability/dcmd/gc/HeapDumpTest fails to verify
>> the dump
>> https://bugs.openjdk.java.net/browse/JDK-8134030
>>
>> Webrev: http://cr.openjdk.java.net/~aeriksso/8134030/webrev.00/
>>
>> A heap dump can occur at a point where an InstanceKlass doesn't have a
>> Java class mirror yet.
>> To avoid a crash because of this JDK-8131600 [1] excluded classes that
>> are not linked yet from the dump.
>>
>> The problem here is that since a class that has been loaded is already
>> exposed to the Java side, another class can have a reference to the
>> excluded class.
>> So, if it has been *loaded* but not *linked* it will be excluded, but
>> can still be referenced by other classes in the dump.
>
> I'm having trouble seeing exactly when the class is marked as "loaded" 
> but it certainly seems to be after the mirror has been set. However 
> linking happens even later than that - so if you still see a crash 
> excluding linked classes then I fear you will also see problems 
> excluding loaded classes!
>
> David
> -----
>

Hi,

The crash does not happen when excluding linked classes, sorry if that 
was unclear.

Let me try to explain it again:
The crash was fixed by excluding non-linked classes (in JDK-8131600).
But that excludes some classes that are already referenced in the heap 
dump by other classes.
By changing to excluding non-loaded classes, the crash can still not 
happen, and all classes that are referenced in the dump are also included.

Not dumping these classes is only a problem when we try to look at the 
heap dump at a later point (and even then it is only a problem of 
missing information, not a crash).
For example, HeapDumpTest gives the following warning when it tries to 
follow a reference to a class that was excluded from the dump: WARNING: 
Failed to resolve object id 0x6c003d348 [...].
VisualVM on the other hand will ignore the value, and tell the user the 
field was null (which is very misleading).

I hope that made sense.

Thanks,
Andreas

>
>> This gives the following warning in HeapDumpTest:
>> WARNING: Failed to resolve object id 0x6c003d348 [...].
>>
>> This fix changes heapDumper.cpp to only exclude classes that are not yet
>> loaded instead.
>>
>> I'd like confirmation on this from someone who knows more about loaded
>> vs linked, but I think that this is correct:
>> When a class has been loaded, we are guaranteed that a Java class mirror
>> is setup, so the crash seen in JDK-8131600 still cannot happen.
>> This seems to be confirmed by the manual reproducer from JDK-8131600,
>> which still succeeds with this fix.
>>
>> Also in this change is the re-addition of the actual heap dump
>> verification call in HeapDumpTest, which was accidentally removed when
>> the test was re-factored.
>>
>> Regards,
>> Andreas
>>
>> [1]: https://bugs.openjdk.java.net/browse/JDK-8131600