[15] RFR: 8248476: No helpful NullPointerException message after calling fillInStackTrace
David Holmes
david.holmes at oracle.com
Wed Jul 15 11:36:49 UTC 2020
Hi Goetz,
On 15/07/2020 7:23 pm, Lindenmaier, Goetz wrote:
> Hi everybody ��
>
> First of all thanks for all the feedback!
>
> I updated webrev 06 to 06.2:
> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-jdk15/06.2/
>
> - removed volatile
> - renamed the field so it's obvious it's a ternary state,
> documented the states and implemented it as proposed by
> David
> - added a state change in getMessage(). This avoids repeated
> calls to the native method in case it returns null.
>
> Should I use finals for the states? Or enums?
> private final int NO_BACKTRACE = 0;
> private final int MUST_COMPUTE_LAZY_MESSAGE = 1;
> private final int MESSAGE_COMPUTED = 2;
final statics works okay for me - though if the state variable is still
a "counter" you don't need symbolic names for the states. But this is
good to go for me.
Thanks,
David
> Best regards,
> Goetz.
>
>
>> -----Original Message-----
>> From: hotspot-runtime-dev <hotspot-runtime-dev-retn at openjdk.java.net>
>> On Behalf Of coleen.phillimore at oracle.com
>> Sent: Tuesday, July 14, 2020 10:27 PM
>> To: Mandy Chung <mandy.chung at oracle.com>
>> Cc: hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: [15] RFR: 8248476: No helpful NullPointerException message
>> after calling fillInStackTrace
>>
>>
>>
>> On 7/14/20 4:17 PM, Mandy Chung wrote:
>>> fillInStackTrace and setStackTrace replace the stack trace of a NPE
>>> instance. Therefore I think both should behave consistently for any
>>> NPE instances with and without an explicit message.
>>>
>>> For webrev.06/webrev.07, this would behave as if NPE was created with
>>> an extended message which cannot be altered once constructed. I
>>> expect that it'd be rare to see NPE instance thrown by VM (not
>>> explicitly constructed) but whose stack trace is replaced. So I'm
>>> fine with this approach.
>>>
>>> webrev.06 is okay while I think checking Throwable::backtrace != null
>>> is clearer as I suggested.
>>
>> I like that version 06 isolates knowledge to NullPointerException.java
>> and doesn't have to know what the expected value of backtrace is in the
>> super class. I maintain my vote for 06.
>>
>> Thanks, I was trying to understand the fillInStackTrace vs.
>> setStackTrace issue, and your description makes sense to me.
>>
>> Coleen
>>
>>>
>>> Mandy
>>>
>>> On 7/14/20 12:55 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>> Goetz and all,
>>>>
>>>> I have to admit, the version with the counter 06 is more intuitive to
>>>> me. It would be even better if it was a boolean. I don't think an
>>>> extra 32 bits in an NPE Throwable matters considering the backtrace
>>>> is a lot bigger. The NPE Throwable in general shouldn't be a long
>>>> lived object, and there shouldn't be thousands of them.
>>>>
>>>> There seemed to be disagreement on the issue of the message not
>>>> matching the stack trace if the code calls setStackTrace(). It
>>>> doesn't seem like it should be the same at all to fillInStackTrace()
>>>> to me, but this latest patch maintains the status quo. If you want
>>>> to explore this further, I think you should file a separate RFE, and
>>>> fix the reported bug with this patch.
>>>>
>>>> So if I get a vote, I'd pick 06.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 7/14/20 9:48 AM, Lindenmaier, Goetz wrote:
>>>>> Hi,
>>>>>
>>>>> Yes, Coleen, you are right. We must preserve the lazy
>>>>> computation, and also reduce overhead on discarded
>>>>> exceptions.
>>>>>
>>>>> And yes, we can do it with a counter:
>>>>> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-
>> jdk15/06/
>>>>>
>>>>> but I would prefer placeholder strings:
>>>>> http://cr.openjdk.java.net/~goetz/wr20/8248476-NPE_fillInStackTrace-
>> jdk15/07/
>>>>>
>>>>> This way we need only one new field.
>>>>>
>>>>> (I need two placeholders, because the getExtendedNPEMessage0()
>>>>> sometimes returns null. If I write null into the extendedMessage field,
>>>>> fillInStackTrace sets it to mustComputeExtendedNPEMessage a second
>>>>> time.)
>>>>>
>>>>> With webrev 07 the overhead on discarded exceptions is basically the
>>>>> same as with webrev 05: one additional field, one assignment in
>>>>> fillInStackTrace().
>>>>>
>>>>> What do you think?
>>>>>
>>>>> Best regards,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: David Holmes <david.holmes at oracle.com>
>>>>>> Sent: Tuesday, July 14, 2020 1:55 PM
>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
>>>>>> 'forax at univ-mlv.fr'
>>>>>> <forax at univ-mlv.fr>; Alan Bateman <Alan.Bateman at oracle.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
>>>>>>
>>>>>> Correction ...
>>>>>>
>>>>>> On 14/07/2020 12:11 pm, David Holmes wrote:
>>>>>>> Hi Goetz,
>>>>>>>
>>>>>>> Okay ... if I understand your position correctly you are looking
>>>>>>> at this
>>>>>>> as if the extended message is created at the time the NPE is
>>>>>>> thrown, and
>>>>>>> it is an implementation detail that we actually determine it
>>>>>>> lazily. If
>>>>>>> it were eagerly determined then neither fillInstacktrace() nor
>>>>>>> setStackTrace() would make any difference to the message - just as
>>>>>>> with
>>>>>>> any other exception message.
>>>>>>>
>>>>>>> However, the lazy determination of the message causes a problem
>> with
>>>>>>> fillInStackTrace() because that call will destroy the original
>>>>>>> backtrace
>>>>>>> needed to produce the original message, and create an incorrect
>>>>>>> message.
>>>>>>> setStackTrace() does not have a similar problem because, simply by
>>>>>>> the
>>>>>>> way the current implementation works it doesn't touch the original
>>>>>>> backtrace.
>>>>>>>
>>>>>>> So you are proposing to only fix the bug that is evident in
>>>>>>> relation to
>>>>>>> fillInStackTrace() by no longer evaluating the extended message if
>>>>>>> fillInStackTrace() is called after the NPE was constructed.
>>>>>>>
>>>>>>> But in doing so you break the illusion that the extended message acts
>>>>>>> as-if determined at construction time, because you now effectively
>>>>>>> clear
>>>>>>> it when fillInStackTrace is called.
>>>>>>>
>>>>>>> My position was that if fillInStackTrace can be seen to clear it,
>>>>>>> then
>>>>>>> setStackTrace (which is logically somewhat equivalent) should also be
>>>>>>> seen to clear it.
>>>>>>>
>>>>>>> Alternatively, add a new field to NPE to cache the extended error
>>>>>>> message, and explicitly evaluate the message if fillInStackTrace() is
>>>>>>> called. That will continue the illusion that the extended message was
>>>>>>> actually set at construction time. No changes needed to
>>>>>>> setStackTrace()
>>>>>>> as we can still lazily compute the extended message.
>>>>>>>
>>>>>>> Something like:
>>>>>>>
>>>>>>> private String extendedMessage;
>>>>>>>
>>>>>>> public synchronized Throwable fillInStackTrace() {
>>>>>>> if (extendedMessage == NULL) {
>>>>>>> extendedMessage = getExtendedNPEMessage();
>>>>>>> }
>>>>>>> return super.fillInStackTrace();
>>>>>>> }
>>>>>> Coleen pointed out to me that we can't do it like this because we need
>>>>>> the initial fillInStacktrace to be fast and we want the extended
>>>>>> message
>>>>>> computed lazily. So it will still need a counter so we only do this on
>>>>>> the second call.
>>>>>>
>>>>>>
>>>>>> private String extendedMessage;
>>>>>> private int fillInCount;
>>>>>>
>>>>>> public synchronized Throwable fillInStackTrace() {
>>>>>> if (extendedMessage == NULL && (fillInCount++ == 1)) {
>>>>>> extendedMessage = getExtendedNPEMessage();
>>>>>> }
>>>>>> return super.fillInStackTrace();
>>>>>> }
>>>>>>
>>>>>> or something to that effect.
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>> public String getMessage() {
>>>>>>> String message = super.getMessage();
>>>>>>> synchronized(this) {
>>>>>>> if (message == null) {
>>>>>>> // This NPE should have an extended message.
>>>>>>> if (extendedMessage == NULL) {
>>>>>>> extendedMessage = getExtendedNPEMessage();
>>>>>>> }
>>>>>>> message = extendedMessage;
>>>>>>> }
>>>>>>> }
>>>>>>> return message;
>>>>>>> }
>>>>>>>
>>>>>>> Cheers,
>>>>>>> David
>>>>>>>
>>>>>>> On 14/07/2020 12:48 am, Lindenmaier, Goetz wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>>> Your extended message is only computed when there is no original
>>>>>>>>> message.
>>>>>>>> Hmm. I would say the extended message is only computed when
>>>>>>>> The NPE was raised by the runtime. It happens to never have a
>>>>>>>> message so far in these cases.
>>>>>>>> But this is two views to the same thing ��
>>>>>>>>
>>>>>>>>> You're concerned about this scenario:
>>>>>>>>>
>>>>>>>>> catch (NullPointerException npe) {
>>>>>>>>> String msg1 = npe.getMessage(); // gets extends NPE message
>>>>>>>>> npe.setStackTrace(...);
>>>>>>>>> String msg2 = npe.getMessage(); // gets null
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> While I find it hard to imagine anyone doing this
>>>>>>>> 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?
>>>>>>>>> you can easily have
>>>>>>>>> specified that the extended message is only available with the
>>>>>>>>> original
>>>>>>>>> stacktrace, hence after a second call to fillInStackTrace, or a
>>>>>>>>> call to
>>>>>>>>> setStackTrace, then the message reverts to being empty.
>>>>>>>> The message is not meant to be a special thing that behaves
>>>>>>>> different
>>>>>>>> from other messages. Like sometime be available, sometime not.
>>>>>>>> It ended up being different through requirements during the
>>>>>>>> review.
>>>>>>>>
>>>>>>>>> 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.
>>>>>>>> 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().
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>>> We are not talking about all exceptions only about your NPE
>>>>>>>>> extended
>>>>>>>>> error message.
>>>>>>>> Hmm, the inconsistency caused by the code you posted above
>>>>>>>> holds for all exceptions. If you fiddle with the stack trace,
>>>>>>>> the message might become pointless. Wrt. setStackTrace
>>>>>>>> they all behave the same.
>>>>>>>> Wrt. fillInStackTrace the message will be wrong. Only this
>>>>>>>> needs to be fixed.
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Goetz.
>>>>>>>>
>>>>>>>>
>>>>>>>>> David
>>>>>>>>> -----
>>>>>>>>>
>>>>>>>>>> I implemented an example where wrong stack traces are
>>>>>>>>>> printed with LinkageError and NPE, modifying a jtreg test:
>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr20/8248476-
>>>>>> NPE_fillInStackTrace-
>>>>>>>>> jdk15/05/mess_with_exceptions.patch
>>>>>>>>>> See also the generated output added to a comment in the patch.
>>>>>>>>>> If the NEP message text was missing in the second printout, I
>>>>>>>>>> think
>>>>>>>>>> this really would be unexpected.
>>>>>>>>>> Please note that the correct message is printed after messing
>>>>>>>>>> with the stack trace, it's the stack trace that is wrong.
>>>>>>>>>> (Not as with the problem I am fixing here where a wrong
>>>>>>>>>> message is printed.)
>>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>> Goetz.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> I guess the normal usecase of setStackTrace is the other way
>>>>>>>>>>>> around:
>>>>>>>>>>>> Change the message and throw a new exception with the
>> existing
>>>>>>>>>>>> stack trace:
>>>>>>>>>>>>
>>>>>>>>>>>> try {
>>>>>>>>>>>> a.x;
>>>>>>>>>>>> catch (NullPointerException e) {
>>>>>>>>>>>> throw new NullPointerException("My own error
>>>>>>>>>>> message").setStackTrace(e.getStackTrace);
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> And not taking an arbitrary stack trace and put it into an
>>>>>>>>>>>> exception
>>>>>>>>>>>> with existing message.
>>>>>>>>>>> Interesting usage.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> David
>>>>>>>>>>> -----
>>>>>>>>>>>
>>>>>>>>>>>> Best regards,
>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>> From: David Holmes <david.holmes at oracle.com>
>>>>>>>>>>>>> Sent: Friday, July 3, 2020 9:30 AM
>>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
>>>>>> 'forax at univ-
>>>>>>>>>>> mlv.fr'
>>>>>>>>>>>>> <forax at univ-mlv.fr>; Alan Bateman
>> <Alan.Bateman at oracle.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
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Goetz,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 3/07/2020 4:32 pm, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> True. To ensure you process the original backtrace only you
>>>>>>>>>>>>>>> need to
>>>>>>>>>>> add
>>>>>>>>>>>>>>> synchronization in getMessage():
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr20/8248476-
>>>>>>>>> NPE_fillInStackTrace-
>>>>>>>>>>>>> jdk15/05/
>>>>>>>>>>>>>> I added the volatile, too, but as I understand the
>>>>>>>>>>>>>> synchronized
>>>>>>>>>>>>>> block brings sufficient memory barriers that this also works
>>>>>>>>>>>>>> without.
>>>>>>>>>>>>> No "volatile" needed, or wanted, when all access is within
>>>>>>>>>>>>> synchronized
>>>>>>>>>>>>> regions.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>> Yes, contention here is quite unlikely, so it should not harm
>>>>>>>>> performance
>>>>>>>>>>>>> ��
>>>>>>>>>>>>>
>>>>>>>>>>>>> Contention was not my concern at all. :)
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Though after looking at comments in the test I would also
>>>>>>>>>>>>>>> suggest that setStackTrace be updated:
>>>>>>>>>>>>>> The test shows that after setStackTrace still the correct
>>>>>>>>>>>>>> message
>>>>>>>>>>>>>> is computed. This is because the algorithm uses
>>>>>>>>>>>>>> Throwable::backtrace
>>>>>>>>>>>>>> and not Throwable::stacktrace. Throwable::backtrace is not
>>>>>>>>>>>>>> affected by setStackTrace.
>>>>>>>>>>>>>> The behavior is just as with any exception. If you fiddle
>>>>>>>>>>>>>> with the stack trace, but don't adapt the message text,
>>>>>>>>>>>>>> the message might refer to other code than the stack trace
>>>>>>>>>>>>>> points to.
>>>>>>>>>>>>> But you can't adapt the message text - there is no
>>>>>>>>>>>>> setMessage! If
>>>>>>>>>>>>> the
>>>>>>>>>>>>> message is NULL and you call setStackTrace() then
>>>>>>>>>>>>> getMessage(), it
>>>>>>>>> makes
>>>>>>>>>>>>> no sense to return the extended error message that was
>>>>>>>>>>>>> associated
>>>>>>>>> with
>>>>>>>>>>>>> the original stack/backtrace.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> David
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>> From: David Holmes <david.holmes at oracle.com>
>>>>>>>>>>>>>>> Sent: Friday, July 3, 2020 3:37 AM
>>>>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
>>>>>>>>> 'forax at univ-
>>>>>>>>>>>>> mlv.fr'
>>>>>>>>>>>>>>> <forax at univ-mlv.fr>; Alan Bateman
>> <Alan.Bateman at oracle.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
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 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