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

[Bengt Rutisson]

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
>>
>