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

[Vladimir Ivanov]

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.

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
>