RFR: JDK-8131600: heapdump/JMapHeap EXCEPTION_ACCESS_VIOLATION

[Bengt Rutisson]

Hi Thomas,

On 2015-07-23 08:54, Thomas Schatzl wrote:
> Hi Bengt,
>
> On Thu, 2015-07-23 at 08:37 +0200, Bengt Rutisson wrote:
>> On 2015-07-22 20:03, Kim Barrett wrote:
>>> On Jul 22, 2015, at 12:11 PM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>>>> On 2015-07-22 17:11, Kim Barrett wrote:
>>>>> On Jul 22, 2015, at 8:12 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>>>>>> 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
>>>>>> […]
>>>>>> 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.
>>>>> I think this change shouldn't be needed.
>>>>>
>>>>> The klass involved is the result of array_klass_or_null(). That
>>>>> shouldn't be able to return an incompletely initialized (array) klass.
>>>> Yes, I was discussing this a bit with Mikael and we weren't really sure. I think you are right, I'm just a bit worried about how fragile the class initialization seems to be.
>>>>
>>>> But I'll remove this part of the fix. Let's just keep our eyes open if a crash in this area comes up again.
>>> With that part backed out, change looks good to me.
>> Thanks, Kim!
>>
>> Here's an updated webrev:
>> http://cr.openjdk.java.net/~brutisso/8131600/webrev.01/
>    looks good.

Thanks for the review!

Bengt

>
> Thomas
>
>