[15] RFR: 8248476: No helpful NullPointerException message after calling fillInStackTrace

[Mandy Chung]

Hi Goetz,

I digged up some history of setStackTrace.

> Well, all the scenario are quite artificial:
>   - why would you call fillInStackTrace on an exception thrown by the VM?
>   - why would you call setStackTrace at all?

FYI on the history on `setStackTrace` - this API is designed for the 
client side to fill in the stack trace for example concatening the 
server-side stack trace  with appropriate client-side stack trace (see 
JDK-4010355).

As described in JEP 358, NPEs that are explicitly created and/or 
explicitly thrown by programs running on the JVM are not subject to the 
bytecode analysis.   So it's reasonable to include the case that NPE 
whose stack trace is explicitly replaced via `fillInStackTrace` or 
`setStackTrace` are not subject to the bytecode analysis (rather than 
only `fillInStackTrace`).

>> To me that makes
>> far more sense than having msg2 continue to report the extended info for
>> the original stacktrace when it now has a new stacktrace.
>>
>> I'm really not seeing why calling fillInstackTrace() a second time
>> should be treated any differently to calling setStackTrace(). They
>> should be handled consistently IMO.

I agree with David on this point.  `fillInStackTrace` and 
`setStackTrace` both override this throwable with a different stack 
trace and I don't see why we want them to behave differently.

> But then you treat setStackTrace() differently from setStackTrace()
> with other exceptions.
> The reason to treat fillInStackTrace differently is that we lost information
> needed to compute it. This is not the case with setStackTrace().

This is because backtrace is not reset to null which is just the current 
implementation.  A simple way is to override NPE::setStackTrace to 
indicate that this NPE instance is not subject to bytecode analysis.

> A different solution, the one I would have proposed if I had not
> considered previous comments from reviews,  would be to just
> compute the message in the runtime in the call of fillInStackTrace
> before the old stack trace is lost and assign it to the message field.
> This way it would behave similar to all other exceptions. The message
> would just be there ... just that it's computed lazily.
> The cost of the algorithm wouldn't harm that much as other costly
> algorithms (walking the stack) are performed at this point, too.
>

I expect most common cases calling fillInStackTrace and setStackTrace 
are on explicitly created NPEs which are not subject to bytecode 
analysis.    While it should be rare to see NPE with an extended message 
but its stack trace is replaced, such NPE message + stack trace would be 
quite confusing.

OTOH, it's okay with me if you want to replace 
NPE::numStackTracesFilledIn with a volatile NPE::message field instead 
such that it's assigned to the result returned from 
getExtendedNPEMessage() and reset to an empty string if the stack trace 
is overridden.

Mandy