COR-201 - Lease a pooled IMAP connection for exclusive use

Package.swift

@@ -265,7 +265,7 @@ var targets: [Target] = [
             "unittest.cpp",
             "unittest.mm"
         ],
-        sources: ["CertificateUtilsTests.swift", "IMAPIdleCancellationTests.swift", "IMAPInterruptCurrentCommandTests.swift", "LibetpanHelperTests.swift", "unittest.swift"],
+        sources: ["CertificateUtilsTests.swift", "IMAPConnectionLeaseTests.swift", "IMAPIdleCancellationTests.swift", "IMAPInterruptCurrentCommandTests.swift", "LeaseTestTCPEndpoint.swift", "LibetpanHelperTests.swift", "unittest.swift"],
         resources: [
             .copy("data")
         ]

build-windows-5.10/build_headers.list

@@ -83,6 +83,7 @@ src\async\smtp\MCSMTPAsyncSession.h
 src\async\smtp\MCSMTPOperation.h
 src\async\smtp\MCSMTPOperationCallback.h
 src\async\imap\MCAsyncIMAP.h
+src\async\imap\MCIMAPAsyncConnection.h
 src\async\imap\MCIMAPAsyncSession.h
 src\async\imap\MCIMAPOperation.h
 src\async\imap\MCIMAPFetchFoldersOperation.h

configure-headers.sh

@@ -220,6 +220,7 @@ cp c/smtp/CSMTPSession.h ./include/MailCore
 
 cp c/imap/CIMAPAppendMessageOperation.h ./include/MailCore
 cp c/imap/CIMAPAsyncSession.h ./include/MailCore
+cp c/imap/CIMAPAsyncConnection.h ./include/MailCore
 cp c/imap/CIMAPBaseOperation.h ./include/MailCore
 cp c/imap/CIMAPCapabilityOperation.h ./include/MailCore
 cp c/imap/CIMAPCheckAccountOperation.h ./include/MailCore

src/async/imap/MCIMAPAsyncConnection.cpp

@@ -108,10 +108,14 @@ IMAPAsyncConnection::IMAPAsyncConnection()
     mOwner = NULL;
     mConnectionLogger = NULL;
     MCB_LOCK_INIT(&mConnectionLoggerLock);
+    MCB_LOCK_INIT(&mReservationLock);
     mInternalLogger = new IMAPConnectionLogger(this);
     mAutomaticConfigurationEnabled = true;
     mQueueRunning = false;
     mScheduledAutomaticDisconnect = false;
+    mReserved = false;
+    mLeaseGeneration = 0;
+    mAutomaticDisconnectDelay = 30;
 }
 
 IMAPAsyncConnection::~IMAPAsyncConnection()
@@ -122,6 +126,7 @@ IMAPAsyncConnection::~IMAPAsyncConnection()
     cancelDelayedPerformMethod((Object::Method) &IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay, NULL);
 #endif
     MCB_LOCK_DESTROY(&mConnectionLoggerLock);
+    MCB_LOCK_DESTROY(&mReservationLock);
     MC_SAFE_RELEASE(mInternalLogger);
     MC_SAFE_RELEASE(mQueueCallback);
     MC_SAFE_RELEASE(mLastFolder);
@@ -281,6 +286,11 @@ IMAPSession * IMAPAsyncConnection::session()
     return mSession;
 }
 
+double IMAPAsyncConnection::lastLoginTime()
+{
+    return mSession->lastLoginTime();
+}
+
 unsigned int IMAPAsyncConnection::operationsCount()
 {
     return mQueue->count();
@@ -291,6 +301,54 @@ void IMAPAsyncConnection::cancelAllOperations()
     mQueue->cancelAllOperations();
 }
 
