JDK-8067662 : "java.lang.NullPointerException: Method name is null" from StackTraceElement.
  • Type: Bug
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: 8,8u60,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2014-12-16
  • Updated: 2016-02-15
  • Resolved: 2015-03-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u60Fixed 9 b64Fixed
Related Reports
Duplicate :  
JDK-8149794 - Method name is null from StackTraceElement
Relates :  
JDK-7174978 - NPG: Fix bactrace builder for class redefinition
Description
We see strange NPE when taking Thread-dump via JMX when doing CPU profiling in NetBeans Profiler and VisualVM.
The NPE looks like this:

java.lang.NullPointerException: Method name is null
	at java.util.Objects.requireNonNull(Objects.java:228)
	at java.lang.StackTraceElement.<init>(StackTraceElement.java:72)
	at sun.management.StackTraceElementCompositeData.from(StackTraceElementCompositeData.java:55)
	at sun.management.ThreadInfoCompositeData.stackTrace(ThreadInfoCompositeData.java:317)
	at java.lang.management.ThreadInfo.<init>(ThreadInfo.java:277)
	at java.lang.management.ThreadInfo.from(ThreadInfo.java:794)

and if I do thread dump of the same profiled application via Attach API, I see this:

"Intro" #26 prio=1 os_prio=31 tid=0x00007f949bc10800 nid=0xe41f waiting on condition [0x000000012bec8000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
	at java.lang.Thread.sleep(Native Method)
	at java2d.Intro$Surface.null (Redefined)
	at java.lang.Thread.null (Redefined)

