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

David Holmes david.holmes at oracle.com
Fri Jul 3 01:37:20 UTC 2020


Hi Goetz,

On 3/07/2020 5:30 am, Lindenmaier, Goetz wrote:
> Hi Remi,
> 
> But how does volatile help?
> I see the test for numStackTracesFilledIn == 1 then gets always the
> right value.
> But the backtrace may not be changed until I read it in
> getExtendedNPEMessage.  The other thread could change it after
> checking numStackTracesFilledIn and before I read the backtrace.

True. To ensure you process the original backtrace only you need to add 
synchronization in getMessage():

       public String getMessage() {
           String message = super.getMessage();
           // If the stack trace was changed the extended NPE algorithm
           // will compute a wrong message.
+         synchronized(this) {
!             if (message == null && numStackTracesFilledIn == 1) {
!                 return getExtendedNPEMessage();
!             }
+         }
           return message;
       }

To be honest the idea that someone would share an exception instance and 
concurrently mutate it with fillInStackTrace() whilst printing out 
information about it just seems highly unrealistic. But the above fixes 
it simply. Though after looking at comments in the test I would also 
suggest that setStackTrace be updated:

        synchronized (this) {
             if (this.stackTrace == null && // Immutable stack
                 backtrace == null) // Test for out of protocol state
                 return;
+           numStackTracesFilledIn++;
             this.stackTrace = defensiveCopy;
         }
     }

as that would seem to be another hole in the mechanism.

> I want to vote again for the much more simple version
> proposed in webrev 02:
> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-jdk15/02/

I much prefer the latest version that recognises that only the original 
stack can be processed.

In the test:

+         // This holds for explicitly crated NPEs, but also for implicilty

Two typos: crated  & implicilty

Thanks,
David
-----


> It's drawback is only that for this code:
>    ex = null;
>    ex.fillInStackTrace()
> no message is created.
> 
> I think this really is acceptable.
> 
> 
> Remi, I didn't comment on this statement from a previous mail:
>>> Hmm, Throwable.stackTrace is used for the stack trace at some point.
>> yes, it contains the Java stack trace, but if the Java stack trace is filled you don't
>> compute any helpful message anyway.
> The internal structure is no more deleted when the stack trace
> is filled. So the message can be computed later, too.
> 
> Best regards,
>    Goetz.
> 
>> -----Original Message-----
>> From: forax at univ-mlv.fr <forax at univ-mlv.fr>
>> Sent: Thursday, July 2, 2020 8:52 PM
>> To: Alan Bateman <Alan.Bateman at oracle.com>
>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Christoph Dreis
>> <christoph.dreis at freenet.de>; hotspot-runtime-dev <hotspot-runtime-
>> dev at openjdk.java.net>; David Holmes <david.holmes at oracle.com>
>> Subject: Re: [15] RFR: 8248476: No helpful NullPointerException message
>> after calling fillInStackTrace
>>
>> yes,
>> it's what i was saying,
>> given that a NPE can be thrown very early, before VarHandle is initialized, i
>> believe that declaring numStackTracesFilledIn volatile is the best way to
>> tackle that.
>>
>> Rémi
>>
>> ----- Mail original -----
>>> De: "Alan Bateman" <Alan.Bateman at oracle.com>
>>> À: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>, "Christoph Dreis"
>> <christoph.dreis at freenet.de>
>>> Cc: "hotspot-runtime-dev" <hotspot-runtime-dev at openjdk.java.net>,
>> "David Holmes" <david.holmes at oracle.com>, "Remi Forax"
>>> <forax at univ-mlv.fr>
>>> Envoyé: Jeudi 2 Juillet 2020 20:47:31
>>> Objet: Re: [15] RFR: 8248476: No helpful NullPointerException message
>> after calling fillInStackTrace
>>
>>> On 02/07/2020 17:45, Lindenmaier, Goetz wrote:
>>>> Hi Christoph,
>>>>
>>>> I fixed the comment, thanks for pointing that out.
>>>>
>>> One other thing is that NPE::getMessage reads numStackTracesFilledIn
>>> without synchronization.
>>>
>>> -Alan


More information about the hotspot-runtime-dev mailing list