8u60 backport RFR (M) 8067662: "java.lang.NullPointerException: Method name is null" from StackTraceElement.<init>
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Wed Apr 8 18:58:07 UTC 2015
Thanks, Collen!
It was a great suggestion. :)
Thanks,
Serguei
On 4/8/15 11:49 AM, Coleen Phillimore wrote:
>
> Serguei, This latest version looks great.
> Coleen
>
> On 4/8/15, 2:38 PM, serguei.spitsyn at oracle.com wrote:
>> Thanks, Dan!
>>
>> I will correct the grammar before the push..
>>
>> Thanks,
>> Serguei
>>
>> On 4/8/15 6:54 AM, Daniel D. Daugherty wrote:
>>> On 4/7/15 7:09 PM, serguei.spitsyn at oracle.com wrote:
>>>> On 4/7/15 6:07 PM, serguei.spitsyn at oracle.com wrote:
>>>>>
>>>>>
>>>>> 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.
>>>> Sorry, forgot to paste the link:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8067662-JVMTI-trace.8u60.2/
>>>
>>> Comparing the two patch files looks good. A couple of nits:
>>>
>>> src/share/vm/classfile/javaClasses.cpp
>>> L1451: // Use specific ik version...
>>> L1884: // Use specific ik version...
>>> grammar: 'Use a specific ik version...'
>>>
>>> L1452: // refer to version that...
>>> L1885: // refer to version that...
>>> grammar: 'refer to a version that...'
>>>
>>> src/share/vm/classfile/javaClasses.hpp
>>> No comments.
>>>
>>> src/share/vm/oops/instanceKlass.cpp
>>> No comments.
>>>
>>> src/share/vm/oops/instanceKlass.hpp
>>> No comments.
>>>
>>> Thumbs up.
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the hotspot-dev
mailing list