Review Request: 8006506: Add test for redefining methods in backtraces to java/lang/instrument tests

[serguei.spitsyn at oracle.com]

Stefan,

Thank you for adding comments!


Thanks,
Serguei


On 2/1/13 3:27 AM, Stefan Karlsson wrote:
> On 2013-02-01 12:17, Stefan Karlsson wrote:
>> On 2013-02-01 12:11, serguei.spitsyn at oracle.com wrote:
>>> On 2/1/13 1:57 AM, Stefan Karlsson wrote:
>>>> On 2013-02-01 10:22, serguei.spitsyn at oracle.com wrote:
>>>>> Nice test!
>>>>> It looks good.
>>>> Thanks for reviewing!
>>>>>
>>>>> As the original bug and the test are non-trivial, it'd make sense 
>>>>> to add a comment to
>>>>> the class RedefineMethodInBacktraceApp and explain a little bit 
>>>>> what the test is doing,
>>>>> and what behavior is expected.
>>>> http://cr.openjdk.java.net/~stefank/8006506/webrev.04/
>>>>
>>>> Tell me if you think this is good enough.
>>>
>>> It is good.
>>>
>>> Nit: it'd be enough if it is more specific. :)
>>> This method is a key point:
>>>    90     private static void touchRedefinedMethodInBacktrace(Throwable throwable) {
>>>    91         throwable.getStackTrace();
>>>    92     }
>>> Is it true that the test expects the getStackTrace() does not crash nor
>>> throw an exception which would happen if the old/obsolete method is 
>>> gc'ed?
>>
>> I see. I'll add a comment that we shouldn't crash.
>
> http://cr.openjdk.java.net/~stefank/8006506/webrev.05/
>
> StefanK
>
>>
>> Thanks,
>> StefanK
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>>
>>>> thanks,
>>>> StefanK
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Sergueri
>>>>>
>>>>>
>>>>> On 2/1/13 12:13 AM, Stefan Karlsson wrote:
>>>>>> http://cr.openjdk.java.net/~stefank/8006506/webrev.03/
>>>>>>
>>>>>> 1) Reverted the ProblemList change, since the fix has already 
>>>>>> propagaged to jdk8/tl
>>>>>> 2) Renamed do_redefine -> doRedefine
>>>>>> 3) Updated the .sh file with the bug number of the original CR 
>>>>>> instead of the test CR.
>>>>>>
>>>>>> thanks,
>>>>>> StefanK
>>>>>>
>>>>>> On 2013-01-22 14:11, Stefan Karlsson wrote:
>>>>>>> http://cr.openjdk.java.net/~stefank/8006506/webrev.00/
>>>>>>>
>>>>>>> This test provokes the JVM crash described in bug: JDK-7174978.
>>>>>>>
>>>>>>> I intend to push this to:
>>>>>>> http://hg.openjdk.java.net/jdk8/tl/jdk
>>>>>>>
>>>>>>> thanks,
>>>>>>> StefanK
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

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