On Thu, 11 Sep 2025 10:45:43 GMT, David Beaumont <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Feedback.
>
> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 382:
>
>> 380:
>> 381: /** Returns whether a resource exists in the given module. */
>> 382: boolean containsResource(String moduleName, String
>> resourcePath) {
>
> I *think* I can drop synchronization here since I'm not using the nodes cache
> in this method. However I'm going to triple check that BasicImageReader is
> thread safe before going with this (it's not documented either way).
I've convinced myself by looking at the code that BasicImageReader is thread
safe as it stands, so I'm confident the locking isn't needed here (but in
preview mode in Valhalla some locking will have to come back). Please speak up
now if you have any reason to suspect I'm wrong...
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2341844363
