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

Coleen Phillimore coleen.phillimore at oracle.com
Wed Feb 20 09:37:55 PST 2013


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

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


More information about the serviceability-dev mailing list