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

Peter Levart peter.levart at gmail.com
Fri Feb 8 15:08:04 UTC 2019


...just one thing if you go that route. Make sure to initialize the
NPE_MESSAGE_PENDING to a new String("something") or else you may be
sharing this constant reference with somebody else via string
interning...

2019-02-08 16:01 GMT+01.00, Peter Levart <peter.levart at gmail.com>:
> Hi Goetz,
>
> In NPE:
>
>  97         String extendedMessage =
> getExtendedNPEMessage(super.getMessage());
>
> ...do you expect super.getMessage() to return anything else than null?
> Wouldn't that only occur when there was a data race between two
> threads observing that lazyComputeMessage is strill true in the
> synchronized block before that and then one thread proceeding to
> compute and set the message while the other reading the set message
> via data race? Or are you planing to replace default message in some
> cases with computed one?
>
> I would just pass null there or even get rid of this parameter if that
> is not the case.
>
> If you do that, there is an alternative to having a boolean field in
> NPE. You could create a private static final String constant, call it
> NPE_MESSAGE_PENDING for example and pass it to super constructor in
> NPE no-arg constructor. Then instead of testing for
> lazyComputeMessage, you could test for super.getMessage() ==
> NPE_MESSAGE_PENDING...
>
> Not that this buys much space as NPEs are not numbered. Just a thought...
>
>
> Regards, Peter
>
> 2019-02-08 14:39 GMT+01.00, Lindenmaier, Goetz <goetz.lindenmaier at sap.com>:
>> Hi,
>>
>> ok, so here a new webrev just adding a setter for the field:
>> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/02
>>
>> ... and incorporating the other comments.
>>
>> Best regards,
>>   Goetz.
>>
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: David Holmes <david.holmes at oracle.com>
>>> Sent: Freitag, 8. Februar 2019 13:31
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>>> dev at openjdk.java.net; Java Core Libs <core-libs-dev at openjdk.java.net>
>>> Subject: Re: RFR(L): 8218628: Add detailed message to
>>> NullPointerException
>>> describing what is null.
>>>
>>> Hi Goetz,
>>>
>>> Just one follow up for now:
>>>
>>> >   * Add package visible "void setMessage (String msg)" to Throwable.
>>>
>>> Yes, just use package accessibility to deal with this, no need to jump
>>> through hoops (or the VM :) ).
>>>
>>> Thanks,
>>> David
>>>
>>> On 8/02/2019 9:51 pm, Lindenmaier, Goetz wrote:
>>> > Hi David,
>>> >
>>> >> Hi Volker,
>>> > ... I assume Volker could have contributed this as well, but actually
>>> > I must mention Ralf Schmelter as the original author of this :)
>>> >
>>> >> You know I'm not going to be a big fan of this :), but as long as we
>>> >> don't pay for it if we don't want it, then that's okay. (I'm still
>>> >> trying to gauge that)
>>> >>
>>> >> I have a little test for this that I ran through your patch:
>>> >>
>>> >> public class NPE {
>>> >>     static class B {
>>> >>       C b() { return null; }
>>> >>     }
>>> >>     static class C {
>>> >>       int c(Object o, String s) {  return 0; }
>>> >>     }
>>> >>
>>> >>     public static void main(String[] args) {
>>> >>       int x  = a().b().c(d(), e().toString());
>>> >>     }
>>> >>
>>> >>     static B a() { return null; }
>>> >>     static Object d() { return null; }
>>> >>     static Object e() { return null; }
>>> >>
>>> >> }
>>> >>
>>> >> and the result was a bit confusing for me:
>>> >>
>>> >> java.lang.NullPointerException: while trying to invoke the method
>>> >> NPE$B.b(()LNPE$C;) of a null object returned from NPE.a(()LNPE$B;)
>>> >>
>>> >> The placement and format of the return type descriptors obfuscates
>>> >> things to me - especially the Lxxx; format. Can we make that more
>>> >> Java
>>> >> programmer friendly eg:
>>> >>
>>> >> "while trying to invoke the method 'NPE$C NPE$B.b()' ..."
>>> >>
>>> >> though I think trying to produce signatures within the message is
>>> >> going
>>> >> to be very awkward in the general case. The key part is the "method
>>> >> NPE.b() ... returned from NPE.a()"
>>> > Actually, I have left out code that changes the signatures to the
>>> > Java source code wording. I already left that out in my former
>>> > exception message contributions.  For example see the messages in
>>> > jtreg/runtime/exceptionMsgs/IllegalAccessError/IllegalAccessErrorTest.java,
>>> > they have the same bad format:
>>> > "class test.Runner4 tried to access private method
>>> test.IllegalAccessErrorTest.iae4_m()V"
>>> >
>>> > I would like to fix them altogether in a follow-up, is that acceptable
>>> > to
>>> > you?
>>> >
>>> >> Also "of a null object" would read better as "on a null reference".
>>> > Makes sense, fixed.
>>> >
>>> > But I'm not that sure about changing these:
>>> > "while trying to read the field '%s' of a null object"
>>> > --> "while trying to read the field '%s' from a null reference"
>>> > "while trying to write the field %s of a null object"
>>> > --> "while trying to write the field %s  of a null reference"
>>> >
>>> >> First you will need to file a CSR request for the new product flags.
>>> > I'm not sure whether I need the product flags altogether. I would
>>> > prefer removing them.
>>> >
>>> >> Second, I don't understand why you need to call into the VM with
>>> >> JVM_SetDefaultMessage, to set a field in the Java object? Why isn't
>>> >> that
>>> >> done in Java?
>>> > Obviously, the problem is that the field is private.
>>> > As Christoph points out, there are several ways to implement this.
>>> > Please give advice:
>>> >    * reflection
>>> >    * shared secret
>>> >    * Add package visible "void setMessage (String msg)" to Throwable.
>>> >
>>> > Best regards,
>>> >    Goetz
>>> >
>>> >
>>> >>
>>> >> Thanks,
>>> >> David
>>> >>
>>> >> On 8/02/2019 2:43 am, Lindenmaier, Goetz wrote:
>>> >>> Hi,
>>> >>>
>>> >>> since Java 5, our internal VM reports verbose null pointer exception
>>> >>> messages. I would like to contribute this feature to OpenJDK.
>>> >>>
>>> >>> With this change, messages as
>>> >>>      "java.lang.NullPointerException: while trying to load from a
>>> >>> null
>>> >>> int
>>> array
>>> >> loaded from local variable 'ia1'"
>>> >>> are printed.  For more examples see the JBS bug or the test
>>> >>> included.
>>> >>> https://bugs.openjdk.java.net/browse/JDK-8218628
>>> >>>
>>> >>> The messages are generated by parsing the bytecodes. For not to have
>>> >>> any
>>> >> overhead when the
>>> >>> NPE is allocated, the message is only generated when it is accessed
>>> >>> by
>>> >> getMessage() or
>>> >>> serialization. For this I added a field to NPE to indicate that the
>>> >>> message
>>> still
>>> >> needs to be
>>> >>> computed lazily.
>>> >>>
>>> >>> Please review:
>>> >>> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/01/
>>> >>> I'm happy to incorporate your comments.
>>> >>>
>>> >>> Best regards,
>>> >>>     Goetz
>>> >>>
>>> >>>
>>
>


More information about the core-libs-dev mailing list