Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Feb 20 07:34:13 PST 2013

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 rtbaseline 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 reportanddie has been there for a very long time, pre-dating the addition of the check in reportvmoutofmemory. 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 RTBaseline repo. Dan

I have a proposed fix for the following bug: 6799919 Recursive calls to reportvmoutofmemory are handled incorrectly http://bugs.sun.com/bugdatabase/viewbug.do?bugid=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 reportvmoutofmemory are handled incorrectly Summary: reportvmoutofmemory() should allow VMError.reportanddie() to handle multiple out of native memory errors. Reviewed-by: dcubed, Contributed-by ron.durbin at oracle.com Here is the webrev URL: http://cr.openjdk.java.net/~dcubed/forrdurbin/6799919-webrev/0-hsx25 Testing: - See the README 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

More information about the serviceability-dev mailing list