On Mon, 1 Dec 2025 10:22:19 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to address the issue 
>> noted in https://bugs.openjdk.org/browse/JDK-8372787?
>> 
>> The commit in this PR takes into account the `IllegalStateException` thrown 
>> by `JarFile` APIs and wraps them into a `IOException` to conform with the 
>> expectations of the `ModuleReader` APIs.
>> 
>> A new jtreg test has been introduced to reproduce the issue and verify the 
>> fix. CI testing is currently in progress with this change.
>
> Jaikiran Pai has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - 8372787: ModuleReader should throw IOException consistently when using 
> --patch-module and ModuleReader is closed
>  - Revert "8372787: ModulePatcher throws unspecified IllegalStateException 
> upon being invoked after close()"
>    
>    This reverts commit 42cfa0aa40926b748c6d7bdf814d85ca1ccd8fed.

src/java.base/share/classes/jdk/internal/module/ModulePatcher.java line 314:

> 312:          */
> 313:         public Resource findResource(String name) throws IOException {
> 314:             ensureOpen();

Unlike the other methods which have been updated to call the `ensureOpen()`, 
this `findResource(...)` method isn't part of the `ModuleReader` interface. 
This only gets called from the `BuiltinClassLoader` when defining a `Class`. I 
looked up that code in `BuiltinClassLoader` and it already has a catch clause 
to handle `IOException`, so it felt right to add the `ensureOpen()` check here 
too.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576514762

Reply via email to