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 >> >>> >> >>> >
