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

[Daniel D. Daugherty]

Adding back the other alias...

Coleen,

Thanks for chiming in. May we add you as a reviewer also?

Dan


On 2/20/13 8:56 AM, Coleen Phillimore wrote:
> 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
>>>
>>
>