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

[Ron Durbin]

I will take another look 

 

From: Daniel D. Daugherty 
Sent: Wednesday, February 20, 2013 11:54 AM
To: Ron Durbin
Cc: Coleen Phillimore; serviceability-dev at openjdk.java.net; Christian Törnqvist; hotspot-runtime-dev at openjdk.java.net
Subject: Re: Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly

 

Ron,

I stand corrected on one part. You added a note to
the bug about the 'ErrorHandlerTest' option back on
2013.02.11. I don't see any notes about vmerrors.sh,
but maybe that's less important.

Dan



On 2/20/13 11:47 AM, Ron Durbin wrote:

Dan my cage is rattled

 

Yes I should give more detail on that .  I might still be able to add some detail in the "we tried this  too".

I was not thinking about others needing to understand /follow the road to resolution.

 

 

From: Coleen Phillimore 
Sent: Wednesday, February 20, 2013 10:38 AM
To: Daniel Daugherty
Cc: HYPERLINK "mailto:serviceability-dev at openjdk.java.net"serviceability-dev at openjdk.java.net; HYPERLINK "mailto:=?ISO-8859-1?Q?Christian_T=F6rnq?=@sca-git1.sun.com"=?ISO-8859-1?Q?Christian_T=F6rnq?=@sca-git1.sun.com; HYPERLINK "mailto:hotspot-runtime-dev at openjdk.java.net"hotspot-runtime-dev at openjdk.java.net
Subject: Re: Code Review fix for 6799919 Recursive calls to report_vm_out_of_memory are handled incorrectly

 

On 2/20/2013 12:05 PM, Daniel D. Daugherty wrote:

On 2/20/13 9:57 AM, Daniel D. Daugherty wrote:




On 2/20/13 9:34 AM, Coleen Phillimore wrote:





This looks good.


Thanks for the review! Don't know if Ron is having the same connectivity
problems I'm having this morning, but my access into OWAN is going up
and down...






It looks like the webrev was updated to get rid of the unused variable, so that is good.


The webrev was not updated.


Yes, I see that now.  Mikael has a much better eye than I do.










Is there a test for ErrorHandlerTest in our repository already?


ErrorHandlerTest? Is there yet another possible test that we don't
know about?


OK. I know that test by a different name:

$ rgrep ErrorHandlerTest agent make src test
src/share/vm/runtime/globals.hpp:  notproduct(uintx, ErrorHandlerTest, 0,                                    \
src/share/vm/prims/jni.cpp:  NOT_PRODUCT(test_error_handler(ErrorHandlerTest));
test/runtime/6888954/vmerrors.sh:        -XX:ErrorHandlerTest=${i} -version > ${i2}.out 2>&1
test/runtime/6888954/vmerrors.sh:    # If ErrorHandlerTest is ignored (product build), stop.
test/runtime/6888954/vmerrors.sh:            echo "ErrorHandlerTest=$i failed ($f)"

Ron had previously explored using vmerror.sh to exercise the
vm_exit_out_of_memory() code path as one of his early experiments.
He also did some testing using the ErrorHandlerTest command line
option. In neither case did it seem possible to get multi-threaded
test cases up and running. Perhaps both he and I missed something.

It also looks like Ron didn't record the specifics of his testing
with either vmerrors.sh or the ErrorHandlerTest command line option
in the bug report. I'll have to rattle his cage about that.


My question was mostly if we had a jtreg test in hotspot/test repository that exercised this ErrorHandlerTest option.   The second part of the question is whether we should have a test because it'll look like it failed.   Maybe this is more of a question for Christian Tornqvist and SQE and is not tied to this specific change.

I talked to Ron about trying to test this also and we couldn't come up with a good strategy either.   We need a good way to test out of C heap memory without actually allocating lots of memory and running out of C heap.  Such tests don't run well.  Maybe we can do something in the future with this ErrorHandlerTest option to have the VM return NULL or assert for various malloc calls triggered through some heuristic.   Having the experiments to date recorded somewhere would be great.

Thanks,
Coleen





Dan






Dan







Thanks,
Coleen

On 2/19/2013 6:48 PM, 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 HYPERLINK "mailto:ron.durbin at oracle.com"ron.durbin at oracle.com 

Here is the webrev URL: 

HYPERLINK "http://cr.openjdk.java.net/%7Edcubed/for_rdurbin/6799919-webrev/0-hsx25"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 

 

 

 

 

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130220/0857e86e/attachment-0001.html