Merge pull request #5380 from k9mail/close_imap_connections

Add a way to close all open IMAP connections
This commit is contained in:
cketti 2021-07-04 19:36:28 +02:00 committed by GitHub
commit d408a5670e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 113 additions and 28 deletions

View file

@ -7,6 +7,7 @@ import java.net.SocketException
internal interface ImapConnection {
val logId: String
val connectionGeneration: Int
val isConnected: Boolean
val outputStream: OutputStream
val isUidPlusCapable: Boolean

View file

@ -15,6 +15,8 @@ interface ImapStore {
@Throws(MessagingException::class)
fun getFolders(): List<FolderListItem>
fun closeAllConnections()
companion object {
fun create(
serverSettings: ServerSettings,

View file

@ -82,6 +82,7 @@ class RealImapConnection implements ImapConnection {
private final TrustedSocketFactory socketFactory;
private final int socketConnectTimeout;
private final int socketReadTimeout;
private final int connectionGeneration;
private Socket socket;
private PeekableInputStream inputStream;
@ -96,24 +97,27 @@ class RealImapConnection implements ImapConnection {
public RealImapConnection(ImapSettings settings, TrustedSocketFactory socketFactory,
ConnectivityManager connectivityManager, OAuth2TokenProvider oauthTokenProvider) {
ConnectivityManager connectivityManager, OAuth2TokenProvider oauthTokenProvider, int connectionGeneration) {
this.settings = settings;
this.socketFactory = socketFactory;
this.connectivityManager = connectivityManager;
this.oauthTokenProvider = oauthTokenProvider;
this.socketConnectTimeout = SOCKET_CONNECT_TIMEOUT;
this.socketReadTimeout = SOCKET_READ_TIMEOUT;
this.connectionGeneration = connectionGeneration;
}
RealImapConnection(ImapSettings settings, TrustedSocketFactory socketFactory,
public RealImapConnection(ImapSettings settings, TrustedSocketFactory socketFactory,
ConnectivityManager connectivityManager, OAuth2TokenProvider oauthTokenProvider,
int socketConnectTimeout, int socketReadTimeout) {
int socketConnectTimeout, int socketReadTimeout,
int connectionGeneration) {
this.settings = settings;
this.socketFactory = socketFactory;
this.connectivityManager = connectivityManager;
this.oauthTokenProvider = oauthTokenProvider;
this.socketConnectTimeout = socketConnectTimeout;
this.socketReadTimeout = socketReadTimeout;
this.connectionGeneration = connectionGeneration;
}
@Override
@ -910,4 +914,9 @@ class RealImapConnection implements ImapConnection {
public boolean isDataAvailable() throws IOException {
return inputStream.available() > 0;
}
@Override
public int getConnectionGeneration() {
return connectionGeneration;
}
}

View file

@ -53,6 +53,7 @@ class RealImapStore implements ImapStore, ImapConnectionManager, InternalImapSto
private String pathDelimiter = null;
private final Deque<ImapConnection> connections = new LinkedList<>();
private FolderNameCodec folderNameCodec;
private volatile int connectionGeneration = 1;
public RealImapStore(ServerSettings serverSettings, ImapStoreConfig config,
@ -296,18 +297,39 @@ class RealImapStore implements ImapStore, ImapConnectionManager, InternalImapSto
@Override
public void releaseConnection(ImapConnection connection) {
if (connection != null && connection.isConnected()) {
synchronized (connections) {
connections.offer(connection);
if (connection.getConnectionGeneration() == connectionGeneration) {
synchronized (connections) {
connections.offer(connection);
}
} else {
connection.close();
}
}
}
@Override
public void closeAllConnections() {
Timber.v("ImapStore.closeAllConnections()");
List<ImapConnection> connectionsToClose;
synchronized (connections) {
connectionGeneration++;
connectionsToClose = new ArrayList<>(connections);
connections.clear();
}
for (ImapConnection connection : connectionsToClose) {
connection.close();
}
}
ImapConnection createImapConnection() {
return new RealImapConnection(
new StoreImapSettings(),
trustedSocketFactory,
connectivityManager,
oauthTokenProvider);
oauthTokenProvider,
connectionGeneration);
}
@Override

View file

@ -975,7 +975,7 @@ public class RealImapConnectionTest {
private ImapConnection createImapConnection(ImapSettings settings, TrustedSocketFactory socketFactory,
ConnectivityManager connectivityManager, OAuth2TokenProvider oAuth2TokenProvider) {
return new RealImapConnection(settings, socketFactory, connectivityManager, oAuth2TokenProvider,
SOCKET_CONNECT_TIMEOUT, SOCKET_READ_TIMEOUT);
SOCKET_CONNECT_TIMEOUT, SOCKET_READ_TIMEOUT, 1);
}
private ImapConnection startServerAndCreateImapConnection(MockImapServer server) throws IOException {

View file

@ -56,7 +56,7 @@ public class RealImapStoreTest {
@Test
public void checkSettings_shouldCreateImapConnectionAndCallOpen() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
imapStore.enqueueImapConnection(imapConnection);
imapStore.checkSettings();
@ -66,7 +66,7 @@ public class RealImapStoreTest {
@Test
public void checkSettings_withOpenThrowing_shouldThrowMessagingException() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
doThrow(IOException.class).when(imapConnection).open();
imapStore.enqueueImapConnection(imapConnection);
@ -82,7 +82,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withSpecialUseCapability_shouldReturnSpecialFolderInfo() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(true);
List<ImapResponse> imapResponses = Arrays.asList(
@ -113,7 +113,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withoutSpecialUseCapability_shouldUseSimpleListCommand() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(false);
imapStore.enqueueImapConnection(imapConnection);
@ -126,7 +126,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withoutListExtendedCapability_shouldUseSimpleListCommand() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(false);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(true);
imapStore.enqueueImapConnection(imapConnection);
@ -140,7 +140,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withoutSubscribedFoldersOnly() throws Exception {
when(config.isSubscribedFoldersOnly()).thenReturn(false);
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
List<ImapResponse> imapResponses = Arrays.asList(
createImapResponse("* LIST (\\HasNoChildren) \".\" \"INBOX\""),
createImapResponse("* LIST (\\Noselect \\HasChildren) \".\" \"Folder\""),
@ -160,7 +160,7 @@ public class RealImapStoreTest {
public void getFolders_withSubscribedFoldersOnly_shouldOnlyReturnExistingSubscribedFolders()
throws Exception {
when(config.isSubscribedFoldersOnly()).thenReturn(true);
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
List<ImapResponse> lsubResponses = Arrays.asList(
createImapResponse("* LSUB (\\HasNoChildren) \".\" \"INBOX\""),
createImapResponse("* LSUB (\\Noselect \\HasChildren) \".\" \"Folder\""),
@ -186,7 +186,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withNamespacePrefix() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
List<ImapResponse> imapResponses = Arrays.asList(
createImapResponse("* LIST () \".\" \"INBOX\""),
createImapResponse("* LIST () \".\" \"INBOX.FolderOne\""),
@ -207,7 +207,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withFolderNotMatchingNamespacePrefix() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
List<ImapResponse> imapResponses = Arrays.asList(
createImapResponse("* LIST () \".\" \"INBOX\""),
createImapResponse("* LIST () \".\" \"INBOX.FolderOne\""),
@ -229,7 +229,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withDuplicateFolderNames_shouldRemoveDuplicatesAndKeepFolderType()
throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(true);
List<ImapResponse> imapResponses = Arrays.asList(
@ -253,7 +253,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withoutException_shouldLeaveImapConnectionOpen() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
List<ImapResponse> imapResponses = Collections.singletonList(createImapResponse("5 OK Success"));
when(imapConnection.executeSimpleCommand(anyString())).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
@ -265,7 +265,7 @@ public class RealImapStoreTest {
@Test
public void getFolders_withIoException_shouldCloseImapConnection() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
doThrow(IOException.class).when(imapConnection).executeSimpleCommand("LIST \"\" \"*\"");
imapStore.enqueueImapConnection(imapConnection);
@ -280,7 +280,7 @@ public class RealImapStoreTest {
@Test
public void getConnection_shouldCreateImapConnection() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
imapStore.enqueueImapConnection(imapConnection);
ImapConnection result = imapStore.getConnection();
@ -290,8 +290,8 @@ public class RealImapStoreTest {
@Test
public void getConnection_calledTwiceWithoutRelease_shouldCreateTwoImapConnection() throws Exception {
ImapConnection imapConnectionOne = mock(ImapConnection.class);
ImapConnection imapConnectionTwo = mock(ImapConnection.class);
ImapConnection imapConnectionOne = createMockConnection();
ImapConnection imapConnectionTwo = createMockConnection();
imapStore.enqueueImapConnection(imapConnectionOne);
imapStore.enqueueImapConnection(imapConnectionTwo);
@ -304,7 +304,7 @@ public class RealImapStoreTest {
@Test
public void getConnection_calledAfterRelease_shouldReturnCachedImapConnection() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
ImapConnection imapConnection = createMockConnection();
when(imapConnection.isConnected()).thenReturn(true);
imapStore.enqueueImapConnection(imapConnection);
ImapConnection connection = imapStore.getConnection();
@ -318,8 +318,8 @@ public class RealImapStoreTest {
@Test
public void getConnection_calledAfterReleaseWithAClosedConnection_shouldReturnNewImapConnectionInstance()
throws Exception {
ImapConnection imapConnectionOne = mock(ImapConnection.class);
ImapConnection imapConnectionTwo = mock(ImapConnection.class);
ImapConnection imapConnectionOne = createMockConnection();
ImapConnection imapConnectionTwo = createMockConnection();
imapStore.enqueueImapConnection(imapConnectionOne);
imapStore.enqueueImapConnection(imapConnectionTwo);
imapStore.getConnection();
@ -333,8 +333,8 @@ public class RealImapStoreTest {
@Test
public void getConnection_withDeadConnectionInPool_shouldReturnNewImapConnectionInstance() throws Exception {
ImapConnection imapConnectionOne = mock(ImapConnection.class);
ImapConnection imapConnectionTwo = mock(ImapConnection.class);
ImapConnection imapConnectionOne = createMockConnection();
ImapConnection imapConnectionTwo = createMockConnection();
imapStore.enqueueImapConnection(imapConnectionOne);
imapStore.enqueueImapConnection(imapConnectionTwo);
imapStore.getConnection();
@ -347,6 +347,53 @@ public class RealImapStoreTest {
assertSame(imapConnectionTwo, result);
}
@Test
public void getConnection_withConnectionInPoolAndCloseAllConnections_shouldReturnNewImapConnectionInstance()
throws Exception {
ImapConnection imapConnectionOne = createMockConnection(1);
ImapConnection imapConnectionTwo = createMockConnection(2);
imapStore.enqueueImapConnection(imapConnectionOne);
imapStore.enqueueImapConnection(imapConnectionTwo);
imapStore.getConnection();
when(imapConnectionOne.isConnected()).thenReturn(true);
imapStore.releaseConnection(imapConnectionOne);
imapStore.closeAllConnections();
ImapConnection result = imapStore.getConnection();
assertSame(imapConnectionTwo, result);
}
@Test
public void getConnection_withConnectionOutsideOfPoolAndCloseAllConnections_shouldReturnNewImapConnectionInstance()
throws Exception {
ImapConnection imapConnectionOne = createMockConnection(1);
ImapConnection imapConnectionTwo = createMockConnection(2);
imapStore.enqueueImapConnection(imapConnectionOne);
imapStore.enqueueImapConnection(imapConnectionTwo);
imapStore.getConnection();
when(imapConnectionOne.isConnected()).thenReturn(true);
imapStore.closeAllConnections();
imapStore.releaseConnection(imapConnectionOne);
ImapConnection result = imapStore.getConnection();
assertSame(imapConnectionTwo, result);
}
private ImapConnection createMockConnection() {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.getConnectionGeneration()).thenReturn(1);
return imapConnection;
}
private ImapConnection createMockConnection(int connectionGeneration) {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.getConnectionGeneration()).thenReturn(connectionGeneration);
return imapConnection;
}
private ServerSettings createServerSettings() {
Map<String, String> extra = ImapStoreSettings.createExtra(true, null);

View file

@ -6,7 +6,7 @@ import java.util.concurrent.TimeUnit
import java.util.concurrent.locks.ReentrantLock
import kotlin.concurrent.withLock
internal open class TestImapConnection(val timeout: Long) : ImapConnection {
internal open class TestImapConnection(val timeout: Long, override val connectionGeneration: Int = 1) : ImapConnection {
override val logId: String = "testConnection"
override var isConnected: Boolean = false
protected set

View file

@ -17,4 +17,8 @@ internal class TestImapStore(private val folder: ImapFolder) : ImapStore, ImapCo
if (folder !is TestImapFolder) throw AssertionError("getConnection() called with unknown ImapFolder instance")
return folder.connection
}
override fun closeAllConnections() {
throw UnsupportedOperationException("not implemented")
}
}