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

[serguei.spitsyn at oracle.com]

On 1/18/15 9:47 PM, David Holmes wrote:
> On 17/01/2015 12: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?)...
>
> Sorry I didn't follow this through all the rounds, but the bug report 
> states (and the code I looked at seems to be consistent with it) that 
> the throw_IllegalAccessError call is only added to itables - so why 
> are we also walking vtables?

The Runtime team is going to extend use of the throw_IllegalAccessError 
for vtables as well.

Thank you for looking at the fix!
Serguei

>
> David H.
> -----
>
>>      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...
>>
>>      Is there some way to limit this visit to classes where the
>>      magical method can appear? Or can it really appear anywhere?
>>
>> 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
>>>>
>>>
>>