RFR (S) 7179701: MaxJavaStackTraceDepth of zero is not handled correctly/consistently in the VM

[coleen.phillimore at oracle.com]

Thanks Harold,
Coleen

On 1/31/18 9:31 AM, harold seigel wrote:
> Your proposed new 'for' loop is better.
>
> The change looks good.
>
> Thanks, Harold
>
>
> On 1/30/2018 10:44 AM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 1/30/18 9:44 AM, Daniel D. Daugherty wrote:
>>> On 1/30/18 9:39 AM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> On 1/29/18 8:02 PM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> Wow this one is a blast from the past! :)
>>>>
>>>> Yeah, cleaning up some backlog ...
>>>>>
>>>>> On 30/01/2018 9:54 AM, coleen.phillimore at oracle.com wrote:
>>>>>> Summary: Value of zero means unlimited stack trace.  If you want 
>>>>>> no stack trace, use -XX:-StackTraceInThrowable
>>>>>>
>>>>>> I think I broke this with rework to Throwable 
>>>>>> fill_in_stack_trace. Tested with jvmti, jdi and stack stress 
>>>>>> tests.   Also tested manually with version of the new test with 
>>>>>> kill -3 to test the thread.cpp code.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/7179701.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-7179701
>>>>>
>>>>> Overall looks fine. One nit ...
>>>>>
>>>>> src/hotspot/share/classfile/javaClasses.cpp
>>>>>
>>>>> This looks really awkward for a loop condition:
>>>>>
>>>>> 1971   for (frame fr = thread->last_frame(); max_depth > 0 ? 
>>>>> max_depth != total_count : true;) {
>>>>>
>>>>> how about changing:
>>>>>
>>>>> 1936   int max_depth = MaxJavaStackTraceDepth;
>>>>>
>>>>> to
>>>>>
>>>>> 1936   int max_depth = MaxJavaStackTraceDepth > 0 ?
>>>>>                        MaxJavaStackTracedepth : INT_MAX;
>>>>>
>>>>> instead? Or, in the style of the other changes:
>>>>>
>>>>> 1971   for (frame fr = thread->last_frame();
>>>>>             max_depth == 0 || max_depth < total_count;) {
>>>>
>>>> I'll change it to like this, which does look nicer:
>>>>   for (frame fr = thread->last_frame(); max_depth == 0 || max_depth 
>>>> != total_count;) {
>>>>
>>>>>
>>>>> ?
>>>>>
>>>>> ---
>>>>>
>>>>> src/hotspot/share/prims/forte.cpp
>>>>>
>>>>> I'm wondering what tests actually hit this code?
>>>>>
>>>>
>>>> I couldn't find any, and don't really know how to test this code, 
>>>> which is why I ran a lot of serviceability tests.
>>>
>>> AsyncGetCallTrace() is exercised by the analyzer tool. I'm not
>>> aware of any open (or closed) test suites that exercise it.
>>
>> Thanks, Dan. I  did a search for any tests containing 
>> AsyncGetCallTrace() and didn't find any.
>>
>> Coleen
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>
>>>
>>
>