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

[David Holmes]

Seems quite reasonable.

Aside: when looking at other uses of method_counters() for missing NULL 
checks I noticed:

MethodCounters* Method::build_method_counters(Method* m, TRAPS) {
   methodHandle mh(m);
   ClassLoaderData* loader_data = mh->method_holder()->class_loader_data();
   MethodCounters* counters = MethodCounters::allocate(loader_data, 
CHECK_NULL);
   if (mh->method_counters() == NULL) {
     mh->set_method_counters(counters);
   } else {
     MetadataFactory::free_metadata(loader_data, counters);
   }
   return mh->method_counters();
}


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();
}

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