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

Thu May 22 11:39:13 UTC 2014

Hi,

Bengt:

Right, that should be enough, thanks.

Mikael:

Can I use you as a reviewer for this latest version as well?

Regards,
Andreas

On 2014-05-22 13:02, Bengt Rutisson wrote:
>
> Hi Andreas,
>
> This is a HotSpot change and for JDK7 HotSpot was developed in the hsx 
> project. I am a Reviewer in the hsx project 
> (http://openjdk.java.net/census#brutisso) isn't that enough to review 
> this change?
>
> Anyway, last webrev looks good. Thanks for fixing the test!
>
> Bengt
>
> On 5/22/14 10:43 AM, Andreas Eriksson wrote:
>> Hi,
>>
>> Adding jdk7u-dev.
>>
>> Could I have a jdk7u Reviewer look at this as well please? This is a 
>> jdk7 only fix.
>> (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.03/
>> https://bugs.openjdk.java.net/browse/JDK-8038925
>>
>> Regards,
>> Andreas
>>
>> On 2014-05-22 10:14, Andreas Eriksson wrote:
>>> OK, I'll remove it.
>>>
>>> Thanks,
>>> Andreas
>>>
>>> On 2014-05-22 10:02, Bengt Rutisson wrote:
>>>>
>>>>
>>>> Hi Andreas,
>>>>
>>>> On 5/21/14 2:05 PM, Andreas Eriksson wrote:
>>>>> 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.
>>>>
>>>> Thanks for the update, Andreas.
>>>>
>>>> The test looks good except for the @run tag.
>>>>
>>>> @run main/othervm -Xms512m -Xmx1024m -XX:+UseG1GC 
>>>> -XX:+ExplicitGCInvokesConcurrent TestG1ConcurrentGCHeapDump
>>>>
>>>> The problem is that more GC flags will be added when the test is 
>>>> run in nightly testing. Some GC flags will conflict with UseG1GC. 
>>>> On the other hand at some point UseG1GC will be one of the flags 
>>>> that is added.
>>>>
>>>> So, I think what you need to do is just remove "-XX:+UseG1GC" from 
>>>> the @run tag. Then your test will report log "skipped" when it is 
>>>> run in the nightly testing except for the nightly testing done with 
>>>> G1 where it will actually do something.
>>>>
>>>> Bengt
>>>>
>>>>>
>>>>> 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140522/8316e40d/attachment.htm>