On Wed, 6 Aug 2025 19:41:41 GMT, Phil Race <p...@openjdk.org> wrote:
>> This PR removes javax/imageio/stream/ImageInputStreamImpl.finalize()
>> As a result, sub-classes which over-ride it to be empty no longer need to do
>> so.
>> Also it means that the 2 remaining classes which used it no longer can.
>> FileCacheImageOutputStream will have its cache cleaned up by a disposer.
>> The impact on applications is that they, or the ImageWriter may need to call
>> flush() IF they relied on finalization.
>> However that should be extremely unlikely given that finalization will
>> happen far too late in most cases, and is
>> really meant to clean up internal resources.
>> The JDK's GIF and TIFF image writers don't flush themselves, so applications
>> which use these together with one of these caching streams would have
>> learned this already.
>>
>> The principal outside risk is to 3rd party ImageIO stream subclasses which
>> both allocate native resources and rely on finalization as a backstop clean
>> up in case applications forget to call close. But it will be the
>> applications that are affected if the resource is depleted.
>> The risks of this will be covered in the CSR.
>>
>> There's also a lengthy write up in the JBS issue.
>
> 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?
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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258381256
PR Review Comment: https://git.openjdk.org/jdk/pull/26650#discussion_r2258358972
