RFR (S) 8205608: Fix 'frames()' in ThreadReferenceImpl.c to prevent quadratic runtime behavior

[Volker Simonis]

Hi Sergey,

thanks for your help, but I've just pushed the fix now.

@Thomas: sorry, I really apologize, but I've just realized that I've
forgot to add you as a Reviewer :( I'll promise to look more carefully
next time.

Regards,
Volker


On Tue, Jul 24, 2018 at 6:01 PM, serguei.spitsyn at oracle.com
<serguei.spitsyn at oracle.com> wrote:
> Hi Ralf,
>
> I think, you have to consider it reviewed.
> Sorry, I was not clear no new webrev is needed.
>
> Do you need a sponsor for the push?
>
> Thanks,
> Serguei
>
>
>
> On 7/24/18 06:32, Schmelter, Ralf wrote:
>>
>> Hi all,
>>
>> here is the update webref with the fixed copyright:
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8205608.v5/
>>
>> Best regards,
>> Ralf
>>
>> -----Original Message-----
>> From: serguei.spitsyn at oracle.com [mailto:serguei.spitsyn at oracle.com]
>> Sent: Freitag, 20. Juli 2018 23:04
>> To: Chris Plummer <chris.plummer at oracle.com>; Schmelter, Ralf
>> <ralf.schmelter at sap.com>; serviceability-dev at openjdk.java.net; Stuefe,
>> Thomas <thomas.stuefe at sap.com>
>> Subject: Re: RFR (S) 8205608: Fix 'frames()' in ThreadReferenceImpl.c to
>> prevent quadratic runtime behavior
>>
>> On 7/20/18 13:44, Chris Plummer wrote:
>>>
>>> On 7/20/18 1:40 PM, serguei.spitsyn at oracle.com wrote:
>>>>
>>>> Hi Ralf,
>>>>
>>>>
>>>> On 7/20/18 07:28, Schmelter, Ralf wrote:
>>>>>
>>>>> Hi Sergue,
>>>>>
>>>>> I’ve updated the webref:
>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8205608.v4/
>>>>
>>>> The copyright year in ThreadReferenceImpl.c still has to be 2018, not
>>>> 2008.
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8205608.v4/test/jdk/com/sun/jdi/Frames2Test.java.html
>>>>
>>>>
>>>>    72             if (newDepth == -1_000) {
>>>>    73                 // Pop some frames so there is room on the stack
>>>> for the
>>>>    74                 // call (including println()).
>>>>    75                 notifyRecursionEnded();
>>>>    76             }
>>>>
>>>>    I have a concern on potential issue mentioned in the comment above.
>>>>    Should a StackOverflowError be expected here?
>>>>
>>>>    79         } catch (StackOverflowError e) {
>>>>    80             // Use negative depth to indicate the recursion has
>>>> ended.
>>>>    81             return -1;
>>>>    82         }
>>>>
>>>>    What is going to happen if the StackOverflowError was really caught
>>>> above?
>>>
>>> The SOE is really caught in the above code. I returns -1, and starts
>>> the unwinding of the stack. After 1000 frames have been popped via
>>> returns, notifyRecursionEnded() will be called. The pops are so
>>> notifyRecursionEnded() can be called without worry of another SOE.
>>
>> Got it, thanks Chris.
>>
>> So, I'm Okay with the fix assuming the copyright year is fixed.
>>
>> Thanks,
>> Serguei
>
>