RFR (XS): 8023004: JSR 292: java.lang.RuntimeException: Original target method was called.

[serguei.spitsyn at oracle.com]

Vladimir,


Just want to share some details with you.


On 10/17/13 5:46 AM, Vladimir Ivanov wrote:
> Serguei, Chris, Igor, thanks for looking at this fix.
>
> I need to look more thoroughly at this code.
> I assumed that member name is initialized only once, but I was wrong - 
> asserting empty slot in MethodHandles::init_method_MemberName fails 
> during initialization of JSR292 core classes.

John or Chris, please, correct me if I'm wrong...
But, I think, John had a plan in mind to utilize the MNT for uniqueness 
of member names.
It was about all member names, not only method member names.


Thanks,
Serguei

>
> Best regards,
> Vladimir Ivanov
>
> On 10/17/13 1:39 PM, serguei.spitsyn at oracle.com wrote:
>> On 10/16/13 1:28 PM, Christian Thalinger wrote:
>>> On Oct 14, 2013, at 12:15 PM, Vladimir Ivanov
>>> <vladimir.x.ivanov at oracle.com> wrote:
>>>
>>>> http://cr.openjdk.java.net/~vlivanov/8023004/webrev.00/
>>>> 7 lines changed: 5 ins; 1 del; 1 mod
>>>>
>>>> Assertion code in JDK fetches info from VM using MHN.getMemberVMInfo:
>>>> assert(m.vminfoIsConsistent());
>>>>
>>>> After that check, class redefinition is broken - it's still possible
>>>> to invoke old version of a method.
>>>>
>>>> Analysis from Serguei:
>>>> "The problem is here:
>>>> oop MethodHandles::init_method_MemberName(Handle mname, CallInfo&
>>>> info) {
>>>>   . . . . . . . .
>>>> m->method_holder()->add_member_name(m->method_idnum(), mname);
>>>>
>>>>   return mname();
>>>> }
>>>>
>>>> The call to add_member_name() resets the member name at the index
>>>> m->method_idnum(). After that one of the two member names with the
>>>> same method_idnum() is out of the MNT and is not updated when the
>>>> method is redefined."
>>> As I understand this the problem is calling
>>> MethodHandles::init_method_MemberName which installs a new MemberName
>>> instance in the MemberNameTable.  Correct?
>>>
>>> If that's the case then there is a potential race here.
>>>
>>> +     x = m->method_holder()->get_member_name(m->method_idnum());
>>> +     if (x == NULL) {
>>> +       Handle mname2 = MethodHandles::new_MemberName(CHECK_NULL);
>>>          x = MethodHandles::init_method_MemberName(mname2, info);
>>>        }
>>>
>>> Racing threads can get back null when calling get_member_name and then
>>> install different MemberName objects in the table.
>>
>> Good catch, Chris.
>> I do not see a way to fix it easily without some refactoring.
>>
>> It is probably better to add to the InstanceKlass a helper function that
>> will do
>> this dance with a required synchronization.
>> But the following line needs to be taken out of the
>> init_method_MemberName() :
>> m->method_holder()->add_member_name(m->method_idnum(), mname);
>>
>>
>> Thanks,
>> Serguei
>>
>>>
>>>> The fix is to check whether there's already a member name associated
>>>> with the before creating a new one.
>>>>
>>>> Testing: failing test.
>>>>
>>>> Contributed-by: sspitsyn, vlivanov
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8023004
>>