8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs #26650

prrace · 2025-08-05T21:17:36Z

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.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8364778 to be approved
Commit message must refer to an issue

Issues

JDK-8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs (Bug - P4)
JDK-8364778: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs (CSR)

Reviewers

Alisen Chung (@alisenchung - Committer)
Alexander Zvegintsev (@azvegint - Reviewer)
Sergey Bylokhov (@mrserb - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26650/head:pull/26650
$ git checkout pull/26650

Update a local copy of the PR:
$ git checkout pull/26650
$ git pull https://git.openjdk.org/jdk.git pull/26650/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26650

View PR using the GUI difftool:
$ git pr show -t 26650

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26650.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-08-05T21:18:19Z

👋 Welcome back prr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-08-05T21:19:31Z

@prrace This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs

Reviewed-by: achung, azvegint, serb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 134 new commits pushed to the master branch:

e04a310: 8364806: Test sun/security/krb5/config/IncludeRandom.java times out on Windows
ec7361e: 8365660: test/jdk/sun/security/pkcs11/KeyAgreement/ tests skipped without SkipExceprion
a005301: 8364780: Unicode extension clarifications for NumberFormat/DecimalFormatSymbols
... and 131 more: https://git.openjdk.org/jdk/compare/68a35511ebd3fd77716242db973104657bc7b541...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-08-05T21:20:11Z

@prrace The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-08-05T21:26:44Z

Webrevs

prrace · 2025-08-05T21:58:45Z

/label car

prrace · 2025-08-05T21:58:59Z

/label csr

openjdk · 2025-08-05T22:00:38Z

@prrace
The label car is not a valid label.
These labels are valid:

graal
serviceability
hotspot
hotspot-compiler
ide-support
i18n
shenandoah
jdk
javadoc
security
hotspot-runtime
jmx
build
nio
client
core-libs
compiler
net
hotspot-gc
hotspot-jfr

openjdk · 2025-08-05T22:00:43Z

@prrace
The label csr is not a valid label.
These labels are valid:

graal
serviceability
hotspot
hotspot-compiler
ide-support
i18n
shenandoah
jdk
javadoc
security
hotspot-runtime
jmx
build
nio
client
core-libs
compiler
net
hotspot-gc
hotspot-jfr

src/java.desktop/share/classes/javax/imageio/stream/package-info.java

src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java

alisenchung · 2025-08-06T21:32:22Z

src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java

+                return;
+            }
+            try {
+                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.

Use of final is recommended wherever possible, as the values are then guaranteed to be visible on other threads (e.g. the disposer thread).

However IIUC, values set in a DisposerRecord by a program thread should (already) be visible on the disposer thread because Disposer.add() and Disposer.run() (via records.put() and records.remove(), respectively) lock the same records Hashtable object.

alisenchung · 2025-08-06T21:41:27Z

src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java

+        private final RandomAccessFile cache;
+        private volatile boolean disposed;
+
+        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.

alisenchung · 2025-08-06T21:56:26Z

src/java.desktop/share/classes/javax/imageio/stream/package-info.java

+ * A simple pattern would be
+ * {@snippet lang='java':
+ * try (FileOutputStream fos = new FileOutputStream("out.jpg");
+ *     (ImageOutputStream ios = new FileCacheImageOutputStream(fos, null)) {


I think there is an extra left parenthesis here at the start of the line

alisenchung · 2025-08-06T21:57:44Z

src/java.desktop/share/classes/javax/imageio/stream/package-info.java

+ * } // implicit finally block closes the streams in the reverse order to opening
+ * }
+ * <p>
+ * 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

bchristi-git

Can MemoryCacheImageOutputStream be included in this PR?

prrace · 2025-08-08T19:39:52Z

Can MemoryCacheImageOutputStream be included in this PR?

Why ? It has nothing to dispose.

bchristi-git · 2025-08-08T19:48:31Z

Can MemoryCacheImageOutputStream be included in this PR?

Why ? It has nothing to dispose.

Doesn't it have a cache, similar to MemoryCacheImageInputStream ?

public class MemoryCacheImageOutputStream extends ImageOutputStreamImpl {
....
private MemoryCache cache = new MemoryCache();

prrace · 2025-08-08T20:04:00Z

Can MemoryCacheImageOutputStream be included in this PR?

Why ? It has nothing to dispose.

Doesn't it have a cache, similar to MemoryCacheImageInputStream ?

public class MemoryCacheImageOutputStream extends ImageOutputStreamImpl { .... private MemoryCache cache = new MemoryCache();

That just wraps an ArrayList<byte[]>
I was planning on later removing the disposer from the MemoryCacheImageInputStream

prrace · 2025-08-10T19:03:12Z

The CSR is ready for review https://bugs.openjdk.org/browse/JDK-8364778

prrace · 2025-08-19T20:03:10Z

/integrate

openjdk · 2025-08-19T20:03:55Z

Going to push as commit 0858743.
Since your change was applied there have been 145 commits pushed to the master branch:

884076f: 8365719: Refactor uses of JLA.uncheckedNewStringNoRepl
024292a: 8365614: JFR: Improve PrettyWriter::printValue
0755477: 8342705: Add dark mode for docs
... and 142 more: https://git.openjdk.org/jdk/compare/68a35511ebd3fd77716242db973104657bc7b541...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-08-19T20:04:17Z

@prrace Pushed as commit 0858743.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8277585

eba4aef

openjdk bot changed the title ~~8277585~~ 8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs Aug 5, 2025

openjdk bot added the client [email protected] label Aug 5, 2025

8277585

edbbbd4

openjdk bot added the rfr Pull request is ready for review label Aug 5, 2025

openjdk bot added the csr Pull request needs approved CSR before integration label Aug 5, 2025

bchristi-git suggested changes Aug 6, 2025

View reviewed changes

src/java.desktop/share/classes/javax/imageio/stream/package-info.java Outdated Show resolved Hide resolved

src/java.desktop/share/classes/javax/imageio/stream/FileCacheImageOutputStream.java Outdated Show resolved Hide resolved

8277585

8aabb9d

alisenchung reviewed Aug 6, 2025

View reviewed changes

8277585

7d42251

bchristi-git suggested changes Aug 8, 2025

View reviewed changes

alisenchung approved these changes Aug 8, 2025

View reviewed changes

azvegint approved these changes Aug 11, 2025

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Aug 15, 2025

mrserb approved these changes Aug 19, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Aug 19, 2025

openjdk bot closed this Aug 19, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Aug 19, 2025

8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs #26650

8277585: Remove the terminally deprecated finalize() method from javax.imageio.stream APIs #26650

Conversation

prrace commented Aug 5, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Aug 5, 2025

Uh oh!

openjdk bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Aug 5, 2025

Uh oh!

mlbridge bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

prrace commented Aug 5, 2025

Uh oh!

prrace commented Aug 5, 2025

Uh oh!

openjdk bot commented Aug 5, 2025

Uh oh!

openjdk bot commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bchristi-git left a comment

Choose a reason for hiding this comment

Uh oh!

prrace commented Aug 8, 2025

Uh oh!

bchristi-git commented Aug 8, 2025

Uh oh!

prrace commented Aug 8, 2025

Uh oh!

prrace commented Aug 10, 2025

Uh oh!

prrace commented Aug 19, 2025

Uh oh!

openjdk bot commented Aug 19, 2025

Uh oh!

openjdk bot commented Aug 19, 2025

Uh oh!

Uh oh!

prrace commented Aug 5, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Aug 5, 2025 •

edited

Loading

mlbridge bot commented Aug 5, 2025 •

edited

Loading