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 228: > 226: private final URL delegateCodeSourceURL; > 227: private volatile ModuleReader delegate; > 228: private volatile boolean closed; A ModuleReader is specified to not require to support async close, so need to think about whether the closed flag should be volatile or not. src/java.base/share/classes/jdk/internal/module/ModulePatcher.java line 420: > 418: } > 419: > 420: private void ensureOpen() throws IOException { Can you move this to after the delegate() method? test/jdk/java/lang/module/ModuleReader/patched/PatchedModuleReaderTest.java line 84: > 82: "ModuleReader.open(String)"); > 83: assertThrows(IOException.class, () -> reader.find(rn), > 84: "ModuleReader.find(String)"); Would it be possible to say which methods throw ISE or IOE? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576549408 PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576545166 PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576553111
