4-th round RFR (XS) 8068162: jvmtiRedefineClasses.cpp: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

[serguei.spitsyn at oracle.com]

Coleen and Dan,

Thanks!
Serguei

On 1/23/15 8:51 AM, Coleen Phillimore wrote:
>
> This still looks good to me.
> Coleen
>
> On 1/23/15, 9:06 AM, Daniel D. Daugherty wrote:
>> Serguei,
>>
>> Sorry I forgot to close the loop on this review.
>>
>> I'm OK with the answers below. Thumbs up.
>>
>> Dan
>>
>>
>> On 1/19/15 10:22 AM, serguei.spitsyn at oracle.com wrote:
>>> Coleen,
>>>
>>> Thank you for answering questions below!
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 1/19/15 7:55 AM, Coleen Phillimore wrote:
>>>>
>>>> On 1/16/15, 9:24 PM, Daniel D. Daugherty wrote:
>>>>> > 
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8068162-JVMTI-old.4/
>>>>>
>>>>> src/share/vm/memory/universe.hpp
>>>>>     No comments.
>>>>>
>>>>> src/share/vm/memory/universe.cpp
>>>>>     No comments.
>>>>>
>>>>> src/share/vm/prims/jvmtiRedefineClasses.cpp
>>>>>     So redefining the Unsafe class is now very expensive because
>>>>>     we have to visit the i-table and v-table of every class (and
>>>>>     maybe interface?)...
>>>>>
>>>>>     Based on the bug report 'Unsafe::throw_illegal_access' is some
>>>>>     magical method that can appear in any i-table or v-table entry.
>>>>>     Maybe only as part of some default methods thing? That's not
>>>>>     clear to me so I'm just guessing...
>>>>
>>>> True.
>>>>>
>>>>>     Is there some way to limit this visit to classes where the
>>>>>     magical method can appear? Or can it really appear anywhere?
>>>>
>>>> The Unsafe methods can appear in any itable now.   I don't know of 
>>>> a way to limit this.  Fortunately, redefining Unsafe seems to be an 
>>>> unusual thing to do, except for this stress test.
>>>>
>>>> Coleen
>>>>
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> On 1/16/15 12:14 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> Dan, David H. or David C.,
>>>>>>
>>>>>> May I ask one of you to look at the webrev below?
>>>>>> The issue itself is a little bit tricky, so it is not easy to 
>>>>>> review despite the small size.
>>>>>>
>>>>>> Coleen,
>>>>>>
>>>>>> Does the webrev matches what we discussed with you?
>>>>>> Do you give me a thumbs up?
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>> May I ask
>>>>>>
>>>>>> On 1/13/15 9:47 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>> Please, review the fix for:
>>>>>>>   https://bugs.openjdk.java.net/browse/JDK-8068162
>>>>>>>
>>>>>>>
>>>>>>> Open webrevs:
>>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8068162-JVMTI-old.4/ 
>>>>>>>
>>>>>
>>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8068162-Test-IsModifiableAgent/ 
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Summary:
>>>>>>>
>>>>>>>    The sun.misc.Unsafe:throwIllegalAccessError() method is used 
>>>>>>> in place of a default
>>>>>>>    interface method in the itable if a default method was not 
>>>>>>> defined in the interface.
>>>>>>>    In fact, it happens for two interfaces that purhaps are 
>>>>>>> auto-generated:
>>>>>>>      java/nio/CharBuffer
>>>>>>>      java/nio/HeapCharBuffer
>>>>>>>
>>>>>>>    This approach creates a problem when the class 
>>>>>>> sun.misc.Unsafe is retransformed.
>>>>>>>    The Method* pointer to the old (redefined) method in the 
>>>>>>> itable triggers an assert
>>>>>>>    (see the hs_err log in the bug report).
>>>>>>>    Coleen told me that a similar approach is going to be 
>>>>>>> implemented for some vtable entries.
>>>>>>>    Coleen, thanks for suggesting a better fix for this issue!
>>>>>>>
>>>>>>>    The fix is to replace the old Unsafe method in the 
>>>>>>> itable/vtable with the latest method version.
>>>>>>>
>>>>>>>
>>>>>>> Testing:
>>>>>>>   In progress: nsk.jdi.testlist, JTREG java/lang/instrument tests
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>