Store finalizers should not talk to server - bug 7472

Author: bshannon

doc/release/CHANGES.txt

@@ -31,6 +31,7 @@ K  7356	NPE in Tomcat ClassLoader causes Session.getInstance to fail
 K  7378	Deadlock in IMAPFolder.doProtocolCommand()
 K  7471	InternetAddress.getLocalAddress should use
 	InetAddress.getCanonicalHostName
+K  7472	Store finalizers should not talk to server
 
 
 		  CHANGES IN THE 1.5.5 RELEASE

doc/release/COMPAT.txt

@@ -16,6 +16,20 @@ with this release of the JavaMail API.
 
 -- JavaMail 1.5.6 --
 
+- finalizers close sockets abruptly
+
+	It's important for finalizers to close an open socket
+	connection to prevent file descriptor leaks.  Previously the
+	finalizers for the IMAP and POP3 providers would try to close
+	the connection cleanly, which could result in a timeout waiting
+	for the server.  They now close the connection without
+	performing any socket I/O, which may result in an unclean
+	shutdown when seen from the server.  Applications should always
+	close Stores and Folders when done with them to avoid the need
+	for the finalizer to do this cleanup.  The Session property
+	"mail.<protocol>.finalizecleanclose" can be set to "true" to
+	force the connection to be closed cleanly in the finalizer.
+
 - InternetAddress.getLocalAddress uses canonical host name
 
 	The InternetAddress.getLocalAddress method now uses the

mail/src/main/java/com/sun/mail/imap/IMAPStore.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2015 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2016 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -225,6 +225,7 @@ public class IMAPStore extends Store
     private boolean peek = false;
     private boolean closeFoldersOnStoreFailure = true;
     private boolean enableCompress = false;	// enable COMPRESS=DEFLATE
+    private boolean finalizeCleanClose = false;
 
     /*
      * This field is set in the Store's response handler if we see
@@ -598,6 +599,12 @@ protected IMAPStore(Session session, URLName url,
 	if (enableCompress)
 	    logger.config("enable COMPRESS");
 
+	// check if finalizeCleanClose is enabled
+	finalizeCleanClose = PropUtil.getBooleanSessionProperty(session,
+	    "mail." + name + ".finalizecleanclose", false);
+	if (finalizeCleanClose)
+	    logger.config("close connection cleanly in finalize");
+
 	s = session.getProperty("mail." + name + ".folder.class");
 	if (s != null) {
 	    logger.log(Level.CONFIG, "IMAP: folder class: {0}", s);
@@ -1662,6 +1669,14 @@ public synchronized void close() throws MessagingException {
     }
 
     protected void finalize() throws Throwable {
+	if (!finalizeCleanClose) {
+	    // when finalizing, close connections abruptly
+	    synchronized (connectionFailedLock) {
+		connectionFailed = true;
+		forceClose = true;
+	    }
+	    closeFoldersOnStoreFailure = true;	// make sure folders get closed
+	}
 	try {
 	    close();
 	} finally {

mail/src/main/java/com/sun/mail/imap/package.html

@@ -5,7 +5,7 @@
 
     DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
 
-    Copyright (c) 1997-2015 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 1997-2016 Oracle and/or its affiliates. All rights reserved.
 
     The contents of this file are subject to the terms of either the GNU
     General Public License Version 2 only ("GPL") or the Common Development
@@ -867,6 +867,20 @@ <H4>Properties</H4>
 </TD>
 </TR>
 
+<A NAME="mail.imap.finalizecleanclose"></A>
+<TR id="mail.imap.finalizecleanclose">
+<TD>mail.imap.finalizecleanclose</TD>
+<TD>boolean</TD>
+<TD>
+When the finalizer for IMAPStore is called,
+should the connection to the server be closed cleanly, as if the
+application called the close method?
+Or should the connection to the server be closed without sending
+any commands to the server?
+Defaults to false, the connection is not closed cleanly.
+</TD>
+</TR>
+
 <A NAME="mail.imap.referralexception"></A>
 <TR id="mail.imap.referralexception">
 <TD>mail.imap.referralexception</TD>

mail/src/main/java/com/sun/mail/pop3/POP3Folder.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2015 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2016 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -75,6 +75,7 @@ public class POP3Folder extends Folder {
     private POP3Message[] message_cache;
     private boolean doneUidl = false;
     private volatile TempFile fileCache = null;
+    private boolean forceClose;
 
     MailLogger logger;	// package private, for POP3Message
 
@@ -250,10 +251,10 @@ public synchronized void close(boolean expunge) throws MessagingException {
 	     * the "marked for deletion" flags.  We can then explicitly
 	     * delete messages as desired.
 	     */
