On Mon, 11 Aug 2025 15:09:56 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> I mean without that, as a reader of the code who doesn't know details, I'd >> definitely be questioning why a closeable object (ImageReader) is being >> obtained and used, but not closed. I probably even added it because I'd had >> to dig around the code to understand that it was actually correct to not >> close it here. >> >> Perhaps the docs for SystemImage (currently `Holder class for the >> ImageReader.` could be improved to make the lifecycle clear and benefit all >> callers of `reader()` ?) > > Maybe, or the singleton returned by SystemImage.reader() is non-closeable. My > only comment for now is tha the "should not be called by callers." is > confusing here and would be better to remove. We do do this in the next > change in the area if you want. The comment is there because that line is: a) right above a try-catch block and b) comes with a "click here to put this into a try-with-resources block" lint action in an IDE So, at a glance, it looks like it's a simple mistake that it's not a resource in the following try-block. It just feels tempting for a future maintainer to mistakenly "fix it". That's the "job" of the comment, to catch when someone is about to do that. I reworded slightly though. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2267466083
