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

[serguei.spitsyn at oracle.com]

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/20150406/8b07d8e0/attachment.html>