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

[serguei.spitsyn at oracle.com]

On 9/12/13 12:58 PM, Stefan Karlsson wrote:
> On 9/12/13 9:21 PM, 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.
>>
>>>
>>> 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.
>>
>> 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.
>>
> Just understand that my suggested patches were just temporary patches 
> to get me past the bugs, not final fixes.

Yes, of course.
It is my resp. to double-check and make it final.
But you deserved the credit for this investigation anyway. :)

Thanks,
Serguei

>
> thanks,
> StefanK
>>
>> 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
>>
>