+unsigned int IMAPAsyncConnection::leaseGeneration()
+{
+    MCB_LOCK(&mReservationLock);
+    unsigned int generation = mLeaseGeneration;
+    MCB_UNLOCK(&mReservationLock);
+    return generation;
+}
+
+bool IMAPAsyncConnection::reserve()
+{
+    MCB_LOCK(&mReservationLock);
+    bool reserved = !mReserved;
+    if (reserved) {
+        mReserved = true;
+        mLeaseGeneration ++;
+    }
+    MCB_UNLOCK(&mReservationLock);
+    return reserved;
+}
+
+bool IMAPAsyncConnection::endLease(unsigned int leaseGeneration, bool disconnect)
+{
+    IMAPOperation * op = disconnect ? disconnectOperation() : NULL;
+    MCB_LOCK(&mReservationLock);
+    bool ended = mReserved && mLeaseGeneration == leaseGeneration;
+    if (ended) {
+        if (op != NULL) {
+            op->start();
+        }
+        mReserved = false;
+    }
+    MCB_UNLOCK(&mReservationLock);
+    return ended;
+}
+
+bool IMAPAsyncConnection::isReserved()
+{
+    MCB_LOCK(&mReservationLock);
+    bool reserved = mReserved;
+    MCB_UNLOCK(&mReservationLock);
+    return reserved;
+}
+
+void IMAPAsyncConnection::setAutomaticDisconnectDelay(time_t delay)
+{
+    mAutomaticDisconnectDelay = delay;
+}
+
 bool IMAPAsyncConnection::interruptCurrentCommand(IMAPOperation * operation)
 {
     // Only for the operation the queue is executing right now - its command is the one holding this
@@ -336,22 +394,60 @@ void IMAPAsyncConnection::tryAutomaticDisconnect()
     mOwner->retain();
     mScheduledAutomaticDisconnect = true;
 #if MC_HAS_GCD
-    performMethodOnDispatchQueueAfterDelay((Object::Method) &IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay, NULL, dispatchQueue(), 30);
+    performMethodOnDispatchQueueAfterDelay((Object::Method) &IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay, NULL, dispatchQueue(), (double) mAutomaticDisconnectDelay);
 #else
-    performMethodAfterDelay((Object::Method) &IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay, NULL, 30);
+    performMethodAfterDelay((Object::Method) &IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay, NULL, (double) mAutomaticDisconnectDelay);
 #endif
 
     if (scheduledAutomaticDisconnect) {
         mOwner->release();
     }
 }
 
+void IMAPAsyncConnection::scheduleAutomaticDisconnect()
+{
+    // Both kept alive until the hop lands: the block holds raw pointers, and a session dropped
+    // by its last user right after a release would otherwise take this connection with it
+    // before the block runs. Same pairing as the timer's own retain of the owner.
+    mOwner->retain();
+    retain();
+#if MC_HAS_GCD
+    performMethodOnDispatchQueue((Object::Method) &IMAPAsyncConnection::scheduleAutomaticDisconnectOnQueue, NULL, dispatchQueue());
+#else
+    performMethodOnMainThread((Object::Method) &IMAPAsyncConnection::scheduleAutomaticDisconnectOnQueue, NULL);
+#endif
+}
+
+void IMAPAsyncConnection::scheduleAutomaticDisconnectOnQueue(void * context)
+{
+    IMAPAsyncSession * owner = mOwner;
+    tryAutomaticDisconnect();
+    release();
+    owner->release();
+}
+
 void IMAPAsyncConnection::tryAutomaticDisconnectAfterDelay(void * context)
 {
     mScheduledAutomaticDisconnect = false;
 
     IMAPOperation * op = disconnectOperation();
+    // Checked and enqueued under the reservation lock, so an acquire on another thread lands
+    // either before the check - and the timer stands down - or after the enqueue - and the
+    // lease inherits a disconnect already queued ahead of its first command, which costs it a
+    // login and nothing else. Without the lock the disconnect could be enqueued after the
+    // reservation was published, and close the socket under a holder mid-operation.
+    MCB_LOCK(&mReservationLock);
+    if (mReserved) {
+        // A lease holder is between commands: leave its connection alone and let the timer
+        // die. Re-arming here instead would keep an owner retain and a periodic wakeup alive
+        // for as long as the lease is held - forever, if the lease leaks. releaseConnection
+        // arms the timer anew when the connection returns to the pool.
+        MCB_UNLOCK(&mReservationLock);
+        mOwner->release();
+        return;
+    }
     op->start();
+    MCB_UNLOCK(&mReservationLock);
 
     mOwner->release();
 }

src/async/imap/MCIMAPAsyncConnection.h

