RFR: 8038925: Java with G1 crashes in dump_instance_fields using jmap or jcmd without fullgc

[Andreas Eriksson]

A new webrev is up:
http://cr.openjdk.java.net/~aeriksso/8038925/webrev.02/

The test now verifies that no full GC has been done after doing the heap 
dump.
I also modified the test to not run if it for some reason is not using G1.

Regards,
Andreas

On 2014-05-21 12:31, Bengt Rutisson wrote:
> On 5/21/14 12:12 PM, Andreas Eriksson wrote:
>> Hi Bengt, thanks for looking at this.
>>
>> I agree that verifying that no full GC has happened would be good.
>> One thing I see as problematic though, is what to do if a full GC has 
>> happened.
>> Should I make the test fail? Or is there some way to signal that the 
>> test was inconclusive / couldn't finish?
>
> Personally I would prefer to make the test fail. JTreg only has two 
> states, PASS or FAIL, and I think that if we allow it to pass we might 
> not notice if there is some change that makes the test always get a 
> full GC and then never actually testing anything.
>
> I don't think it will fail intermittently by getting full GCs. I think 
> the test is pretty stable. But I think it would be good to have a way 
> of noticing if it stops testing what it is supposed to test.
>
> (By the way, I would really like a "SKIPPED" state in JTreg but I 
> haven't had any luck pushing for that. I think it could be useful for 
> other things as well. There is for example no good reason for your 
> test to be run 4 times each night with the exact same binary. But that 
> is what happens since we can't say that we should skip this test if we 
> run with any other GC than G1.)
>
> Thanks,
> Bengt
>
>>
>> Regards,
>> Andreas
>>
>> On 2014-05-21 11:55, Bengt Rutisson wrote:
>>>
>>> Hi Andreas,
>>>
>>> The fix looks good.
>>>
>>> One comment about the test. It does not verify that no full GC 
>>> happens. The way the test is set up I guess that should not happen 
>>> and I am not sure it is worth the effort to add a check for it. Just 
>>> wanted to mention it if you want to make test more resilient to 
>>> future changes in the JVM that for some reason can trigger a full GC 
>>> for this test.
>>>
>>> I'm fine with leaving the test as it is.
>>>
>>> Thanks,
>>> Bengt
>>>
>>>
>>> On 5/20/14 4:26 PM, Andreas Eriksson wrote:
>>>> Hi,
>>>>
>>>> Could I have a review for this G1 jdk7 only fix please?
>>>> (There is a related problem in jdk8 and jdk9, where an assert can 
>>>> fail because of this problem. I have logged another bug for this.)
>>>>
>>>> Description:
>>>> Due to the marking cleanup reclaiming empty regions, and having 
>>>> stale references a crash can occur when doing a heap dump.
>>>> The code tries to do an is_klass check on the object, which crashes 
>>>> the VM.
>>>> The fix is to add an is_perm check before doing the check, since 
>>>> is_perm will do a bounds check on the oop and if it's in the perm 
>>>> gen we know it's safe to look at it since G1 only ever does full 
>>>> compactions of the perm gen.
>>>>
>>>> For more information, and a more in-depth analysis, please see the 
>>>> jira bug.
>>>>
>>>> http://cr.openjdk.java.net/~aeriksso/8038925/webrev.01/
>>>> https://bugs.openjdk.java.net/browse/JDK-8038925
>>>>
>>>> Regards,
>>>> Andreas
>>>
>>
>