Skip to content

Commit

Permalink
Merge branch 'master' into issue_665
Browse files Browse the repository at this point in the history
  • Loading branch information
jmehrens authored Jan 4, 2024
2 parents 5acf47b + 6c9ac50 commit af596c3
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 46 deletions.
62 changes: 18 additions & 44 deletions api/src/main/java/jakarta/mail/util/SharedFileInputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.RandomAccessFile;
import java.util.Objects;

/**
* A <code>SharedFileInputStream</code> is a
Expand All @@ -34,15 +35,6 @@
* A <code>RandomAccessFile</code> object is used to
* access the file data. <p>
*
* Note that when the SharedFileInputStream is closed,
* all streams created with the <code>newStream</code>
* method are also closed. This allows the creator of the
* SharedFileInputStream object to control access to the
* underlying file and ensure that it is closed when
* needed, to avoid leaking file descriptors. Note also
* that this behavior contradicts the requirements of
* SharedInputStream and may change in a future release.
*
* @author Bill Shannon
* @since JavaMail 1.4
*/
Expand Down Expand Up @@ -78,12 +70,6 @@ public class SharedFileInputStream extends BufferedInputStream
*/
protected long datalen;

/**
* True if this is a top level stream created directly by "new".
* False if this is a derived stream created by newStream.
*/
private boolean master = true;

/**
* A shared class that keeps track of the references
* to a particular file so it can be closed when the
Expand Down Expand Up @@ -111,22 +97,8 @@ public synchronized void close() throws IOException {
in.close();
}

public synchronized void forceClose() throws IOException {
if (cnt > 0) {
// normal case, close exceptions propagated
cnt = 0;
in.close();
} else {
// should already be closed, ignore exception
try {
in.close();
} catch (IOException ioex) {
}
}
}

@Override
protected void finalize() throws Throwable {
protected synchronized void finalize() throws Throwable {
try {
in.close();
} finally {
Expand Down Expand Up @@ -214,7 +186,6 @@ private void init(SharedFile sf, int size) throws IOException {
private SharedFileInputStream(SharedFile sf, long start, long len,
int bufsize) {
super(null);
this.master = false;
this.sf = sf;
this.in = sf.open();
this.start = start;
Expand Down Expand Up @@ -465,14 +436,12 @@ public void close() throws IOException {
if (in == null)
return;
try {
if (master)
sf.forceClose();
else
sf.close();
sf.close();
} finally {
sf = null;
in = null;
buf = null;
Objects.requireNonNull(this); //TODO: replace with Reference.reachabilityFence
}
}

Expand Down Expand Up @@ -505,14 +474,19 @@ public long getPosition() {
*/
@Override
public synchronized InputStream newStream(long start, long end) {
if (in == null)
throw new RuntimeException("Stream closed");
if (start < 0)
throw new IllegalArgumentException("start < 0");
if (end == -1)
end = datalen;
return new SharedFileInputStream(sf,
this.start + start, end - start, bufsize);
try {
if (in == null)
throw new RuntimeException("Stream closed");
if (start < 0)
throw new IllegalArgumentException("start < 0");
if (end == -1)
end = datalen;

return new SharedFileInputStream(sf,
this.start + start, end - start, bufsize);
} finally {
Objects.requireNonNull(this); //TODO: replace with Reference.reachabilityFence
}
}

// for testing...
Expand All @@ -537,7 +511,7 @@ public static void main(String[] argv) throws Exception {
* Force this stream to close.
*/
@Override
protected void finalize() throws Throwable {
protected synchronized void finalize() throws Throwable {
super.finalize();
close();
}
Expand Down
65 changes: 65 additions & 0 deletions api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright (c) 2021, 2023 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package jakarta.mail.util;

import org.junit.Test;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;

import static org.junit.Assert.fail;

/**
* Please note:
* In version 2.1.2 Final Release, a difference in test results was observed based on the choice of assertion method.
* Invoking stream.read() directly yielded distinct outcomes compared to using Assertions.assertDoesNotThrow() from JUnit 5.
* This divergence is likely attributed to scope of the code.
* After the patch, this issue did not reoccur. However, it remains essential to be attentive
* If any changes are deemed necessary, please ensure a comprehensive review and thorough testing to uphold the original behavior.
*/
public class SharedFileInputStreamTest {

@Test
public void testChild() throws Exception {
File file = File.createTempFile(SharedFileInputStreamTest.class.getName(), "testChild");

try (InputStream childStream = new SharedFileInputStream(file).newStream(0, -1)) {
System.gc();
childStream.read();
} catch (IOException e) {
fail("IOException is not expected");
} finally {
file.delete();
}
}


@Test
public void testGrandChild() throws Exception {
File file = File.createTempFile(SharedFileInputStreamTest.class.getName(), "testGrandChild");

try (InputStream grandChild = ((SharedFileInputStream) new SharedFileInputStream(file).newStream(0, -1)).newStream(0, -1)) {
System.gc();
grandChild.read();
} catch (IOException e) {
fail("IOException is not expected");
} finally {
file.delete();
}
}
}
5 changes: 3 additions & 2 deletions doc/release/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ Bug IDs that start with "G" can be found in the GlassFish Issue Tracker
Seven digit bug numbers are from the old Sun bug database, which is no
longer available.

CHANGES IN THE 2.1.3 RELEASE
----------------------------
CHANGES IN THE 2.1.3 RELEASE
----------------------------
E 695 SharedFileInputStream should comply with spec
E 665 Jakarta Mail erroneously assumes that classes can be loaded from Thread#getContextClassLoader


Expand Down
9 changes: 9 additions & 0 deletions www/docs/COMPAT.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
Jakarta Mail API 2.0.1 release
------------------------------

-- Jakarta Mail 2.1.3 --

- SharedFileInputStream should comply with spec

The root SharedFileInputStream no longer closes all streams
created with the newStream. This behavior was not compliant with
the contract specified in the SharedInputStream interface
which specifies that all streams must be closed before the shared resource is closed.

-- Jakarta Mail 2.0.0 --

The Jakarta Mail 2.0 specification is the successor of the Jakarta
Expand Down

0 comments on commit af596c3

Please sign in to comment.