Request for review: 8001341/8004223: SIGSEGV in methodOopDesc::fast_exception_handler_bci_for(KlassHandle, int, Thread*)+0x3e9

[Jiangli Zhou]

Hi Chris,

Thanks for the review again.

Jiangli

On 01/07/2013 04:03 PM, Christian Thalinger wrote:
> On Jan 7, 2013, at 3:25 PM, Jiangli Zhou<jiangli.zhou at oracle.com>  wrote:
>
>> Hi Chris,
>>
>> I ended up with adding the following comments in JvmtiExport::post_exception_throw() in src/share/vm/prims/jvmtiExport.cpp, since that seems be a more appropriate place.
>>
>>         // A GC may occur during the Method::fast_exception_handler_bci_for()
>>         // call below if it needs to load the constraint class. Using a
>>         // methodHandle to keep the 'current_method' from being deallocated
>>         // if GC happens.
> That's good.  I guess we all just have to get familiar with why we still have methodHandles.  That comment for metadata handles helps:
>
> // Metadata Handles.  Unlike oop Handles these are needed to prevent metadata
> // from being reclaimed by RedefineClasses.
>
> -- Chris
>
>> The updated jdk8 webrev is http://cr.openjdk.java.net/~jiangli/8001341/webrev_jdk8.01/.
>>
>> Thanks,
>> Jiangli
>>
>> On 01/07/2013 02:31 PM, Jiangli Zhou wrote:
>>> Hi Chris,
>>>
>>> Thanks for the review and suggestion! Will add a comment in src/share/vm/oops/method.hpp.
>>>
>>> Thanks,
>>> Jiangli
>>>
>>>
>>> On 01/07/2013 02:34 PM, Christian Thalinger wrote:
>>>> On Jan 7, 2013, at 12:30 PM, Jiangli Zhou<jiangli.zhou at oracle.com>   wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please review the fix for 8001341/8004223: SIGSEGV in methodOopDesc::fast_exception_handler_bci_for(KlassHandle,int,Thread*)+0x3e9.
>>>>>
>>>>> hs24 webrev: http://cr.openjdk.java.net/~jiangli/8001341/webrev_hs24.00/
>>>>> jdk8 webrev: http://cr.openjdk.java.net/~jiangli/8001341/webrev_jdk8.00/
>>>>>
>>>>> The SIGSEGV was caused by an unhandled methodOop in methodOopDesc::fast_exception_handler_bci_for() and JvmtiExport::post_exception_throw(). It's only reproducible on hs24 as oops in the perm-gen could be moved by the GC. The bug was not reported on jdk8. On jdk8, Method* doesn't move. However the fix is still needed on jdk8 since the Method* in JvmtiExport::post_exception_throw() could be redefined after the  methodOopDesc::fast_exception_handler_bci_for() call. The handle will keep it from being deallocated.
>>>> I was just going to say it's not required for HS25 but the redefinition is a valid point.  Although imperceptible.  Could we add a comment in:
>>>>
>>>> src/share/vm/oops/method.hpp
>>>>
>>>> to prevent someone undoing this change?
>>>>
>>>> -- Chris
>>>>
>>>>> Tested with -XX:+CheckUnhandledOops on solaris (x64) using vm.quick.testlist. Tested with jprt.
>>>>>
>>>>> Thanks,
>>>>> Jiangli