Skip to content

Commit 6c9ac50

Browse files
SeongEon-JoSeongEon-Jo
authored
SharedFileInputStream should comply with spec (#695)
1 parent 5bb4a5e commit 6c9ac50

File tree

4 files changed

+97
-44
lines changed

4 files changed

+97
-44
lines changed

‎api/src/main/java/jakarta/mail/util/SharedFileInputStream.java‎

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.IOException;
2424
import java.io.InputStream;
2525
import java.io.RandomAccessFile;
26+
import java.util.Objects;
2627

2728
/**
2829
* A <code>SharedFileInputStream</code> is a
@@ -34,15 +35,6 @@
3435
* A <code>RandomAccessFile</code> object is used to
3536
* access the file data. <p>
3637
*
37-
* Note that when the SharedFileInputStream is closed,
38-
* all streams created with the <code>newStream</code>
39-
* method are also closed. This allows the creator of the
40-
* SharedFileInputStream object to control access to the
41-
* underlying file and ensure that it is closed when
42-
* needed, to avoid leaking file descriptors. Note also
43-
* that this behavior contradicts the requirements of
44-
* SharedInputStream and may change in a future release.
45-
*
4638
* @author Bill Shannon
4739
* @since JavaMail 1.4
4840
*/
@@ -78,12 +70,6 @@ public class SharedFileInputStream extends BufferedInputStream
7870
*/
7971
protected long datalen;
8072

81-
/**
82-
* True if this is a top level stream created directly by "new".
83-
* False if this is a derived stream created by newStream.
84-
*/
85-
private boolean master = true;
86-
8773
/**
8874
* A shared class that keeps track of the references
8975
* to a particular file so it can be closed when the
@@ -111,22 +97,8 @@ public synchronized void close() throws IOException {
11197
in.close();
11298
}
11399

114-
public synchronized void forceClose() throws IOException {
115-
if (cnt > 0) {
116-
// normal case, close exceptions propagated
117-
cnt = 0;
118-
in.close();
119-
} else {
120-
// should already be closed, ignore exception
121-
try {
122-
in.close();
123-
} catch (IOException ioex) {
124-
}
125-
}
126-
}
127-
128100
@Override
129-
protected void finalize() throws Throwable {
101+
protected synchronized void finalize() throws Throwable {
130102
try {
131103
in.close();
132104
} finally {
@@ -214,7 +186,6 @@ private void init(SharedFile sf, int size) throws IOException {
214186
private SharedFileInputStream(SharedFile sf, long start, long len,
215187
int bufsize) {
216188
super(null);
217-
this.master = false;
218189
this.sf = sf;
219190
this.in = sf.open();
220191
this.start = start;
@@ -465,14 +436,12 @@ public void close() throws IOException {
465436
if (in == null)
466437
return;
467438
try {
468-
if (master)
469-
sf.forceClose();
470-
else
471-
sf.close();
439+
sf.close();
472440
} finally {
473441
sf = null;
474442
in = null;
475443
buf = null;
444+
Objects.requireNonNull(this); //TODO: replace with Reference.reachabilityFence
476445
}
477446
}
478447

@@ -505,14 +474,19 @@ public long getPosition() {
505474
*/
506475
@Override
507476
public synchronized InputStream newStream(long start, long end) {
508-
if (in == null)
509-
throw new RuntimeException("Stream closed");
510-
if (start < 0)
511-
throw new IllegalArgumentException("start < 0");
512-
if (end == -1)
513-
end = datalen;
514-
return new SharedFileInputStream(sf,
515-
this.start + start, end - start, bufsize);
477+
try {
478+
if (in == null)
479+
throw new RuntimeException("Stream closed");
480+
if (start < 0)
481+
throw new IllegalArgumentException("start < 0");
482+
if (end == -1)
483+
end = datalen;
484+
485+
return new SharedFileInputStream(sf,
486+
this.start + start, end - start, bufsize);
487+
} finally {
488+
Objects.requireNonNull(this); //TODO: replace with Reference.reachabilityFence
489+
}
516490
}
517491

518492
// for testing...
@@ -537,7 +511,7 @@ public static void main(String[] argv) throws Exception {
537511
* Force this stream to close.
538512
*/
539513
@Override
540-
protected void finalize() throws Throwable {
514+
protected synchronized void finalize() throws Throwable {
541515
super.finalize();
542516
close();
543517
}

‎api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright (c) 2021, 2023 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0, which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the
10+
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
11+
* version 2 with the GNU Classpath Exception, which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
*/
16+
17+
package jakarta.mail.util;
18+
19+
import org.junit.Test;
20+
21+
import java.io.File;
22+
import java.io.IOException;
23+
import java.io.InputStream;
24+
25+
import static org.junit.Assert.fail;
26+
27+
/**
28+
* Please note:
29+
* In version 2.1.2 Final Release, a difference in test results was observed based on the choice of assertion method.
30+
* Invoking stream.read() directly yielded distinct outcomes compared to using Assertions.assertDoesNotThrow() from JUnit 5.
31+
* This divergence is likely attributed to scope of the code.
32+
* After the patch, this issue did not reoccur. However, it remains essential to be attentive
33+
* If any changes are deemed necessary, please ensure a comprehensive review and thorough testing to uphold the original behavior.
34+
*/
35+
public class SharedFileInputStreamTest {
36+
37+
@Test
38+
public void testChild() throws Exception {
39+
File file = File.createTempFile(SharedFileInputStreamTest.class.getName(), "testChild");
40+
41+
try (InputStream childStream = new SharedFileInputStream(file).newStream(0, -1)) {
42+
System.gc();
43+
childStream.read();
44+
} catch (IOException e) {
45+
fail("IOException is not expected");
46+
} finally {
47+
file.delete();
48+
}
49+
}
50+
51+
52+
@Test
53+
public void testGrandChild() throws Exception {
54+
File file = File.createTempFile(SharedFileInputStreamTest.class.getName(), "testGrandChild");
55+
56+
try (InputStream grandChild = ((SharedFileInputStream) new SharedFileInputStream(file).newStream(0, -1)).newStream(0, -1)) {
57+
System.gc();
58+
grandChild.read();
59+
} catch (IOException e) {
60+
fail("IOException is not expected");
61+
} finally {
62+
file.delete();
63+
}
64+
}
65+
}

‎doc/release/CHANGES.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ Bug IDs that start with "G" can be found in the GlassFish Issue Tracker
1919
Seven digit bug numbers are from the old Sun bug database, which is no
2020
longer available.
2121

22+
CHANGES IN THE 2.1.3 RELEASE
23+
----------------------------
24+
E 695 SharedFileInputStream should comply with spec
25+
26+
2227
CHANGES IN THE 2.1.2 RELEASE
2328
----------------------------
2429
E 629 jakarta.mail-api-2.1.0.jar does not work in OSGi environment (hk2servicelocator)

‎www/docs/COMPAT.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@
44
Jakarta Mail API 2.0.1 release
55
------------------------------
66

7+
-- Jakarta Mail 2.1.3 --
8+
9+
- SharedFileInputStream should comply with spec
10+
11+
The root SharedFileInputStream no longer closes all streams
12+
created with the newStream. This behavior was not compliant with
13+
the contract specified in the SharedInputStream interface
14+
which specifies that all streams must be closed before the shared resource is closed.
15+
716
-- Jakarta Mail 2.0.0 --
817

918
The Jakarta Mail 2.0 specification is the successor of the Jakarta

0 commit comments

Comments
 (0)