RFR(L): 8218628: Add detailed message to NullPointerException describing what is null.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Wed Mar 27 14:18:15 UTC 2019

Hi Mandy,

> 		 and also the cases when it requires to look at its caller frame.
> 	Never.  Only the method that was executed when the exception is
> 	thrown is needed. I only mention the backtrace because it happens to
> 	store the top method and the corresponding bytecode position in the
> 	top method.
> That explains why I don't see an example of:
>      getNull().m();
>      public void m() {};
>      static NPE getNull() {
>          return null;
>      }
> For a compiler expert or one who study the data flow analysis paper
Yes, I think compiler people should understand this.

> , is this example very clear to them that it won't be supported?
The example for your code is supported, see[1], examples 11-14. (I should enumerate them)
For your code, it will print
"The return value of 'MyClass.getNULL()LNPE;' is null. Can not invoke method 'MyOtherClass.m()V'".

getNull().m() are not nested calls, but subsequent ones. Therefore you will not find
them on the call stack.
The byte code index of the invoke bytecode invoking m() is known from the backtrace or
StackWalker/StackFrame.  Looking at the bytecode the part "Can not invoke method 'MyOtherClass.m()V"
is printed.
With the result of the data flow analysis it is possible to find the bytecode that produced
the reference on which m() is invoked.  This is another invoke bytecode in the method.
Looking at this other bytecode, " The return value of 'MyClass.getNULL()LNPE;' is null." 
is printed.

You probably meant that for 
it is not printed that getNull() was called on the result of n()?
This is a design decision not to make the printout too complex.
> This is related to my comment w.r.t. the data flow analysis comes from.  I was
> looking for some level of information be spelled out in the JEP that can
> sufficiently give a non-compiler expert an idea but of course no point to
> replicate the paper here.
"The simulated stack" ... contains ..."information about which bytecode pushed the value to the stack."
That's all.

> It's good to know that this proposal needs the top frame only.   Please make it
> clear in the JEP.   
I tried to point out more precisely why and when I need the top frame. I 
pushed this down a bit in the text in "basic algorithm".

> The JEP can take out `StackWalker` but instead talks about
> `StackFrame` since doing it in Java means that it only needs to get back a `StackFrame`
> instance representing the top frame stored in `Throwable::backtrace`
Yes. I tried to make the text more precise.

> This will avoid the confusion on calling StackWalker in the NPE constructor and
> constructing many StackFrame and many ResolvedMethodName.  That was
> part of the code review discussion.
Yes, only the top StackFrame is needed.

> 		"Computing either part of these messages can fail due to
> insufficient
> 		information."
> 		What are the cases that it fails to obtain the information? It'd
> be useful
> 		to list some examples if not all.
> 	(a ? b : c).d
> 	You can not know whether the ?-expression yielded b or c, thus
> 	you don't know why the access .d failed.  There is a corresponding
> example
> 	in [1].
> Please describe in the JEP the known cases that this proposal doesn't cover.
> Conditional expression is one and 

The algorithm looks at a lot of bytecodes to print the method. I can not 
enumerate all combinations of bytecodes, it's millions. I will add a per-bytecode table to 
the "Message content" section describing what is printed for which bytecode.

>null receiver if it's the return value from a caller frame (I think this would help the readers).
... this is supported, see above.

> 		The "Different bytecode variants of a method" section
> 		This section raises the case when a method representing an
> execution stack
> 		frame throwing NPE becomes obsolete, it does not have the
> information
> 		to compute the message lazily.  So this falls into the
> "insufficient
> 		information" category and no improved NPE message can be
> generated.
> 	Yes, note this also only addresses the method in which the exception
> was raised.
> This should also go to the list of known cases that this proposal does not cover.

This paragraph lists a possible problem -- multiple variants of methods
with the same name and class name -- and how this is solved. 
I added a section "Cases not covered by this JEP" and mention it there, too.

> Since the JEP quotes that NullPointerExceptions are thrown frequently
> and swallowed, it would help if you can generate some numbers.
> This JEP will help the discussion on the proposed feature and design and avoid
> any
> confusion/assumption of the implementation.
I'll generate some numbers.  Is running jvm2008 fine?

> I think this is related to this JEP.    The question would be:
> - should it include all hidden frames?
> - should it include the top frame if it's a hidden frame since this feature only
> requires the top frame?
> This JEP requires the hidden frame to be recorded in the backtrace.  This has
> impact to the printing of stack trace that needs to know about hidden frames
> and decide if it should be filtered or included when printing.
No. I propose to remember that the frame was hidden. I do not propose to 
remember the hidden frame.  
I now mention in the JEP that the message is not printed for hidden frames in the 
new "cases not covered" section.

