Request for review: 7110428 Crash during HeapDump operation

[Srinivas Ramakrishna]

I tend to agree with David that it's cleaner to have collect_as_vm_thread()
return an indication of
whether GC was done or not, and for the caller to then make the heap
parsable if GC was skipped
and the plan is to subsequently walk the heap regardless.

I also appreciate that what Poonam proposed may be the short-term expedient
fix, and in that
case the fix looks OK to me as a temporary solution (i can imagine that
with very large Edens and
many, many threads on a highly multi-core server just walking the threads
twice is an unnecessary
cost); reviewed.

thanks.
- ramki (openjdk: ysr)

On Thu, Nov 10, 2011 at 11:36 PM, David Holmes <david.holmes at oracle.com>wrote:

> 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<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/<http://cr.openjdk.java.net/%7Epoonam/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<http://www.oracle.com/commitment>>
>> Oracle is committed to
>>
>> developing practices and products that help protect the environment
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20111111/ecdcbc1c/attachment.htm>