On Mon, 10 Apr 2023 16:16:32 GMT, Jeremy <d...@openjdk.org> wrote: > This resolves a 25 year old P4 ticket: a NullPointerException is printed to > System.err needlessly. > > This resolution involves confirming that an ImageConsumer is still registered > before every notification. > > I'll understand if this is rejected as unimportant, but I stumbled across > this in the real world the other day and thought this was a simple enough bug > to practice on.
So the repeated checks for null make it look like you are dealing with some other thread setting it to null If that were so, I don't think this approach is correct because the repeated null checks aren't a guarantee. It can still go null between you checking it and then de-referencing it. But I see that add/removeConsumer are synchronized methods And the private method sendPixels() is only called from the private method produce() which is only called from addConsumer() So it seems impossible for some other thread to do this. Therefore you must be doing it to yourself on the same thread. I tracked down that the catch of null and printStackTrace were added under this bug https://bugs.openjdk.org/browse/JDK-4905411 There it is implied that the callbacks such as theConsumer.imageComplete(ImageConsumer.SINGLEFRAMEDONE); are where the null is happening The person who suggested the fix (not the person who implemented the fix) was the person who architected the whole image producer/consumer model and near as I can tell the idea was that you have a programming bug removing the consumer too soon. I'd expect that you'd see this problem consistently since there's only one thread involved (unless the callback kicks off another thread to do it) It also points out that you're unlikely to need the repeated checks since it can only be nulled out during the call backs (again unless the callback starts another thread to do it - unlikely I think). If you think what you are doing is correct (I think the architect of the code supposed otherwise) and you want to be sure, I'd capture "theConsumer" into a local var in each of the methods sendPixel() and produce() and check and use the local var. Then no one can ever null that out while you are using it. Since that's the intention of the design, I think that's a safe thing to do. However I suggest that if the null check finds it is null on entry that there be an ImageConsumer localConsumer = theConsumer; if (localConsumer == null) { boolean debugging = <wrap in doprivleged get property("awt.consumer.debug")> if (debugging) { System.err.println("theConsumer is null - did you remove the consumer before production is complete ?"); e.printStackTrace()) } return } at the beginning of both methods ------------- PR Comment: https://git.openjdk.org/jdk/pull/13408#issuecomment-1502348061
