diff --git a/api/src/main/java/jakarta/mail/Folder.java b/api/src/main/java/jakarta/mail/Folder.java index 95a7784b..26bda8d5 100644 --- a/api/src/main/java/jakarta/mail/Folder.java +++ b/api/src/main/java/jakarta/mail/Folder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024 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 @@ -643,14 +643,21 @@ public abstract boolean delete(boolean recurse) * * This implementation calls close(true). * - * @throws IllegalStateException if this folder is not opened * @throws MessagingException for other failures * @see jakarta.mail.event.ConnectionEvent * @since JavaMail 1.6 */ @Override public void close() throws MessagingException { - close(true); + try { + close(true); + } catch (IllegalStateException ise) { + if (isOpen()) { + throw ise; + } + } finally { + q.terminateQueue(); + } } /** @@ -1641,15 +1648,6 @@ private void queueEvent(MailEvent event, q.enqueue(event, v); } - @Override - protected void finalize() throws Throwable { - try { - q.terminateQueue(); - } finally { - super.finalize(); - } - } - /** * override the default toString(), it will return the String * from Folder.getFullName() or if that is null, it will use diff --git a/api/src/main/java/jakarta/mail/Service.java b/api/src/main/java/jakarta/mail/Service.java index 6d8af32a..be380d5f 100644 --- a/api/src/main/java/jakarta/mail/Service.java +++ b/api/src/main/java/jakarta/mail/Service.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024 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 @@ -632,18 +632,6 @@ protected void queueEvent(MailEvent event, q.enqueue(event, v); } - /** - * Stop the event dispatcher thread so the queue can be garbage collected. - */ - @Override - protected void finalize() throws Throwable { - try { - q.terminateQueue(); - } finally { - super.finalize(); - } - } - /** * Package private method to allow Folder to get the Session for a Store. */ diff --git a/api/src/main/java/jakarta/mail/util/SharedFileInputStream.java b/api/src/main/java/jakarta/mail/util/SharedFileInputStream.java index e9c01589..4df19084 100644 --- a/api/src/main/java/jakarta/mail/util/SharedFileInputStream.java +++ b/api/src/main/java/jakarta/mail/util/SharedFileInputStream.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024 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 @@ -75,9 +75,9 @@ public class SharedFileInputStream extends BufferedInputStream * to a particular file so it can be closed when the * last reference is gone. */ - static class SharedFile { + static class SharedFile implements AutoCloseable { private int cnt; - private RandomAccessFile in; + RandomAccessFile in; SharedFile(String file) throws IOException { this.in = new RandomAccessFile(file, "r"); @@ -92,19 +92,11 @@ public synchronized RandomAccessFile open() { return in; } + @Override public synchronized void close() throws IOException { if (cnt > 0 && --cnt <= 0) in.close(); } - - @Override - protected synchronized void finalize() throws Throwable { - try { - in.close(); - } finally { - super.finalize(); - } - } } private SharedFile sf; @@ -265,10 +257,10 @@ private int read1(byte[] b, int off, int len) throws IOException { int avail = count - pos; if (avail <= 0) { if (false) { - /* If the requested length is at least as large as the buffer, and - if there is no mark/reset activity, do not bother to copy the - bytes into the local buffer. In this way buffered streams will - cascade harmlessly. */ + /* If the requested length is at least as large as the buffer, and + if there is no mark/reset activity, do not bother to copy the + bytes into the local buffer. In this way buffered streams will + cascade harmlessly. */ if (len >= buf.length && markpos < 0) { // XXX - seek, update bufpos - how? return in.read(b, off, len); @@ -337,10 +329,10 @@ public synchronized long skip(long n) throws IOException { if (avail <= 0) { // If no mark position set then don't keep in buffer - /* + /* if (markpos <0) return in.skip(n); - */ + */ // Fill in buffer to save bytes for reset fill(); @@ -441,7 +433,17 @@ public void close() throws IOException { sf = null; in = null; buf = null; - Objects.requireNonNull(this); //TODO: replace with Reference.reachabilityFence + /* + * This avoids that 'new SharedFileInputStream(file)' + * is GCed meanwhile #newStream is invoked, for example: + * new SharedFileInputStream(file).newStream(0, -1) + * + * That could be an issue if a subclass of this one invokes #close + * from #finalize (not a good practice anyway). + */ + Objects.requireNonNull(this); + // TODO Replace it by the next +// Reference.reachabilityFence(this); } } @@ -454,7 +456,7 @@ public void close() throws IOException { @Override public long getPosition() { //System.out.println("getPosition: start " + start + " pos " + pos -// + " bufpos " + bufpos + " = " + (bufpos + pos - start)); +// + " bufpos " + bufpos + " = " + (bufpos + pos - start)); if (in == null) throw new RuntimeException("Stream closed"); return bufpos + pos - start; @@ -481,7 +483,7 @@ public synchronized InputStream newStream(long start, long end) { throw new IllegalArgumentException("start < 0"); if (end == -1) end = datalen; - + return new SharedFileInputStream(sf, this.start + start, end - start, bufsize); } finally { @@ -492,27 +494,18 @@ public synchronized InputStream newStream(long start, long end) { // for testing... /* public static void main(String[] argv) throws Exception { - SharedFileInputStream is = new SharedFileInputStream(argv[0]); - java.util.Random r = new java.util.Random(); - int b; - while ((b = is.read()) >= 0) { - System.out.write(b); - if (r.nextDouble() < 0.3) { - InputStream is2 = is.newStream(is.getPosition(), -1); - int b2; - while ((b2 = is2.read()) >= 0) - ; - } - } + SharedFileInputStream is = new SharedFileInputStream(argv[0]); + java.util.Random r = new java.util.Random(); + int b; + while ((b = is.read()) >= 0) { + System.out.write(b); + if (r.nextDouble() < 0.3) { + InputStream is2 = is.newStream(is.getPosition(), -1); + int b2; + while ((b2 = is2.read()) >= 0) + ; + } + } } */ - - /** - * Force this stream to close. - */ - @Override - protected synchronized void finalize() throws Throwable { - super.finalize(); - close(); - } } diff --git a/api/src/test/java/jakarta/mail/FolderTest.java b/api/src/test/java/jakarta/mail/FolderTest.java new file mode 100644 index 00000000..007b6bd1 --- /dev/null +++ b/api/src/test/java/jakarta/mail/FolderTest.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2024 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; + +import java.util.Properties; + +import org.junit.Test; + +public class FolderTest { + + @Test + public void closeIdempotent() throws MessagingException { + CustomFolder folder = new CustomFolder(new DummyStore(Session.getDefaultInstance(new Properties()), null)); + folder.close(); + folder.close(); + } + + @Test + public void closeFalse() throws MessagingException { + try (Folder folder = new CustomFolder(new DummyStore(Session.getDefaultInstance(new Properties()), null))) { + folder.close(false); + } + } + + private static class CustomFolder extends Folder { + + private boolean closed; + + protected CustomFolder(Store store) { + super(store); + } + + @Override + public String getName() { + return null; + } + + @Override + public String getFullName() { + return null; + } + + @Override + public Folder getParent() throws MessagingException { + return null; + } + + @Override + public boolean exists() throws MessagingException { + return false; + } + + @Override + public Folder[] list(String pattern) throws MessagingException { + return null; + } + + @Override + public char getSeparator() throws MessagingException { + return 0; + } + + @Override + public int getType() throws MessagingException { + return 0; + } + + @Override + public boolean create(int type) throws MessagingException { + return false; + } + + @Override + public boolean hasNewMessages() throws MessagingException { + return false; + } + + @Override + public Folder getFolder(String name) throws MessagingException { + return null; + } + + @Override + public boolean delete(boolean recurse) throws MessagingException { + return false; + } + + @Override + public boolean renameTo(Folder f) throws MessagingException { + return false; + } + + @Override + public void open(int mode) throws MessagingException { + } + + @Override + public void close(boolean expunge) throws MessagingException { + if (closed) { + throw new IllegalStateException("Already closed"); + } + closed = true; + } + + @Override + public boolean isOpen() { + return !closed; + } + + @Override + public Flags getPermanentFlags() { + return null; + } + + @Override + public int getMessageCount() throws MessagingException { + return 0; + } + + @Override + public Message getMessage(int msgnum) throws MessagingException { + return null; + } + + @Override + public void appendMessages(Message[] msgs) throws MessagingException { + } + + @Override + public Message[] expunge() throws MessagingException { + return null; + } + } + + private static class DummyStore extends Store { + + protected DummyStore(Session session, URLName urlname) { + super(session, urlname); + } + + @Override + public Folder getDefaultFolder() throws MessagingException { + return null; + } + + @Override + public Folder getFolder(String name) throws MessagingException { + return null; + } + + @Override + public Folder getFolder(URLName url) throws MessagingException { + return null; + } + } +} diff --git a/api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java b/api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java index a1f707bb..d3aae154 100644 --- a/api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java +++ b/api/src/test/java/jakarta/mail/util/SharedFileInputStreamTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2024 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 @@ -18,10 +18,16 @@ import org.junit.Test; +import jakarta.mail.util.SharedFileInputStream.SharedFile; + import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.io.RandomAccessFile; +import java.lang.reflect.Field; + +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; /** @@ -62,4 +68,51 @@ public void testGrandChild() throws Exception { file.delete(); } } + + @Test + public void testCloseMultipleTimes() throws Exception { + File file = new File(SharedFileInputStreamTest.class.getResource("/jakarta/mail/util/sharedinputstream.txt").toURI()); + SharedFileInputStream in = new SharedFileInputStream(file); + in.close(); + in.close(); + } + + @Test + public void testOpenIfOneOpened() throws Exception { + File file = new File(SharedFileInputStreamTest.class.getResource("/jakarta/mail/util/sharedinputstream.txt").toURI()); + SharedFileInputStream in0 = null; + SharedFileInputStream in1 = null; + try (SharedFileInputStream in = new SharedFileInputStream(file)) { + in0 = (SharedFileInputStream) in.newStream(0, 6); + in1 = (SharedFileInputStream) in.newStream(6, 12); + } + RandomAccessFile ra0 = getRandomAccessFile(in0); + RandomAccessFile ra1 = getRandomAccessFile(in1); + // It is the same instance + assertEquals(ra0, ra1); + // RandomAccessFile still be open + in1.close(); + assertEquals(false, isClosed(ra1)); + in0.close(); + // All SharedFileInputStream are closed, so RandomAccessFile gets closed too + assertEquals(true, isClosed(ra1)); + } + + private RandomAccessFile getRandomAccessFile(SharedFileInputStream in) throws Exception { + Field f1 = SharedFileInputStream.class.getDeclaredField("sf"); + f1.setAccessible(true); + SharedFile rin = (SharedFile) f1.get(in); + RandomAccessFile rf = rin.in; + return rf; + } + + private boolean isClosed(RandomAccessFile rf) throws Exception { + try { + rf.readByte(); + return false; + } catch (IOException e) { + return true; + } + } + } \ No newline at end of file diff --git a/api/src/test/resources/jakarta/mail/util/sharedinputstream.txt b/api/src/test/resources/jakarta/mail/util/sharedinputstream.txt new file mode 100644 index 00000000..7f136131 --- /dev/null +++ b/api/src/test/resources/jakarta/mail/util/sharedinputstream.txt @@ -0,0 +1,10 @@ +test0 +test1 +test2 +test3 +test4 +test5 +test6 +test7 +test8 +test9 \ No newline at end of file diff --git a/copyright-exclude b/copyright-exclude index f71b6ee3..4d56431b 100644 --- a/copyright-exclude +++ b/copyright-exclude @@ -24,6 +24,7 @@ mail/src/test/resources/jakarta/mail/internet/paramdata mail/src/test/resources/jakarta/mail/internet/paramdatanostrict api/src/test/resources/jakarta/mail/internet/tokenlist api/src/test/resources/jakarta/mail/internet/addrlist +api/src/test/resources/jakarta/mail/util/sharedinputstream.txt mail/src/test/resources/jakarta/mail/internet/MailDateFormat_old.ser mail/src/test/resources/jakarta/mail/internet/MailDateFormat_new.ser mail/src/test/resources/com/sun/mail/test/keystore.jks