note the "null" at the place of the method name. This corresponds to the NPE from JMX. All this leads me to the JDK-8025238, which introduced the "null (Redefined)" in the stack trace. It looks like there are some changes in handling method_id of redefined methods, since the above problems are new in JDK 8. JDK 7u72 works fine. JDK-8025238, fixed the problem for stack trace taken via Attach API, but JMX code is broken - it does not count with the fact that method name can be null. There is one very interesing thing - if you take a heap dump in the situation, when JXM thread dump fails with NPE and thread dump taken via Attach API displays "Thread.null (Redefined)", the stack traces stored in the heap dump have correct method names. So there is a way to do it right without "null" names. As a proof of concept, I am attaching the patch against JDK8u-dev, which fixes NPE in JMX thread dump and also avoids "null" method names in Attach API thread dump. This patch does not use method_id returned by method->method_idnum(). It looks like conversion to method_id and back to Method* does not work correctly for old redefined methods, which are still on the stack. Note that attached patch does not solve similar problem in java_lang_Throwable::get_stack_trace_element(), which should also updated to use MethodHandle instead of method_id.
Comments
Tomas Hurka is asking to fix this in 8u60 as it impacts NetBeans (and maybe VisualVM). I see how my 9 fix can be adjusted for 8u60 and hope to come up with a webrev today.
01-04-2015
Thanks, Coleen. It is reasonable also because the 8055008 fix has more dependencies on other fixes that have not been backported yet.
01-04-2015
8055008 was a pretty large fix so I wasn't planning on backporting it.
01-04-2015
I've got a problem in backporting of this fix to 8u60. It occurred that the fix depends on multiple other fixes in 9 that have not been backported to 8u60 yet. For instance, it depends on the fix of: https://bugs.openjdk.java.net/browse/JDK-8055008 Coleen, do you plan to backport the 8055008 to 8u60? I will proceed correspondingly. If the 8055008 will be backported then I'd prefer wait for that. Otherwise, the 9 fix has to be reworked a little bit for 8u60.
01-04-2015
[To : Tomas Hurka] I'm going to give the fix above some baking time in the HS_RT nightly and to backport it to the 8u60, most likely, at the end of the week.
24-03-2015
Thanks for the comment, Coleen. My guess is that the file name can be bound to the version of the class. Newly redefined version can potentially point to a different file. A method belongs to its klass version, this is why file-name is per method. I'll check what can be done with the source file name before posting the webrev for open review.
12-03-2015
This change seems reasonable. I don't think the extra memory adding a typeArrayOop for u2 vs merging the name with method_idnum into a typeArrayOop for u4 saves that much. If we need to squeeze more footprint out of this, we could do what you said and never print the line number or source file if the redefined version is >0 , which is what I was originally thinking. Why is file-name per method? wouldn't it be per class? It is but your method may have been in the old source file if version doesn't match.
12-03-2015
A slightly extended implementation of the Coleen's approach can be found in this weberev: http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2015/hotspot/8067662-JVMTI-trace.02/ It adds a u2 of the method cp index (cpref) to the backtrace for each frame. Not sure, this kind of memory footprint overhead can be tolerated though. One small improvement can be to merge idnum with cpref in one int. In fact, I don't like this extended approach myself as it is intrusive and non-elegant. Coleen also suggested to replace klass version with method name cp index in backtrace. But I have a problem with it. If there is no klass version then it is impossible to find out if the bci can be used to provide more detailed method info (line #). The only way I see would be to provide the details (includes file name and line #) only if the latest klass version is 0 (the class was never redefined). Not sure, if this kind of simplification would be Ok. The webrev for this approach: http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2015/hotspot/8067662-JVMTI-trace.03/ Also, I was surprised a little bit that the method signatures are not used in backtraces.
12-03-2015
You nailed it, I like the idea! This looks like the simplest and pretty workable approach. Thanks!
04-03-2015
Wait, I got it! Even if we delete the static method, we still have its name as a u2 constant pool index in the class (we store the mirror so we have the class). We could store that in the backtrace rather than the version.
04-03-2015
One of the other options I tried that was pretty slow was to have the stack trace store the class-qualified method_name, bci, file and line (the fields we care about in java/lang/StackTraceElement). Getting the file and line out of Method* for all the backtraces we created (in javac benchmark) was the bottleneck though. I think this is the cleanest of all.
04-03-2015
Returning non-null meaningful string looks like a good tradeoff, as I agree, it takes too much time for such a rare case. However, it still feels feasible to return at least the method name. :)
04-03-2015
Need to think a little bit over all the options. :) I wonder if we can use reference counter instead of memory walking here. I mean, something like this: - a ref counter field in ik or ik->constants() (I admit there can be a footprint issue here - "int version" in cpool can be replaced with "u2 version + u2 ref_count"): - increase the counter in the java_lang_Throwable::fill_in_stack_trace() -> bt.push() - walk the throwable.backtrace() and decrease the counter for each klass version in it when the throwable is about to be deallocated
04-03-2015
Interesting idea. We can increment the Method* reference counter when we put it on the backtrace but we can't decrement it without having GC know about the backtrace element. We were also trying to avoid specific GC code. Although maybe we can have a keep-alive-forever bit for all methods that are in backtraces and never deallocate them. It would prevent some redefined classes from being reclaimed and increase footprint. I'm not sure how many though. We'd have to have another bit in method and complicate the on_stack logic. I spent a lot of time on the original code so that it wouldn't increase footprint or regress performance, which is why I chose returning null if the redefined method was deleted. I still don't see how this unusual case is worth these tradeoffs. I still think we should return some other string rather than null.
04-03-2015
Instead of returning NULL, could we return some interned string like "Method has been deleted due to redefinition" in the stack trace element? the issue is the NPE not really that you can't humanly read the name right?
04-03-2015
If all you need is the name of the method and not file and line number at the bci, then we could save the Symbol* for the method name. This would have footprint impact though.
04-03-2015
One of the implementations to fix this was to save the Method* in a list like MemberNames list so that it could be walked in MetadataOnStackMark to not deallocate the Method*. This implementation wasn't chosen because it had a performance regression in specjvm98 javac benchmark. We create stack traces for all exceptions thrown, whether handled or not.
04-03-2015
The issue is that the old klass version that had to have the deleted method (deleted in new version) is not included into the previous_versions list. So that the required class version can not be found, and so, the orig_method_idnum can not be used too.
04-03-2015
I modified the test java/lang/instrument/RedefineMethodInBacktrace.sh and hoe, was able to reproduce this issue. The problem I see is this: 1. The test method throws an InvocationTargetException that is caught and returned as a Throwable object. 2. The test class is redefined, the redefinition deletes one of the private static methods that is present in the stack of the Throwable. 3. The System.gc() is called. 4. The throwable.getStackTrace() can find the required version of the class for the method represented in the StackTraceElement by its method_idnum. This happens because the InstanceKlass::add_previous_version() does not add the original version of the redefined class into the previous_versions(). It is because the cp_ref->on_stack() returns false. The cp_ref->on_stack() returns false because the MetadataOnStackMark::MetadataOnStackMark() knows only about thread dumps controlled by the ThreadService but nothing about locally used Throwables.
04-03-2015
Yes, MetadataOnStackMark does not walk the Java heap to find Method* in throwable, which is the original problem. Does your current patch resolve this? I didn't think we have a deleted static method on the methods() list after the class is rewritten, so either orig_method_idnum or method_idnum wouldn't find the Method*.
04-03-2015
I've prepared a fix for this that is utilizing the orig_method_idnum() but also adds new function: InstanceKlass::method_with_orig_idnum(int idnum, int version). It looks like the test java/lang/instrument/RedefineMethodInBacktrace.sh can be used to verify my fix.
03-03-2015
I'm puzzled why a method by method_idnum is gone, probably I'm missing something. If method is redefined then the method_idnum still has to be valid and point to the updated version of the method. The old method version is given a new method_idnum. The only way to invalidate the method_idnum is to delete the method as part of class redefinition. Tomas, Could you, please, clarify this? Do you observe this bug when a private method is deleted at redefinition?
27-02-2015
Unfortunately, we build stack traces for every exception thrown, which is a lot, so there are constraints. I'm glad your patch fixes the problem you saw without extra footprint. I'd have to go back and see what other situations didn't allow finding the Method* from the idnum. Or Serguei will find it :)
19-02-2015
Believe it or not this patch really fixes the problem for the described case when redefined method is still on the stack. I verified it. Maybe there are other cases, where Method* can be NULL, I don't know, but as I wrote in description, the patch just uses the same logic used in code, which does heap-dumping. It is nice that you are trying to use as little footprint and time as possible, but this cannot justify a loss of functionality.
19-02-2015
I don't see how this patch fixes the problem. It seems to just call method_with_idnum outside the printing code and can still get NULL for Method* if the method was deleted. We were trying to use as little footprint and time as possible building these stack traces, or else I would have stored the Symbol* for the name itself.
18-02-2015
Unfortunately I just found out that profiling does not work with latest JDK 9 builds. Mainly because jimage files are not zip files. So I don't know if this is reproducible in JDK 9. You can use test case from JDK-8025238 to reproduce this issue. Which Coleen's fix are you talking about?
19-12-2014
Tomas, Thank you for filing this bug. Do you see the same issue with jdk 9 ? Do you have a stand-alone test case reproducer for this? It'd be a great help if you have. The diff above seems to be one of the recent fixes from Coleen in this area.
18-12-2014