From 57c30b8d147847cdddd1e8657e965abdd8a41c88 Mon Sep 17 00:00:00 2001 From: jbescos Date: Thu, 26 Mar 2026 12:31:23 +0100 Subject: [PATCH] IMAP ConnectionPool and IdleManager produce StoreClosedException #193 Signed-off-by: jbescos --- doc/src/main/resources/docs/CHANGES.txt | 6 ++ .../angus/mail/imap/IMAPStoreTest.java | 45 ++++++++++++++- .../eclipse/angus/mail/imap/IMAPStore.java | 55 ++++++++++++++++++- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/doc/src/main/resources/docs/CHANGES.txt b/doc/src/main/resources/docs/CHANGES.txt index d54cb254..613fa099 100644 --- a/doc/src/main/resources/docs/CHANGES.txt +++ b/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/ + 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. diff --git a/providers/angus-mail/src/test/java/org/eclipse/angus/mail/imap/IMAPStoreTest.java b/providers/angus-mail/src/test/java/org/eclipse/angus/mail/imap/IMAPStoreTest.java index 256fc496..c117e063 100644 --- a/providers/angus-mail/src/test/java/org/eclipse/angus/mail/imap/IMAPStoreTest.java +++ b/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 { diff --git a/providers/imap/src/main/java/org/eclipse/angus/mail/imap/IMAPStore.java b/providers/imap/src/main/java/org/eclipse/angus/mail/imap/IMAPStore.java index 96aa97c3..1ba140ba 100644 --- a/providers/imap/src/main/java/org/eclipse/angus/mail/imap/IMAPStore.java +++ b/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