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

[Daniel D. Daugherty]

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 lessimportant.

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:* serviceability-dev at openjdk.java.net; 
> =?ISO-8859-1?Q?Christian_T=F6rnq?=@sca-git1.sun.com; 
> 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 ron.durbin at oracle.com <mailto:ron.durbin at oracle.com>
>
>     Here is the webrev URL:
>
>     http://cr.openjdk.java.net/~dcubed/for_rdurbin/6799919-webrev/0-hsx25
>     <http://cr.openjdk.java.net/%7Edcubed/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/1e1c5c20/attachment.html