Review Request (S) 8017230: Internal Error (jvmtiRedefineClasses.cpp:1662): guarantee(false) failed: insert_space_at() failed

[serguei.spitsyn at oracle.com]

Thank you for the review!
Serguei

On 9/13/13 4:21 AM, David Holmes wrote:
> On 13/09/2013 5:21 AM, serguei.spitsyn at oracle.com wrote:
>> On 9/11/13 8:54 PM, David Holmes wrote:
>>> Hi Dmitry,
>>>
>>> It seems odd that you install the new_method even if there was an
>>> exception. What if the new_method is not valid because of the 
>>> exception ?
>>
>> Coleen suggested this fragment.
>> New methods will be deallocated with the scratch class in a case of
>> exception.
>> It is handled in the doit_prologue where the scratch classes are added
>> to the CL deallocation list.
>
> Ok - I'll take Coleen's word on that ;-)
>
>>>
>>> Also once you've cleared the exception and returned false, the user
>>> has no information as to why this failed. I understand we don't want
>>> to hit the guarantee here, but it seems there is a hole in the error
>>> flow.
>>
>> This issue is fixed in a separate bug fix for 8024346 (see another
>> review request).
>> Sorry for the confusion here.
>
> Okay.
>
> Thanks,
> David
>
>> The whole error flow is not perfect but I'm not targetting to make it
>> perfect now.
>> Multiple bugs on the limited Metaspace topic were filed by Stefan:
>> 8017230, 8024345, 8024346.
>> My role is to apply/test fixes suggested by Stefan and Coleen in the
>> order the issues were discovered.
>>
>>
>> Thanks,
>> Serguei
>>
>>>
>>> David
>>>
>>> On 12/09/2013 7:39 AM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review the fix for:
>>>>    bug: http://bugs.sun.com/view_bug.do?bug_id=8017230
>>>>    jbs: https://bugs.openjdk.java.net/browse/JDK-8017230
>>>>
>>>>
>>>> Open webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8017230-JVMTI-MEM.1 
>>>>
>>>>
>>>>
>>>> Summary:
>>>>    Handle pending exceptions instead of firing a guarantee() in the
>>>> JVMTI rewrite_cp_refs_in_method().
>>>>
>>>>
>>>> Testing:
>>>>    UTE tests - in progress:  vm.quick-pcl.testlist with limited
>>>> Metaspace memory,
>>>>                                          nsk.jvmti.testlist,
>>>> nsk.jdi.testlist,
>>>>                                          Jtreg java/lang/instrument
>>>>
>>>> Thanks,
>>>> Serguei
>>