Skip to content

Commit 4539266

Browse files
rbrirbri
committed
DownloadedContent.OnFile and ImageIOImageData now use Cleaner instead of finalize()
fix a possible resource leak by making sure ImageIOImageData closes the base stream
1 parent e797ef0 commit 4539266

7 files changed

Lines changed: 116 additions & 40 deletions

File tree

‎src/changes/changes.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88

99
<body>
1010
<release version="5.4.0" date="August xx, 2026" description="Firefox 153, Bugfixes">
11+
<action type="fix" dev="rbri">
12+
Fix a possible resource leak by making sure ImageIOImageData closes the base stream.
13+
</action>
14+
<action type="update" dev="rbri">
15+
DownloadedContent.OnFile and ImageIOImageData now use Cleaner instead of finalize().
16+
</action>
1117
<action type="update" dev="rbri" issue="#1152">
1218
Improved headers for XHR preflight requests.
1319
</action>

‎src/main/java/org/htmlunit/DownloadedContent.java‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.IOException;
2020
import java.io.InputStream;
2121
import java.io.Serializable;
22+
import java.lang.ref.Cleaner;
2223
import java.nio.file.Files;
2324

2425
import org.apache.commons.io.FileUtils;
@@ -72,8 +73,25 @@ public long length() {
7273
* Implementation keeping content on the file system.
7374
*/
7475
class OnFile implements DownloadedContent {
76+
7577
private final File file_;
76-
private final boolean temporary_;
78+
private final Cleaner.Cleanable cleanable_;
79+
80+
private static final class OnFileCleaningAction implements Runnable {
81+
private File file_;
82+
83+
OnFileCleaningAction(final File file) {
84+
file_ = file;
85+
}
86+
87+
@Override
88+
public synchronized void run() {
89+
if (file_ != null) {
90+
FileUtils.deleteQuietly(file_);
91+
file_ = null;
92+
}
93+
}
94+
}
7795

7896
/**
7997
* Ctor.
@@ -83,7 +101,12 @@ class OnFile implements DownloadedContent {
83101
*/
84102
OnFile(final File file, final boolean temporary) {
85103
file_ = file;
86-
temporary_ = temporary;
104+
if (temporary) {
105+
cleanable_ = WebClient.registerCleanerAction(this, new OnFileCleaningAction(file));
106+
}
107+
else {
108+
cleanable_ = null;
109+
}
87110
}
88111

89112
@Override
@@ -93,8 +116,8 @@ public InputStream getInputStream() throws IOException {
93116

94117
@Override
95118
public void cleanUp() {
96-
if (temporary_) {
97-
FileUtils.deleteQuietly(file_);
119+
if (cleanable_ != null) {
120+
cleanable_.clean();
98121
}
99122
}
100123

@@ -103,17 +126,12 @@ public boolean isEmpty() {
103126
return false;
104127
}
105128

106-
@Override
107-
protected void finalize() throws Throwable {
108-
super.finalize();
109-
cleanUp();
110-
}
111-
112129
@Override
113130
public long length() {
114131
if (file_ == null) {
115132
return 0;
116133
}
134+
117135
return file_.length();
118136
}
119137
}

‎src/main/java/org/htmlunit/WebClient.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import java.io.InputStream;
2525
import java.io.ObjectInputStream;
2626
import java.io.Serializable;
27+
import java.lang.ref.Cleaner;
28+
import java.lang.ref.Cleaner.Cleanable;
2729
import java.lang.ref.WeakReference;
2830
import java.net.MalformedURLException;
2931
import java.net.URL;
@@ -162,6 +164,8 @@ public class WebClient implements Serializable, AutoCloseable {
162164
private static final WebResponseData RESPONSE_DATA_NO_HTTP_RESPONSE = new WebResponseData(
163165
0, "No HTTP Response", Collections.emptyList());
164166

167+
static final Cleaner CLEANER = Cleaner.create();
168+
165169
/**
166170
* These response headers are not copied from a 304 response to the cached
167171
* response headers. This list is based on Chromium http_response_headers.cc
@@ -1362,6 +1366,18 @@ public void deregisterWebWindow(final WebWindow webWindow) {
13621366
}
13631367
}
13641368

1369+
/**
1370+
* Registers an object and a cleaning action to run when the object
1371+
* becomes phantom reachable. This forwards the call to our static {@link Cleaner}.
1372+
*
1373+
* @param obj the object to monitor
1374+
* @param action a {@code Runnable} to invoke when the object becomes phantom reachable
1375+
* @return a {@code Cleanable} instance
1376+
*/
1377+
public static Cleanable registerCleanerAction(final Object obj, final Runnable action) {
1378+
return CLEANER.register(obj, action);
1379+
}
1380+
13651381
/**
13661382
* Expands a relative URL relative to the specified base. In most situations
13671383
* this is the same as <code>new URL(baseUrl, relativeUrl)</code> but

‎src/main/java/org/htmlunit/platform/image/ImageIOImageData.java‎

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616

1717
import java.io.IOException;
1818
import java.io.InputStream;
19+
import java.lang.ref.Cleaner;
1920
import java.util.Iterator;
2021

2122
import javax.imageio.ImageIO;
2223
import javax.imageio.ImageReader;
2324
import javax.imageio.stream.ImageInputStream;
2425

26+
import org.htmlunit.WebClient;
2527
import org.htmlunit.platform.geom.IntDimension2D;
2628

2729
/**
@@ -47,7 +49,56 @@ public class ImageIOImageData implements ImageData {
4749

4850
// private static final Log LOG = LogFactory.getLog(ImageIOImageData.class);
4951

50-
private final ImageReader imageReader_;
52+
private IntDimension2D dim_;
53+
54+
private final ImageIOImageDataCleaningAction cleaningAction_;
55+
private final Cleaner.Cleanable cleanable_;
56+
57+
private static final class ImageIOImageDataCleaningAction implements Runnable {
58+
private ImageReader imageReader_;
59+
private InputStream inputStream_;
60+
61+
ImageIOImageDataCleaningAction(final ImageReader imageReader, final InputStream inputStream) {
62+
imageReader_ = imageReader;
63+
inputStream_ = inputStream;
64+
}
65+
66+
synchronized ImageReader getImageReader() {
67+
if (imageReader_ == null) {
68+
throw new IllegalStateException("ImageIOImageData is closed");
69+
}
70+
return imageReader_;
71+
}
72+
73+
@Override
74+
public synchronized void run() {
75+
if (imageReader_ == null) {
76+
return;
77+
}
78+
79+
try (ImageInputStream stream = (ImageInputStream) imageReader_.getInput()) {
80+
// nothing
81+
}
82+
catch (final IOException e) {
83+
// optionally log
84+
}
85+
finally {
86+
imageReader_.setInput(null);
87+
imageReader_.dispose();
88+
imageReader_ = null;
89+
}
90+
91+
try {
92+
inputStream_.close();
93+
}
94+
catch (final IOException e) {
95+
// optionally log
96+
}
97+
finally {
98+
inputStream_ = null;
99+
}
100+
}
101+
}
51102

52103
/**
53104
* Ctor.
@@ -59,12 +110,15 @@ public ImageIOImageData(final InputStream inputStream) throws IOException {
59110
final Iterator<ImageReader> iter = ImageIO.getImageReaders(iis);
60111
if (!iter.hasNext()) {
61112
iis.close();
113+
inputStream.close();
62114
throw new IOException("No image detected in response");
63115
}
116+
64117
final ImageReader imageReader = iter.next();
65118
imageReader.setInput(iis);
66119

67-
imageReader_ = imageReader;
120+
cleaningAction_ = new ImageIOImageDataCleaningAction(imageReader, inputStream);
121+
cleanable_ = WebClient.registerCleanerAction(this, cleaningAction_);
68122

69123
// dispose all others
70124
while (iter.hasNext()) {
@@ -78,37 +132,26 @@ public ImageIOImageData(final InputStream inputStream) throws IOException {
78132
* @return the {@link ImageReader}
79133
*/
80134
public ImageReader getImageReader() {
81-
return imageReader_;
135+
return cleaningAction_.getImageReader();
82136
}
83137

84138
/**
85139
* {@inheritDoc}
86140
*/
87141
@Override
88142
public IntDimension2D getWidthHeight() throws IOException {
89-
return new IntDimension2D(imageReader_.getWidth(0), imageReader_.getHeight(0));
143+
if (dim_ == null) {
144+
final ImageReader imgReader = cleaningAction_.getImageReader();
145+
dim_ = new IntDimension2D(imgReader.getWidth(0), imgReader.getHeight(0));
146+
}
147+
return dim_;
90148
}
91149

92150
/**
93151
* {@inheritDoc}
94152
*/
95153
@Override
96-
protected void finalize() throws Throwable {
97-
close();
98-
super.finalize();
99-
}
100-
101-
@Override
102-
@SuppressWarnings("PMD.UnusedLocalVariable")
103-
public void close() throws IOException {
104-
if (imageReader_ != null) {
105-
try (ImageInputStream stream = (ImageInputStream) imageReader_.getInput()) {
106-
// nothing
107-
}
108-
finally {
109-
imageReader_.setInput(null);
110-
imageReader_.dispose();
111-
}
112-
}
154+
public void close() {
155+
cleanable_.clean();
113156
}
114157
}

‎src/main/java/org/htmlunit/platform/image/NoOpImageData.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,6 @@ public IntDimension2D getWidthHeight() throws IOException {
3737
return new IntDimension2D(0, 0);
3838
}
3939

40-
/**
41-
* {@inheritDoc}
42-
*/
43-
@Override
44-
protected void finalize() throws Throwable {
45-
close();
46-
super.finalize();
47-
}
48-
4940
/**
5041
* {@inheritDoc}
5142
*/

‎src/test/java/org/htmlunit/HttpWebConnection3Test.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,6 +3394,7 @@ public void xmlHttpRequestGet() throws Exception {
33943394
assertEquals(Arrays.asList(expectedHeaders).toString(), Arrays.asList(headers).toString());
33953395
}
33963396
}
3397+
33973398
/**
33983399
* Tests a cross-origin (cross-site) XMLHttpRequest GET - the counterpart to
33993400
* {@link #xmlHttpRequestGet()} (same-origin), mirroring the same-origin/cross-origin

‎src/test/java/org/htmlunit/archunit/ArchitectureTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ public void check(final JavaMethod method, final ConditionEvents events) {
543543
public static final ArchRule androidImageio = noClasses()
544544
.that()
545545
.doNotHaveFullyQualifiedName("org.htmlunit.platform.image.ImageIOImageData")
546+
.and().doNotHaveFullyQualifiedName("org.htmlunit.platform.image.ImageIOImageData$ImageIOImageDataCleaningAction")
546547
.and().doNotHaveFullyQualifiedName("org.htmlunit.platform.canvas.rendering.AwtRenderingBackend")
547548
.and().doNotHaveFullyQualifiedName("org.htmlunit.platform.canvas.rendering.AwtRenderingBackend")
548549
.and().resideOutsideOfPackage("org.htmlunit.jetty..")

0 commit comments

Comments
 (0)