> 	I made a webrev of its own for this problem, so that the code
> 	can be looked at isolated of other, orthogonal issues.
> 	http://cr.openjdk.java.net/~goetz/wr19/8221077-hidden_to_frame/
> I don't need the webrev as we are discussing the proposed feature and design.
Well, sometimes code is more precise than language ...
> 		The "Message persistance" section
> 		I read this section like an implementation details
> 	 Yes, I had the feeling that I was talking too much about
> implementation details,
> 	but mostly in the "Basic algorithm" section.
> 	This section discusses characteristics of NPE that differ from other
> exceptions
> 	and that were proposed in the course of the review of my
> 	original webrev. They are visible to the Java implementor.
> 	So I think this is just the basic information needed in a JEP.
> This is a good start that allows us to start the discussion.  This JEP will probably
> need several iterations of improvement.  I'm writing a JEP that I have iterated
> many times already and it makes the JEP and proposed feature better.   Just to
> share with you that it's part of the process.
> 		I think this section
> 		intends to call out that the message of NPE if constructed by
> user code
> 		i.e. via public constructor, should not be altered including no-
> arg
> 		constructor or pass a null message to 1-arg constructor.  The
> enhanced
> 		NPE message is proposed only when NPE is thrown due to null
> dereference
> 		when executing bytecode.
> 	Yes.
> 		I don't understand what you tried to say about
> "npe.getMessage() ==
> 		npe.getMessage()".
> 		Throwable::getMessage does not guarantee to return same
> String object for
> 		each invocation.
> 	It does not guarantee so, but it's the case for most exceptions. Usually,
> 	one calls super(msg) in an Exception constructor.
> 	It is a design decision to implement it that way, my original proposal
> 	was different. I'm fine with this design, but it should be mentioned I
> think.
> I think the JEP should be made clearer  what you intend to say.
> 		When deserializing the class bytes may be of a different
> version, so
> 		StackTraceElements
> 		are serialized with its string form.  Serializing NPE with a
> computed message
> 		seems to be the only viable solution.
> 	I think implementing writeReplace() is a good idea, too. But
> 	Roger does not like it: http://mail.openjdk.java.net/pipermail/core-
> libs-dev/2019-February/058513.html
> The JEP should record this either if this is an open issue or makes it a clear
> proposal to compute the message when NPE is serialized and what the client
> would do (that may be running in a different version)

Ah, OK, I understand. 
Here, I should simply state that persistence is not intended.

In the Alternatives section, I can then point out how to implement

I changed the text accordingly.

> 	IAE quotes the names, which are user defined strings. It does not quote
> class names, method names etc.
> 	Here, I was asked to quote these, which is inconsistent with IAE. But
> there are other
> 	examples where method names are quoted ... So there is no clear line
> followed
> 	yet.  In the end, I don't care, but want advice how to do it. (I would not
> quote ...).
> You can add it to the open issues section.
I'll just state they are quoted.  This was the opinion of basically everybody
who commented on this.  The text currently more reads like I want to 
reflect the past discussions in the JEP, better be clear here...

> 		"Alternatives" section
> 		"The current proposal is to implement this in the Java runtime
> in C++ accessing
> 		 directly the available datastructures in the metaspace."
> 		"Also, ASM and StackWalker do not support the full
> functionality needed.
> 		StackWalker must be called in the NullPointerException
> constructor."
> 		This is not true as you wrote in the subsequent sentence.  If we
> choose
> 		the implementation to Java, StackWalker will need to be
> extended to
> 		read Throwable's backtrace.
> 	The senctence above describes the status quo. The current
> implementation
> 	of StackWalker must be called in the constructor.
> I just called out the confusion this section could cause.  I think it should be
> updated or rephase.
I tried to rephrase it.

> Please refer to the JEP template (http://openjdk.java.net/jeps/2) about the
> Testing section.
> // What kinds of test development and execution will be required in order
> // to validate this enhancement, beyond the usual mandatory unit tests?
> // Be sure to list any special platform or hardware requirements.
I tried to improve it.

> To be clear:
> What I was suggesting is to describe the scenarios this proposed feature will
> improve the message
> but *not* to cut-n-paste the messages from [1].  So this is not tied to what the
> message will be
> printed from the implementation.
> Writing the scenario in English can help discussing the NPE messages should be
> implemented
> that will be easy for Joe developer to understand.

NPEs are a very basic thing in Java. They can happen in any Java code
running in any application, toy program ... etc. I think the message should
help in all cases.  It's more to the person implementing the call to getMessage() that
knows the scenario.  I really don't know what to write here.

Thanks a lot for your detailed comments, I think it already 
improved the text considerably!

I'll run the experiment to count the NPEs and write the 
"message content" section about what is printed and what not.

Best regards,

> Mandy
> 	But I would
> 	like to do that once it is agreed on the messages, so I don't need to
> 	edit the JEP all the time.
> 	The file referenced is simply generated by the test, so it's easy to
> update
> 	and sure to reflect the implementation.
> 		This JEP includes a few links to the C++ functions in the current
> webrev.  I
> 		appreciate that and I assume they will be taken out at some
> point.
> 	Yes, sure, I can remove that.
> 	Best regards,
> 	  Goetz.

More information about the core-libs-dev mailing list