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

[Daniel D. Daugherty]

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