8u60 backport RFR (M) 8067662: "java.lang.NullPointerException: Method name is null" from StackTraceElement.<init>

[serguei.spitsyn at oracle.com]

On 4/7/15 2:01 PM, serguei.spitsyn at oracle.com wrote:
> On 4/7/15 11:20 AM, Coleen Phillimore wrote:
>>
>> шI see.  The PreviousVersionWalker is gone in 9 and you're using it 
>> to find the method.
>>
>> But this is only going to find the method if it happens to be in the 
>> cpCache somewhere.   I think 
>> pv_node->prev_constant_pool()->pool_holder() is scratch_class (not 
>> 100% sure).  In which case, you can look at that class to find the 
>> method with idnum.
> I'm not 100% sure either.
> Ok, I will check if using the prev_constant_pool()->pool_holder() is 
> going to work.
> It'd make it more consistent with the jdk 9 fix.

This is the updated webrev.
It is more consistent with the jdk 9 fix.
Thank you for the suggestion!

Thanks,
Serguei

>
> Important part is that at least the saved cpref can be used to find 
> method name in the
> the_class cpool if the attempt to find in the previous list cpCaches 
> was unsuccessful.
>
> Thanks,
> Serguei
>
>>
>> Coleen
>>
>> On 4/7/15, 1:09 AM, serguei.spitsyn at oracle.com wrote:
>>> Coleen,
>>>
>>> Thank you for looking at this!
>>>
>>> The main difference is in instanceKlass.cpp and instanceKlass.hpp.
>>>
>>> 8u60:
>>> +Method* InstanceKlass::method_with_orig_idnum(int idnum) {
>>> +  if (idnum >= methods()->length()) {
>>> +    return NULL;
>>> +  }
>>> +  Method* m = methods()->at(idnum);
>>> +  if (m != NULL && m->orig_method_idnum() == idnum) {
>>> +    return m;
>>> +  }
>>> +  // Obsolete method idnum does not match the original idnum
>>> +  for (int index = 0; index < methods()->length(); ++index) {
>>> +    m = methods()->at(index);
>>> +    if (m->orig_method_idnum() == idnum) {
>>> +      return m;
>>> +    }
>>> +  }
>>> +  // None found, return null for the caller to handle.
>>> +  return NULL;
>>> +}
>>> +
>>> +
>>> +Method* InstanceKlass::method_with_orig_idnum(int idnum, int version) {
>>> +  if (constants()->version() == version) {
>>> +    return method_with_orig_idnum(idnum);
>>> +  }
>>> +  ConstantPoolCache* cp_cache = NULL;
>>> +  PreviousVersionWalker pvw(Thread::current(), (InstanceKlass*)this);
>>> +  for (PreviousVersionNode * pv_node = pvw.next_previous_version();
>>> +       pv_node != NULL; pv_node = pvw.next_previous_version()) {
>>> +    ConstantPool* prev_cp = pv_node->prev_constant_pool();
>>> +    if (prev_cp->version() == version) {
>>> +      cp_cache = prev_cp->cache();
>>> +      break;
>>> +    }
>>> +  }
>>> +  if (cp_cache == NULL) {
>>> +    return NULL; // The pv_node is gone, no method is found
>>> +  }
>>> +  Method* method = NULL;
>>> +  for (int i = 0; i < cp_cache->length(); i++) {
>>> +    ConstantPoolCacheEntry* entry = cp_cache->entry_at(i);
>>> +    method = entry->get_interesting_method_entry(this);
>>> +    if (method == NULL) {
>>> +      continue; // skip uninteresting entries
>>> +    }
>>> +    if (method->orig_method_idnum() == idnum) {
>>> +      return method;
>>> +    }
>>> +  }
>>> +  // None found, return null for the caller to handle.
>>> +  return NULL;
>>> +}
>>>
>>>
>>> jdk 9:
>>>
>>> +Method* InstanceKlass::method_with_orig_idnum(int idnum) {
>>> +  if (idnum >= methods()->length()) {
>>> +    return NULL;
>>> +  }
>>> +  Method* m = methods()->at(idnum);
>>> +  if (m != NULL && m->orig_method_idnum() == idnum) {
>>> +    return m;
>>> +  }
>>> +  // Obsolete method idnum does not match the original idnum
>>> +  for (int index = 0; index < methods()->length(); ++index) {
>>> +    m = methods()->at(index);
>>> +    if (m->orig_method_idnum() == idnum) {
>>> +      return m;
>>> +    }
>>> +  }
>>> +  // None found, return null for the caller to handle.
>>> +  return NULL;
>>> +}
>>> +
>>> +
>>> +Method* InstanceKlass::method_with_orig_idnum(int idnum, int version) {
>>> +  InstanceKlass* holder = get_klass_version(version);
>>> +  if (holder == NULL) {
>>> +    return NULL; // The version of klass is gone, no method is found
>>> +  }
>>> +  Method* method = holder->method_with_orig_idnum(idnum);
>>> +  return method;
>>> +}
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 4/6/15 6:49 PM, Coleen Phillimore wrote:
>>>>
>>>> This backport looks good.  Why didn't the patch apply cleanly?  It 
>>>> seems mostly the same to me.
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 4/6/15, 7:50 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Coleen and Dan,
>>>>>
>>>>> I'm asking you as the jdk  9 reviewers...
>>>>>
>>>>> Please, let me know if you have any chance and plans to review this.
>>>>> Would it be enough to have just one (R) reviewer for a non-clean 
>>>>> backport?
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>
>>>>> On 4/2/15 5:03 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> Please, review the jdk 8u60 backport for:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8067662
>>>>>>
>>>>>> 8u60 hotspot webrev:
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8067662-JVMTI-trace.8u60 
>>>>>>
>>>>>>
>>>>>> 8u60 webrev for unit test update (the fix applies cleanly):
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8067662-JVMTI-trace-test.8u60 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 9 hotspot webrev:
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8067662-JVMTI-trace.2 
>>>>>>
>>>>>>
>>>>>> 9 webrev for unit test update:
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8067662-JVMTI-trace-test.2 
>>>>>>
>>>>>>
>>>>>>
>>>>>> Summary:
>>>>>>   The 9 fix can not be applied cleanly so that I have to rework 
>>>>>> the fix a little bit.
>>>>>>
>>>>>>   An NPE is trown in a thread dumping via JMX when doing CPU 
>>>>>> profiling in NetBeans Profiler and VisualVM.
>>>>>>   The issue is that the methods and related klass versions are 
>>>>>> not kept alive if a class redefinition
>>>>>>   takes place between catching a Throwable and taking a thread dump.
>>>>>>   It can result in loosing an obsolete method, and so, the 
>>>>>> reconstruction of method name becomes impossible.
>>>>>>   In such a case, the null reference is returned instead of a 
>>>>>> valid method name.
>>>>>>
>>>>>>   The solution is to use current klass version and method 
>>>>>> orig_idnum instead of ordinary method idnum
>>>>>>   to find required method pointer. In the worst case when the 
>>>>>> related klass version is gone
>>>>>>   (was not included to or was removed from the previous_versions 
>>>>>> linked list), a saved method name
>>>>>>   CP index of the latest klass version can be used to restore the 
>>>>>> method name.
>>>>>>   The footprint extra overhead for this approach is u2 per stack 
>>>>>> frame.
>>>>>>
>>>>>> Testing in progress:
>>>>>>   In progress: nsk redefine classes tests, JTREG 
>>>>>> java/lang/instrument
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>
>>>>
>>>
>>
>