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

[David Beaumont]

On Tue, 1 Jul 2025 14:29:28 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updates based on feedback.
>
> src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 62:
> 
>> 60: import jdk.internal.module.ModuleHashes.HashSupplier;
>> 61: 
>> 62: import static java.util.Objects.requireNonNull;
> 
> The existing Objects.requireNonNull are okay, no need to change them all as it is nothing to do with the changes.

Done.

> src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 251:
> 
>> 249:     }
>> 250: 
>> 251:     private static Stream<ModuleInfo.Attributes> getModuleAttributes() {
> 
> Can you rename this to allModuleAttributes and add a method description to match the other methods.

Done.

> src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 257:
> 
>> 255:                 .getChildNames()
>> 256:                 .map(mn -> getNode(reader, mn + "/module-info.class"))
>> 257:                 // This fails with ISE if the node isn't a resource (corrupt JImage).
> 
> Can you drop this comment and check getNode to thrown an Error (ISE isn't right when we have a broken image).

`getNode()` is called twice and while it can throw the error, there's no nice way for `getResourceBuffer()` to be that strict (it's a separate API).
After fiddling a bit I reworked the module attribute loading into a separate helper and caught & escalated any exceptions relating to that into `Error`.
The only question now is that `ModuleInfo.read()` has at least one other runtime exception (`InvalidModuleDescriptorException`) it can throw which we could catch here. Should I add that to the list?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2178463701
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2178463291
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2178462973