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

[Daniel D. Daugherty]

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.

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
>