Request for review: JDK-8012052: java/lang/invoke/6987555/Test6987555.java crashes with assert(mcs != NULL) failed: MethodCounters cannot be NULL

[David Holmes]

<trimmed>

On 13/04/2013 3:44 AM, Jiangli Zhou wrote:
> On 04/11/2013 08:46 PM, David Holmes wrote:
>> On 12/04/2013 12:59 PM, Jiangli Zhou wrote:
>>> On 04/11/2013 07:43 PM, David Holmes wrote:
>>>> Can't we simplify that to:
>>>>
>>>> MethodCounters* Method::build_method_counters(Method* m, TRAPS) {
>>>>   methodHandle mh(m);
>>>>   if (mh->method_counters() == NULL) {
>>>>     ClassLoaderData* loader_data =
>>>> mh->method_holder()->class_loader_data();
>>>>     MethodCounters* counters = MethodCounters::allocate(loader_data,
>>>> CHECK_NULL);
>>>>     mh->set_method_counters(counters);
>>>>   }
>>>>   return mh->method_counters();
>>>> }
>>>
>>> We need to recheck if mh->method_counters() is NULL before
>>> set_method_counters() because some other thread might have created a
>>> MethodCounters object and set to the method after the first NULL check.
>>
>> If you are worried about concurrency then both versions are broken -
>> all you are doing is narrowing the window with the current version.
>
> Yes, agreed.
>
> The race is rare. If it does happen, the overwritten one would just
> become "orphan".

Does "orphan" == leak? Do we need to fix this - either now or in a 
follow up CR? Are there other non-thread-safe operations on MethodHandles?

Thanks,
David

> Thanks,
> Jiangli
>
>>
>> David
>>
>>> Thanks,
>>> Jiangli
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 12/04/2013 10:39 AM, Jiangli Zhou wrote:
>>>>> Please review the following fix for JDK-8012052
>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8012052>:
>>>>>
>>>>> http://cr.openjdk.java.net/~jiangli/8012052/webrev.00/
>>>>>
>>>>> The MethodCounters object is not guaranteed to be present for a method
>>>>> that is being compiled when
>>>>> NonTieredCompPolicy::delay_compilation() is
>>>>> called due to compiler shuts off. In
>>>>> java/lang/invoke/6987555/Test6987555 test case, the current method
>>>>> being
>>>>> compiled is java.lang.invoke.MethodHandle::linkToStatic(LL), which
>>>>> hasn’t been executed before and has no associated MethodCounters at
>>>>> the
>>>>> time. Since delay_compilation() decrements the _counter by dividing 2
>>>>> only if _counter >0, there is no need to call delay_compilation() when
>>>>> the method has no associated MethodCounters.
>>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>
>