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
