On Mon, 1 Dec 2025 11:02:55 GMT, Jaikiran Pai <[email protected]> wrote:
>> 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. > > My original motivation to use `volatile` for `closed` was to match what we > have for `delegate`. But the volatile on that, I believe, is for a different > reason (for example: to avoid multiple `delegate` creations when different > entry points into the `ModuleReader` are used concurrently). > > In the latest update of the PR I have retained the `volatile` on `closed`, > but I'm leaning towards removing that. But I'll wait for your input. The access mode doesn't matter because it doesn't guard against an async close. That is okay because the spec doesn't require the ModuleReader to be asynchronously closeable. Give that --patch-module is mostly for testing then I think what you have is okay. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2577667780
