IMAP ConnectionPool and IdleManager produce StoreClosedException #193

doc/src/main/resources/docs/CHANGES.txt

@@ -7,6 +7,12 @@ for the Eclipse EE4J Angus Mail project:
 
     https://github.com/eclipse-ee4j/angus-mail/issues/<bug-number>
 
+          CHANGES IN THE 2.1.0 RELEASE
+          ----------------------------
+The following bugs have been fixed in the 2.1.0 release.
+
+193: IMAP ConnectionPool and IdleManager produce StoreClosedException
+
           CHANGES IN THE 2.0.5 RELEASE
           ----------------------------
 The following bugs have been fixed in the 2.0.5 release.

providers/angus-mail/src/test/java/org/eclipse/angus/mail/imap/IMAPStoreTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2026 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
@@ -20,6 +20,7 @@
 import jakarta.mail.MessagingException;
 import jakarta.mail.Session;
 import jakarta.mail.Store;
+import jakarta.mail.StoreClosedException;
 import org.eclipse.angus.mail.test.TestServer;
 import org.junit.Rule;
 import org.junit.Test;
@@ -28,6 +29,7 @@
 import java.io.IOException;
 import java.util.Properties;
 import java.util.StringTokenizer;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -374,6 +376,47 @@ public void namespace() throws IOException {
                 });
     }
 
+    /**
+     * Test that a dead store connection borrowed by a Folder isn't
+     * returned to the pool for reuse.
+     */
+    @Test
+    public void testDeadBorrowedStoreConnectionIsDiscarded() {
+        testWithHandler(
+                new IMAPTest() {
+                    @Override
+                    public void test(Store store, TestServer server)
+                            throws Exception {
+                        store.connect("test", "test");
+                        Folder inbox = store.getFolder("INBOX");
+
+                        assertTrue(inbox.exists());
+                        try {
+                            inbox.exists();
+                            fail("StoreClosedException expected");
+                        } catch (StoreClosedException expected) {
+                            // expected
+                        }
+                        assertTrue(inbox.exists());
+                        assertEquals(2, server.clientCount());
+                    }
+                },
+                new IMAPHandler() {
+                    private final AtomicInteger listCount = new AtomicInteger();
+
+                    @Override
+                    public void list(String line) throws IOException {
+                        int count = listCount.incrementAndGet();
+                        if (count == 2) {
+                            exit();
+                            return;
+                        }
+                        untagged("LIST (\\HasNoChildren) \"/\" \"INBOX\"");
+                        ok();
+                    }
+                });
+    }
+
     private void testWithHandler(IMAPTest test, IMAPHandler handler) {
         TestServer server = null;
         try {

providers/imap/src/main/java/org/eclipse/angus/mail/imap/IMAPStore.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2026 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
@@ -1044,6 +1044,13 @@ IMAPProtocol getProtocol(IMAPFolder folder)
                     p = pool.authenticatedConnections.lastElement();
                     pool.authenticatedConnections.removeElement(p);
 
+                    if (!p.isAuthenticated()) {
+                        logger.fine("discarding dead connection from pool");
+                        discardProtocol(p);
+                        p = null;
+                        continue;   // try again, from the top
+                    }
+
                     // check if the connection is still live
                     long lastUsed = System.currentTimeMillis() - p.getTimestamp();
                     if (lastUsed > pool.serverTimeoutInterval) {
@@ -1179,6 +1186,15 @@ private IMAPProtocol getStoreProtocol() throws ProtocolException {
                                 pool.authenticatedConnections.size());
                     p = pool.authenticatedConnections.firstElement();
 
+                    if (!p.isAuthenticated()) {
+                        pool.logger.fine(
+                                "getStoreProtocol() - discarding dead connection");
+                        pool.authenticatedConnections.removeElementAt(0);
+                        discardProtocol(p);
+                        p = null;
+                        continue;   // try again, from the top
+                    }
+
                     // if proxyAuthUser has changed, switch to new user
                     if (proxyAuthUser != null &&
                             !proxyAuthUser.equals(p.getProxyAuthUser()) &&
@@ -1282,6 +1298,29 @@ boolean getMessageCacheDebug() {
         return messageCacheDebug;
     }
 
+    /**
+     * Disconnect a protocol object that can no longer be reused.
+     */
+    private void discardProtocol(IMAPProtocol protocol) {
+        if (protocol == null)
+            return;
+        try {
+            protocol.removeResponseHandler(this);
+        } catch (RuntimeException ignored) {
+            // don't let cleanup failures hide the original problem
+        }
+        try {
+            protocol.removeResponseHandler(nonStoreResponseHandler);
+        } catch (RuntimeException ignored) {
+            // don't let cleanup failures hide the original problem
+        }
+        try {
+            protocol.disconnect();
+        } catch (RuntimeException ignored) {
+            // don't let cleanup failures hide the original problem
+        }
+    }
+
     /**
      * Report whether the connection pool is full.
      */
@@ -1307,7 +1346,10 @@ void releaseProtocol(IMAPFolder folder, IMAPProtocol protocol) {
             if (protocol != null) {
                 // If the pool is not full, add the store as a response handler
                 // and return the protocol object to the connection pool.
-                if (!isConnectionPoolFull()) {
+                if (!protocol.isAuthenticated()) {
+                    logger.fine("discarding dead authenticated connection");
+                    discardProtocol(protocol);
+                } else if (!isConnectionPoolFull()) {
                     protocol.addResponseHandler(this);
                     pool.authenticatedConnections.addElement(protocol);
 
@@ -1383,8 +1425,15 @@ void releaseFolderStoreProtocol(IMAPProtocol protocol) {
         if (protocol == null)
             return;        // should never happen
         protocol.removeResponseHandler(nonStoreResponseHandler);
-        protocol.addResponseHandler(this);
         synchronized (pool) {
+            if (!protocol.isAuthenticated()) {
+                pool.logger.fine(
+                        "releaseFolderStoreProtocol() discarding dead connection");
+                pool.authenticatedConnections.removeElement(protocol);
+                discardProtocol(protocol);
+            } else {
+                protocol.addResponseHandler(this);
+            }
             pool.storeConnectionInUse = false;
             pool.notifyAll();    // in case anyone waiting