RFR: 8372787: ModuleReader should throw IOException consistently when using --patch-module and ModuleReader is closed [v6]

[Jaikiran Pai]

On Mon, 1 Dec 2025 18:18:07 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Alan's suggestion - additional tests to the found resource URI
>
> src/java.base/share/classes/jdk/internal/module/ModulePatcher.java line 424:
> 
>> 422:         @Override
>> 423:         public void close() throws IOException {
>> 424:             closed = true;
> 
> It might be a bit better to avoid the closeAll+delegate if already closed.

Hello Alan, I've now updated the PR to return early if already closed. I have kept it a simple check and it won't take into account concurrent calls to `close()`. I think that's OK given what we discussed in this PR about async close. However, if you would like to have this be a bit more stronger (like we do with the `delegate` creation), then let me know and I will adjust this check in `close()` accordingly.

> test/jdk/java/lang/module/ModuleReader/patched/PatchedModuleReaderTest.java line 40:
> 
>> 38: /*
>> 39:  * @test
>> 40:  * @bug 8372787
> 
> Can you fix the `@summary` so that it starts with "Test the .." or "Verify the ..."

Done.

> test/jdk/java/lang/module/ModuleReader/patched/PatchedModuleReaderTest.java line 110:
> 
>> 108: 
>> 109:         // verify IOException is thrown by the closed ModuleReader
>> 110:         resources.forEach(rn -> {
> 
> The test should also check reader.list() throws IOException.

Good catch. It was there previously but I lost that check after I refactored the first version of this test. I've added it back now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2579872453
PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2579873014
PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2579879350