-	    if (store.rsetBeforeQuit)
+	    if (store.rsetBeforeQuit && !forceClose)
 		port.rset();
 	    POP3Message m;
-	    if (expunge && mode == READ_WRITE) {
+	    if (expunge && mode == READ_WRITE && !forceClose) {
 		// find all messages marked deleted and issue DELE commands
 		for (int i = 0; i < message_cache.length; i++) {
 		    if ((m = message_cache[i]) != null) {
@@ -277,7 +278,10 @@ public synchronized void close(boolean expunge) throws MessagingException {
 		    m.invalidate(true);
 	    }
 
-	    port.quit();
+	    if (forceClose)
+		port.close();
+	    else
+		port.quit();
 	} catch (IOException ex) {
 	    // do nothing
 	} finally {
@@ -545,11 +549,13 @@ public synchronized InputStream listCommand()
      * Close the folder when we're finalized.
      */
     protected void finalize() throws Throwable {
+	forceClose = !store.finalizeCleanClose;
 	try {
 	    if (opened)
 		close(false);
 	} finally {
 	    super.finalize();
+	    forceClose = false;
 	}
     }
 

mail/src/main/java/com/sun/mail/pop3/POP3Store.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2015 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2016 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -95,6 +95,7 @@ public class POP3Store extends Store {
     volatile boolean useFileCache = false;
     volatile File fileCacheDir = null;
     volatile boolean keepMessageContent = false;
+    volatile boolean finalizeCleanClose = false;
 
     public POP3Store(Session session, URLName url) {
 	this(session, url, "pop3", false);
@@ -136,6 +137,9 @@ public POP3Store(Session session, URLName url,
 	// mail.pop3.starttls.required requires use of STLS command
 	requireStartTLS = getBoolProp("starttls.required");
 
+	// mail.pop3.finalizecleanclose requires clean close when finalizing
+	finalizeCleanClose = getBoolProp("finalizecleanclose");
+
 	String s = session.getProperty("mail." + name + ".message.class");
 	if (s != null) {
 	    logger.log(Level.CONFIG, "message class: {0}", s);
@@ -341,9 +345,17 @@ synchronized void closePort(POP3Folder owner) {
     }
 
     public synchronized void close() throws MessagingException {
+	close(false);
+    }
+
+    synchronized void close(boolean force) throws MessagingException {
 	try {
-	    if (port != null)
-		port.quit();
+	    if (port != null) {
+		if (force)
+		    port.close();
+		else
+		    port.quit();
+	    }
 	} catch (IOException ioex) {
 	} finally {
 	    port = null;
@@ -410,7 +422,7 @@ public synchronized boolean isSSL() {
     protected void finalize() throws Throwable {
 	try {
 	    if (port != null)	// don't force a connection attempt
-		close();
+		close(!finalizeCleanClose);
 	} finally {
 	    super.finalize();
 	}

mail/src/main/java/com/sun/mail/pop3/Protocol.java

@@ -346,17 +346,26 @@ synchronized boolean quit() throws IOException {
 	    Response r = simpleCommand("QUIT");
 	    ok = r.ok;
 	} finally {
-	    try {
-		socket.close();
-	    } finally {
-		socket = null;
-		input = null;
-		output = null;
-	    }
+	    close();
 	}
 	return ok;
     }
 
+    /**
+     * Close the connection without sending any commands.
+     */
+    void close() {
+	try {
+	    socket.close();
+	} catch (IOException ex) {
+	    // ignore it
+	} finally {
+	    socket = null;
+	    input = null;
+	    output = null;
+	}
+    }
+
     /**
      * Return the total number of messages and mailbox size,
      * using the STAT command.

mail/src/main/java/com/sun/mail/pop3/package.html

@@ -5,7 +5,7 @@
 
     DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
 
-    Copyright (c) 1997-2015 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 1997-2016 Oracle and/or its affiliates. All rights reserved.
 
     The contents of this file are subject to the terms of either the GNU
     General Public License Version 2 only ("GPL") or the Common Development
@@ -606,6 +606,20 @@ <H4>Properties</H4>
 </TD>
 </TR>
 
+<A NAME="mail.pop3.finalizecleanclose"></A>
+<TR id="mail.pop3.finalizecleanclose">
+<TD>mail.pop3.finalizecleanclose</TD>
+<TD>boolean</TD>
+<TD>
+When the finalizer for POP3Store or POP3Folder is called,
+should the connection to the server be closed cleanly, as if the
+application called the close method?
+Or should the connection to the server be closed without sending
+any commands to the server?
+Defaults to false, the connection is not closed cleanly.
+</TD>
+</TR>
+
 </TABLE>
 <P>
 In general, applications should not need to use the classes in this