On Wed, 12 Apr 2023 08:09:42 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. > > Jeremy has updated the pull request incrementally with one additional commit > since the last revision: > > 4200096: rewrite solution, only print stacktrace if theConsumer != null > > I don't like that we're *expecting* a NullPointerException to get thrown in > the course of an allowed use case, but that's probably just a preferences on > my part. More importantly: this branch passes all new unit tests. Plus it's > readable and easy to code review. > > This is related to the latest round of PR feedback here: > https://github.com/openjdk/jdk/pull/13408#issuecomment-1504605358
I just pushed an update that uses the null-check for `theConsumer`, but it otherwise still expects the NPE. Regarding the System property / debugger message: If we established that removing the listener before production is complete is an OK thing to do, then do you still want this debugger output? (Or to put that another way: do you think that will really get used?) I don't have a lot of experience with debugger output from sun classes, so if you think it's helpful I'll take your word for it. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13408#issuecomment-1504854806
