On Wed, 6 Aug 2025 21:41:27 GMT, Alisen Chung <ach...@openjdk.org> wrote:
>> Phil Race has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8277585
>
> src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java
> line 238:
>
>> 236: private volatile boolean disposed;
>> 237:
>> 238: public FileCacheDisposerRecord(File cacheFile, RandomAccessFile
>> cache) {
>
> FileCacheDisposer makes more sense based on what the code does, but would
> StreamDisposerRecord be better to remain consistent for the input/output
> streams?
Sure, see comment below.
> src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java
> line 249:
>
>> 247: }
>> 248: try {
>> 249: cache.close();
>
> I notice a similar nested class/method in FileCacheImageInputStream with a
> few differences. Does the cache.close() and cacheFile.delete need to be
> wrapped in null checks? And once deleted do the StreamDisposerRecord pointers
> (this.cacheFile and this.cache) need to be set to null like in the
> ImageInputStream version of the code?
Yes, I could re-use that. I just need to make it accessible .. but not public.
That one has null checks because it clears the vars.
My version uses a "disposed" var for the same since I made those vars final.
> src/java.desktop/share/classes/javax/imageio/stream/package-info.java line 60:
>
>> 58: * {@snippet lang='java':
>> 59: * try (FileOutputStream fos = new FileOutputStream("out.jpg");
>> 60: * (ImageOutputStream ios = new FileCacheImageOutputStream(fos,
>> null)) {
>
> I think there is an extra left parenthesis here at the start of the line
fixed
> src/java.desktop/share/classes/javax/imageio/stream/package-info.java line 66:
>
>> 64: * }
>> 65: * <p>
>> 66: * Sub-classers of these Image I/O API stream types can to a limited
>> extent protect
>
> Commas missing here
> `Sub-classers of these Image I/O API stream types can, to a limited extent,
> protect`
ok
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258441776
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258441819
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258443191
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258447549
