...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.lev...@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.lindenma...@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.hol...@oracle.com> >>> Sent: Freitag, 8. Februar 2019 13:31 >>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; hotspot-runtime- >>> d...@openjdk.java.net; Java Core Libs <core-libs-dev@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 >>> >>> >>> >>> >> >