@@ -93,6 +93,9 @@ namespace mailcore {
 
         virtual IMAPOperation * disconnectOperation();
 
+        // How long an idle connection stays open once its queue drains.
+        virtual void setAutomaticDisconnectDelay(time_t delay);
+
     private:
         IMAPSession * mSession;
         OperationQueue * mQueue;
@@ -107,21 +110,54 @@ namespace mailcore {
         bool mAutomaticConfigurationEnabled;
         bool mQueueRunning;
         bool mScheduledAutomaticDisconnect;
+        // Guarded: the session writes these while acquiring or releasing, and reads them back
+        // from sessionWithMinQueue, which runs on whatever thread called IMAPOperation::start.
+        MCB_LOCK_TYPE mReservationLock;
+        bool mReserved;
+        unsigned int mLeaseGeneration;
+        time_t mAutomaticDisconnectDelay;
 
         virtual void tryAutomaticDisconnectAfterDelay(void * context);
+        virtual void scheduleAutomaticDisconnectOnQueue(void * context);
 
     public: // private
         virtual void runOperation(IMAPOperation * operation);
         virtual IMAPSession * session();
 
         virtual void cancelAllOperations();
         virtual bool interruptCurrentCommand(IMAPOperation * operation);
+
+        // Wall-clock moment of this connection's last successful LOGIN (see
+        // IMAPSession::lastLoginTime), 0 when it has never logged in.
+        virtual double lastLoginTime();
         virtual unsigned int operationsCount();
+
+        // A reserved connection belongs to one lease holder: the session selection skips it,
+        // so only operations explicitly pointed at it (IMAPOperation::setSession) run there.
+        // reserve() fails on a connection already reserved; endLease() fails unless the
+        // connection is reserved under exactly that lease generation. Each is one step under the
+        // reservation lock, so a release racing an acquire from another thread cannot slip a
+        // check past a set - and endLease enqueues its disconnect before unreserving, so no
+        // newcomer lands ahead of it.
+        virtual bool reserve();
+        virtual bool endLease(unsigned int leaseGeneration, bool disconnect);
+        virtual bool isReserved();
+
+        // Counts the reservations this connection has had. Reserving bumps it, so a holder that
+        // remembers the value it saw can tell its own lease from the next one on the same
+        // connection — which is what stops a late release from cancelling somebody else's lease
+        // (see IMAPAsyncSession::releaseConnection).
+        virtual unsigned int leaseGeneration();
 
         virtual void setLastFolder(String * folder);
         virtual String * lastFolder();
 
         virtual void tryAutomaticDisconnect();
+        // tryAutomaticDisconnect for a caller on a foreign thread - a lease release. The idle
+        // timer's bookkeeping (the scheduled flag, the owner retain it holds) is touched only
+        // from the connection's dispatch queue, where the timer fires and the drain re-arms it;
+        // this hops there instead of joining in from outside.
+        virtual void scheduleAutomaticDisconnect();
         virtual void queueStartRunning();
         virtual void queueStoppedRunning();
 

src/async/imap/MCIMAPAsyncSession.cpp

@@ -55,6 +55,7 @@ IMAPAsyncSession::IMAPAsyncSession()
     mSessions = new Array();
     mMaximumConnections = DEFAULT_MAX_CONNECTIONS;
     mAllowsFolderConcurrentAccessEnabled = true;
+    mAutomaticDisconnectDelay = 30;
 
     mHostname = NULL;
     mPort = 0;
@@ -239,6 +240,16 @@ unsigned int IMAPAsyncSession::maximumConnections()
     return mMaximumConnections;
 }
 
+void IMAPAsyncSession::setAutomaticDisconnectDelay(time_t delay)
+{
+    mAutomaticDisconnectDelay = delay;
+}
+
+time_t IMAPAsyncSession::automaticDisconnectDelay()
+{
+    return mAutomaticDisconnectDelay;
+}
+
 IMAPIdentity * IMAPAsyncSession::serverIdentity()
 {
     return mServerIdentity;
@@ -279,6 +290,7 @@ IMAPAsyncConnection * IMAPAsyncSession::session()
     session->setAuthType(mAuthType);
     session->setConnectionType(mConnectionType);
     session->setTimeout(mTimeout);
+    session->setAutomaticDisconnectDelay(mAutomaticDisconnectDelay);
     session->setCheckCertificateEnabled(mCheckCertificateEnabled);
     session->setVoIPEnabled(mVoIPEnabled);
     session->setDefaultNamespace(mDefaultNamespace);
@@ -316,14 +328,21 @@ IMAPAsyncConnection * IMAPAsyncSession::sessionForFolder(String * folder, bool u
             // empty queue or create new one, if maximum connections limit does not reached.
             s = availableSession();
             if (s->operationsCount() == 0) {
-                s->setLastFolder(folder);
+                if (!s->isReserved()) {
+                    s->setLastFolder(folder);
+                }
                 return s;
             }
         }
 
         // otherwise returns session with minimum size of queue among selected to the folder.
+        // A reserved result (the all-reserved fallback) keeps its affinity hint: it belongs to
+        // the lease holder, and an acquireConnection call that lands here runs no operation at
+        // all - stamping the hint would desync it from the actually selected mailbox.
         s = matchingSessionForFolder(folder);
-        s->setLastFolder(folder);
+        if (!s->isReserved()) {
+            s->setLastFolder(folder);
+        }
         return s;
     }
 }
