Request for review: 7110428 Crash during HeapDump operation

[David Holmes]

As a short-term quick-fix I'm okay with this but I defer to the GC 
folks. If similar code is already being used then this seems reasonable.

David

On 11/11/2011 5:14 PM, poonam.bajaj at oracle.com wrote:
> Hi David,
>
> On 11/11/2011 11:48 AM, David Holmes wrote:
>> Hi Poonam,
>>
>> I've cc'ed GC-dev as this is more their area. It seems very wasteful
>> to potentially ensure_parsability and also do a full GC. And the check
>> for the active GC_Locker is somewhat arbitrary as it could change the
>> instant you've queried it.
>>
> Thanks for adding GC-dev.
>
>> It seems to me that a more complete solution here would enable
>> collect_as_vm_thread to indicate whether or not GC occurred. Which in
>> turn requires that do_full_collection report that info - a RFE that is
>> already noted in the G1 code at least:
>>
>> void G1CollectedHeap::do_full_collection(bool clear_all_soft_refs) {
>>   // do_collection() will return whether it succeeded in performing
>>   // the GC. Currently, there is no facility on the
>>   // do_full_collection() API to notify the caller than the collection
>>   // did not succeed (e.g., because it was locked out by the GC
>>   // locker). So, right now, we'll ignore the return value.
>>   bool dummy = do_collection(true,                /* explicit_gc */
>>                              clear_all_soft_refs,
>>                              0                    /* word_size */);
>> }
>>
> Yes, that would be the ideal solution.
>
> What is the time frame of having this RFE implemented for other
> collectors? This bug was discovered during the FMW stress testing and
> needs to fixed as soon as we can.
>
> The solution I proposed is being used by other VM operations as well
> e.g. VM_GC_HeapInspection and was somehow missed for VM_HeapDumper.. So,
> I guess until we have the RFE implemented that allows us to have the
> result whether a GC succeeded  or not, this could be a short term solution.
>
>
> Thanks,
> Poonam
>
>
>> David
>> -----
>>
>> On 11/11/2011 3:20 PM, poonam.bajaj at oracle.com wrote:
>>> Hi,
>>>
>>> Can I get a couple of reviews for this change:
>>>
>>> CR 7110428: Crash during HeapDump operation
>>> http://monaco.sfbay.sun.com/detail.jsf?cr=7110428
>>>
>>> The crash happens when VM thread traverses the heap during heapdump
>>> operation and it finds the heap in non-parseable state. The heap can be
>>> left in non-parseable state if due to some reason GC is skipped and we
>>> didn't call ensure_parsability() either before dumping the heap. The
>>> solution is to always call ensure_parsability() In
>>> VM_HeapDumper::doit(), whether or not a GC is requested before the heap
>>> dumping.
>>>
>>> http://cr.openjdk.java.net/~poonam/7110428/webrev.00/
>>>
>>>
>>> Thanks,
>>> Poonam
>>>
>>>
>
> --
> Best regards, Poonam
>
> Oracle <http://www.oracle.com>
> Poonam Bajaj | Principal Member of Technical Staff
> Phone: +91 80 66937451 <tel:+91%2080%2066937451> | Mobile: +91
> 9844511366 <tel:+91%209844511366>
> Oracle JVM Sustaining Engineering
>
> ORACLE India Bangalore
> Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
> developing practices and products that help protect the environment
>