leaf-soba commented on PR #18684: URL: https://github.com/apache/kafka/pull/18684#issuecomment-2692077881
> @leaf-soba, thanks for the patch! I agree that the existing code does theoretically expose a potential NPE, though I'm not sure it's exactly a problem. > > To my knowledge, the map in Snapshot is only "erased" once it is about to be disposed of. In other words, we don't reuse Snapshot objects. Once erased, it should be impossible for the object to be accessed except from the call site which did the erase. > > However, since we can't guarantee that the caller properly disposes of the object and does not make any further calls which touch `map`, it would be good to include some sanity checks. > > Instead of clearing the map (which may give the illusion of re-usability), maybe it's best to add a null check to the public methods. We also need some unit tests for any changes here. @mumrah, thanks for the feedback! I've added a null check to the public methods to prevent potential NPEs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
