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

[Mandy Chung]

Hi Goetz,

This approach seems reasonable.  This is only specific to NPE and so I 
also prefer implementing an overridden NPE::fillInStackTrace to setting 
Throwable::stackTrace field to another marker object.

For the javadoc of NPE::fillInStackTrace, it should simply inherit from 
Throwable, i.e. using   {@inheritDoc}.  I think your @implNote is not 
needed.


   94         if (result == this) numStackTracesFilledIn++;

Do you see any case that returns a different object than this?  I 
believe fillInStackTrace always returns this.

Mandy

On 7/1/20 6:01 AM, Lindenmaier, Goetz wrote:
> Hi David,
>   
> This obviously works, but I'm not sure whether the case
> skipped is worth adding a field to NPE.
> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-jdk15/03/
> (This is not a final webrev, the other fix is still in there commented out.)
>
> The NPE is already skipped or simplified in a row of other cases
>    - serialization
>    - expressions involving more than 5 bytecodes (recursion depth
>       of the algorithm)
>    - hidden top frames
>    - internal backtrace information is lost
> So this further case, which I consider quite unlikely, just adds
> to existing ones.
> Other decisions were made to reduce the changes to the
> NPE class and memory consumption. E.g., the message is
> not persisted once computed, but recomputed on each
> call to getMessage().
>
> I'll post it to core-libs-dev if we follow this version of the
> fix further.
>
> Best regards,
>    Goetz.
>
>
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Wednesday, July 1, 2020 12:50 AM
>> To: Remi Forax <forax at univ-mlv.fr>; Lindenmaier, Goetz
>> <goetz.lindenmaier at sap.com>
>> Cc: Christoph Dreis <christoph.dreis at freenet.de>; hotspot-runtime-dev
>> <hotspot-runtime-dev at openjdk.java.net>
>> Subject: Re: [15] RFR: 8248476: No helpful NullPointerException message
>> after calling fillInStackTrace
>>
>> On 30/06/2020 5:53 pm, Remi Forax wrote:
>>> ----- Mail original -----
>>>> De: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>
>>>> À: "Christoph Dreis" <christoph.dreis at freenet.de>, "hotspot-runtime-
>> dev" <hotspot-runtime-dev at openjdk.java.net>
>>>> Envoyé: Lundi 29 Juin 2020 14:07:08
>>>> Objet: RE: [15] RFR: 8248476: No helpful NullPointerException message
>> after calling fillInStackTrace
>>> [...]
>>>
>>>> An alternative fix I could imagine would be to
>>>> override fillInStackTrace in NPE.java. It could
>>>> call getMessage() and then super.fillInStackTrace,
>>>> and return a new exception with the message.
>>> overriding fillInStackTrace() and marking it when it is called twice is the
>> other solution.
>>
>> That was my thought now I understand how/when this process kicks in.
>> Just add a "filled" count that the overriding fillInStackTrace
>> increments, and have getMessage check the count before calling
>> getExtendedNPEMessage.
>>
>> Cheers,
>> David
>> -----
>>
>>>> But this would also compute the message in cases where it is not printed.
>>> you don't need that, you can change the field Throwable.stackTrace with a
>> marker object not unlike UNASSIGNED_STACK to detect if NPE.fillInStackTrace
>> is called twice,
>>> but as is said earlier, it's not a small change because Throwable should stay
>> thread safe and serializable.
>>> So i agree with you and Christoph that the "best" solution is to document
>> that null.fillInStackTrace() doesn't get a detailed error message, apart if
>> someone has a better fix.
>>>> Best regards,
>>>>    Goetz.
>>> Rémi
>>>
>>>>> -----Original Message-----
>>>>> From: Christoph Dreis <christoph.dreis at freenet.de>
>>>>> Sent: Monday, June 29, 2020 1:28 PM
>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
>> runtime-
>>>>> dev at openjdk.java.net
>>>>> Subject: Re: [15] RFR: 8248476: No helpful NullPointerException message
>>>>> after calling fillInStackTrace
>>>>>
>>>>> Hi Goetz,
>>>>>
>>>>>> If changing the stack trace by calling fillInStackTrace in user code, the
>>>>>> NPE algorithm lacks the proper information to compute the message.
>>>>>> Thus, we must omit it after that call.
>>>>>> I implement this by checking for a call to fillInStackTrace at the bci
>>>>>> recorded in the exception.
>>>>>> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-
>>>>> jdk15/01/
>>>>>
>>>>> I tried this when reporting the issue already. The problem with this is,
>> that it
>>>>> suppresses any valid exception.
>>>>>
>>>>> E.g. the following example would not throw any helpful NPE anymore.
>>>>>
>>>>> public class Main {
>>>>> 	public static void main(String[] args) {
>>>>> 		NullPointerException ex = null;
>>>>> 		ex.fillInStackTrace();
>>>>> 	}
>>>>> }
>>>>>
>>>>> Cheers,
>>>>> Christoph
>>>>>