On Thu, 18 Dec 2025 13:36:12 GMT, David Beaumont <[email protected]> wrote:
> Tidying up syncrhonization around shared image.
src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 408:
> 406: synchronized (OPEN_FILES) {
> 407: ReaderKey key = new ReaderKey(imagePath, previewMode);
> 408: SharedImageReader reader = OPEN_FILES.get(key);
I changed the name of this for clarity. It's a bit confusing to have two "image
reader" instances in scope with one called "image" and one called just "reader".
src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 444:
> 442: nodes.clear();
> 443: }
> 444: super.close();
I will double check if moving this outside the synchronization of OPEN_FILES is
an issue.
The underlying BasicImageReader using a file channel, which is close (with
locking) in this close() method.
The vague worry I have is that now the outter OPEN_FILES lock isn't held, we
can get a race where the same file has a file channel being closed as a new one
is being opened, and I'm not 100% sure I know if that's safe.
Moving this back into the OPEN_FILES lock is possible, but leaves this code
doing more work with the locks held, which I'm inclined to avoid if possible.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1828#discussion_r2631177032
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1828#discussion_r2631195409
