Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly

[Coleen Phillimore]

On 2/20/2013 10:34 AM, Daniel D. Daugherty wrote:
> I've addeda 6799919_code_history.eml attachment to the bug report. 
> Here is
> the interesting part of the putback message:
>
> > 5073464: Can we improve error handling for create_itable_stub 
> allocation
> >
> > Previously many CodeCache allocations would fatal() if no storage was
> > available.  Updated these sites to call vm_exit_out_of_memory() instead
> > so that the "Out of swap space?" warning will be displayed to encourage
> > user diagnosis.  Also updated vm_exit_out_of_memory and the vmError 
> "out of
> > memory" path to display the file/line of the caller, like fatal() does
> > already.
>
> I found the review request for 5073464 from 2005.07.28 and it contains
> virtually the same info as the putback message above. I didn't find any
> other e-mails on the review thread so any review discussion must have
> taken place in private e-mails to Steve B.
>
> I can find no indications that the redundant recursion handling was added
> by Steve B. to avoid any particular issue with the existing mechanism.
> Given the complexity of VMError::report_and_die() which is 237 lines 
> long,
> it is possible that Steve didn't realize that it already handled 
> recursion.

The last time I looked at this code, I came to this same conclusion.   
VMError::report_and_die() handles the recursive case. 
vm_exit_out_of_memory doesn't need to (and if it does, it'll not leave 
an hs_err file).
Coleen

>
> Dan
>
>
> On 2/19/13 10:04 PM, Daniel D. Daugherty wrote:
>> David,
>>
>> Thanks for the review!
>>
>> I had previously done a code history analysis for the offending code
>> block as an example for Ron on how that is done in this crazy multi
>> source code control system we call Java... Yes, this went from Mercurial
>> back into TeamWare and I had to find the old rt_baseline workspace...
>>
>> We'll dig it up tomorrow and attach it to the bug report along with
>> other analysis if needed.
>>
>> Dan
>>
>>
>> On 2/19/13 9:41 PM, David Holmes wrote:
>>> Okay full disclosure. :) I filed this bug back in 2009 so I 
>>> undoubtedly ran into a failure where this guard was causing the 
>>> useful error information to be lost, and so I filed the bug.
>>>
>>> Now I am so much older and wiser I'm wondering about the original 
>>> reason for the guard again. ;-)
>>>
>>> David
>>>
>>> On 20/02/2013 2:23 PM, David Holmes wrote:
>>>> The one-reporter only logic in report_and_die has been there for a 
>>>> very
>>>> long time, pre-dating the addition of the check in
>>>> report_vm_out_of_memory. So I have to wonder what prompted the 
>>>> inclusion
>>>> of that guard originally? No doubt there was some failure case 
>>>> where we
>>>> were getting recursive errors that just totally broke the error 
>>>> reporting.
>>>>
>>>> So while I can give this the Reviewer's tick if still needed. I 
>>>> suspect
>>>> that this will be a case of fixing one specific example, but in the
>>>> process potentially breaking others.
>>>>
>>>> David
>>>>
>>>> On 20/02/2013 9:48 AM, Daniel D. Daugherty wrote:
>>>>> Greetings,
>>>>>
>>>>> I'm sponsoring this code review request from Ron Durbin. This change
>>>>> is targeted at JDK8/HSX-25 in the RT_Baseline repo.
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> I have a proposed fix for the following bug:
>>>>>
>>>>>      6799919 Recursive calls to report_vm_out_of_memory are handled
>>>>> incorrectly
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6799919
>>>>>      https://jbs.oracle.com/bugs/browse/JDK-6799919
>>>>>
>>>>> This is one of those bug fixes where the commit message nicely 
>>>>> describes
>>>>> the change:
>>>>>
>>>>> 6799919: Recursive calls to report_vm_out_of_memory are handled
>>>>> incorrectly
>>>>> Summary: report_vm_out_of_memory() should allow 
>>>>> VMError.report_and_die()
>>>>> to handle multiple out of native memory errors.
>>>>> Reviewed-by: dcubed, <other-reviewers>
>>>>> Contributed-by ron.durbin at oracle.com
>>>>>
>>>>> Here is the webrev URL:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/for_rdurbin/6799919-webrev/0-hsx25
>>>>>
>>>>> Testing:
>>>>>     - See the READ_ME file attached to the JDK-6799919 for the gory
>>>>> details
>>>>>       of the testing needed to reproduce this failure and verify 
>>>>> the fix
>>>>>     - regular JPRT test job is in process
>>>>>
>>>>> Comments, questions and suggestions are welcome.
>>>>>
>>>>> Ron
>>
>