@@ -343,6 +362,13 @@ IMAPAsyncConnection * IMAPAsyncSession::availableSession()
         return chosenSession;
     }
 
+    if (chosenSession == NULL) {
+        // every connection is reserved and the pool is at its limit: share the least busy
+        // reserved one. The lease it belongs to loses exclusivity, but callers dereference
+        // the result, so NULL here would be a crash rather than backpressure.
+        chosenSession = sessionWithMinQueue(false, NULL, true);
+    }
+
     // otherwise returns existant session with minimum size of queue.
     return chosenSession;
 }
@@ -377,15 +403,20 @@ IMAPAsyncConnection * IMAPAsyncSession::matchingSessionForFolder(String * folder
 }
 
 IMAPAsyncConnection * IMAPAsyncSession::sessionWithMinQueue(bool filterByFolder, String * folder)
+{
+    return sessionWithMinQueue(filterByFolder, folder, false);
+}
+
+IMAPAsyncConnection * IMAPAsyncSession::sessionWithMinQueue(bool filterByFolder, String * folder, bool includeReserved)
 {
     IMAPAsyncConnection * chosenSession = NULL;
     unsigned int minOperationsCount = 0;
 
     for (unsigned int i = 0 ; i < mSessions->count() ; i ++) {
         IMAPAsyncConnection * s = (IMAPAsyncConnection *) mSessions->objectAtIndex(i);
         if ((chosenSession == NULL) || (s->operationsCount() < minOperationsCount)) {
-            bool matched = true;
-            if (filterByFolder) {
+            bool matched = includeReserved || !s->isReserved();
+            if (matched && filterByFolder) {
                 // filter by last selested folder
                 matched = ((folder != NULL && s->lastFolder() != NULL && s->lastFolder()->isEqual(folder))
                            || (folder == NULL && s->lastFolder() == NULL));
@@ -400,6 +431,37 @@ IMAPAsyncConnection * IMAPAsyncSession::sessionWithMinQueue(bool filterByFolder,
     return chosenSession;
 }
 
+IMAPAsyncConnection * IMAPAsyncSession::acquireConnection(String * folder)
+{
+    // A lease wants the shortest possible foreign backlog ahead of it, so an idle or new
+    // connection is preferred over the busiest matching one: urgent mode when folder affinity
+    // is worth trying first, the plain available-session pick when there is no folder to match
+    // (sessionForFolder ignores urgent for a NULL folder).
+    IMAPAsyncConnection * connection = (folder != NULL) ? sessionForFolder(folder, true) : availableSession();
+    if (connection == NULL || !connection->reserve()) {
+        // sessionForFolder only hands out a reserved connection when the whole pool is
+        // reserved. Reserving it again would give one connection two lease holders.
+        return NULL;
+    }
+    return connection;
+}
+
+void IMAPAsyncSession::releaseConnection(IMAPAsyncConnection * connection, unsigned int leaseGeneration, bool disconnect)
+{
+    if (connection == NULL || connection->owner() != this || !connection->endLease(leaseGeneration, disconnect)) {
+        // Idempotent by contract: a second release would enqueue its disconnect on a
+        // connection that is back in the shared pool - or, once leased again, under its next
+        // holder, which the generation check is for.
+        return;
+    }
+    if (!disconnect) {
+        // The idle timer died if it fired during the lease; arm it anew so a pooled connection
+        // nobody picks up still goes away. The disconnect path needs none: the connection is
+        // down already.
+        connection->scheduleAutomaticDisconnect();
+    }
+}
+
 IMAPFolderInfoOperation * IMAPAsyncSession::folderInfoOperation(String * folder)
 {
     IMAPFolderInfoOperation * op = new IMAPFolderInfoOperation();