RFR: 8360037: Refactor ImageReader in preparation for Valhalla support [v15]

[Alan Bateman]

On Tue, 29 Jul 2025 17:52:27 GMT, David Beaumont <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 257:
>> 
>>> 255:     private static Stream<ModuleInfo.Attributes> allModuleAttributes() {
>>> 256:         // System reader is a singleton and should not be closed by callers.
>>> 257:         ImageReader reader = SystemImage.reader();
>> 
>> The changes to SystemModuleFinders looks okay but I think we should drop the "System reader is a singleton and should not be closed by callers" here as it sets doubt that the ImageReader might be closed.
>
> 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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2267081379