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

[serguei.spitsyn at oracle.com]

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.

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
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20150407/3db74953/attachment-0001.html>