From 1749c8ac01168edec6df9fbb23f7bec6cddb0dc8 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Fri, 11 Sep 2026 16:42:56 +0300 Subject: [PATCH 1/3] Add: lease a pooled IMAP connection for exclusive use COR-201 IMAPAsyncSession::acquireConnection(folder) reserves one connection of the pool: the per-operation selection skips it, so only operations pinned to it with IMAPOperation::setSession run there, and the idle auto-disconnect stands down until releaseConnection hands it back (optionally tearing the socket down first, for servers that pin a mailbox view per connection). A lease generation counter makes a late or duplicated release harmless to the next holder. With every connection reserved and the pool at its limit, ordinary selection falls back to sharing the least busy reserved one, as documented. IMAPSession::lastLoginTime() reports the wall-clock moment of the last successful LOGIN, so a client can tell whether a pooled connection's mailbox view predates an event of its own. The idle disconnect delay is configurable (default unchanged at 30 s) so the stand-down can be observed in a test. C bridge and Swift wrapper: MCOIMAPAsyncConnection handle (returns a dropped lease from deinit), MCOIMAPSession.acquireConnection / releaseConnection, MCOIMAPBaseOperation.setConnection. Nothing changes for a client that does not call the new API. Co-Authored-By: Claude Fable 5.1 --- build-windows-5.10/build_headers.list | 1 + configure-headers.sh | 1 + src/async/imap/MCIMAPAsyncConnection.cpp | 100 ++++++++++++++++++- src/async/imap/MCIMAPAsyncConnection.h | 36 +++++++ src/async/imap/MCIMAPAsyncSession.cpp | 70 ++++++++++++- src/async/imap/MCIMAPAsyncSession.h | 46 +++++++++ src/c/CCore.h | 1 + src/c/imap/CIMAPAsyncConnection.cpp | 18 ++++ src/c/imap/CIMAPAsyncConnection.h | 33 ++++++ src/c/imap/CIMAPAsyncSession.cpp | 7 ++ src/c/imap/CIMAPAsyncSession.h | 4 + src/c/imap/CIMAPBaseOperation.cpp | 1 + src/c/imap/CIMAPBaseOperation.h | 2 + src/cmake/cmailcore-public-headers.cmake | 1 + src/cmake/public-headers.cmake | 1 + src/core/imap/MCIMAPSession.cpp | 25 +++++ src/core/imap/MCIMAPSession.h | 7 ++ src/include/MailCore/CCore.h | 1 + src/include/MailCore/CIMAPAsyncConnection.h | 33 ++++++ src/include/MailCore/CIMAPAsyncSession.h | 4 + src/include/MailCore/CIMAPBaseOperation.h | 2 + src/include/MailCore/MCIMAPAsyncConnection.h | 36 +++++++ src/include/MailCore/MCIMAPAsyncSession.h | 46 +++++++++ src/include/MailCore/MCIMAPSession.h | 7 ++ src/swift/imap/IMAPAsyncConnection.swift | 98 ++++++++++++++++++ src/swift/imap/IMAPBaseOperation.swift | 12 +++ src/swift/imap/IMAPSession.swift | 57 ++++++++++- 27 files changed, 643 insertions(+), 7 deletions(-) create mode 100644 src/c/imap/CIMAPAsyncConnection.cpp create mode 100644 src/c/imap/CIMAPAsyncConnection.h create mode 100644 src/include/MailCore/CIMAPAsyncConnection.h create mode 100644 src/swift/imap/IMAPAsyncConnection.swift diff --git a/build-windows-5.10/build_headers.list b/build-windows-5.10/build_headers.list index afdb61c6c..3488b264b 100644 --- a/build-windows-5.10/build_headers.list +++ b/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 diff --git a/configure-headers.sh b/configure-headers.sh index 19974991c..4451934b5 100755 --- a/configure-headers.sh +++ b/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 diff --git a/src/async/imap/MCIMAPAsyncConnection.cpp b/src/async/imap/MCIMAPAsyncConnection.cpp index 8b9b07efb..06467e263 100644 --- a/src/async/imap/MCIMAPAsyncConnection.cpp +++ b/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,9 +394,9 @@ 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) { @@ -346,12 +404,50 @@ void IMAPAsyncConnection::tryAutomaticDisconnect() } } +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(); } diff --git a/src/async/imap/MCIMAPAsyncConnection.h b/src/async/imap/MCIMAPAsyncConnection.h index 41b3ddd2f..a8c7b2742 100644 --- a/src/async/imap/MCIMAPAsyncConnection.h +++ b/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,8 +110,15 @@ 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); @@ -116,12 +126,38 @@ namespace mailcore { 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(); diff --git a/src/async/imap/MCIMAPAsyncSession.cpp b/src/async/imap/MCIMAPAsyncSession.cpp index d29754e16..7a4e680be 100644 --- a/src/async/imap/MCIMAPAsyncSession.cpp +++ b/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,6 +403,11 @@ 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; @@ -384,8 +415,8 @@ IMAPAsyncConnection * IMAPAsyncSession::sessionWithMinQueue(bool filterByFolder, 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(); diff --git a/src/async/imap/MCIMAPAsyncSession.h b/src/async/imap/MCIMAPAsyncSession.h index 76e8503b3..604be5f76 100644 --- a/src/async/imap/MCIMAPAsyncSession.h +++ b/src/async/imap/MCIMAPAsyncSession.h @@ -93,6 +93,46 @@ namespace mailcore { virtual void setMaximumConnections(unsigned int maxConnections); virtual unsigned int maximumConnections(); + + // How long an idle connection stays open once its queue drains, in seconds. + // Applied to connections created after the change. + virtual void setAutomaticDisconnectDelay(time_t delay); + virtual time_t automaticDisconnectDelay(); + + /*! Reserves a connection for exclusive use: the regular per-operation selection stops + seeing it, so only operations explicitly pointed at it (IMAPOperation::setSession) run + there, and the idle auto-disconnect stands down until release. Exclusivity is + forward-only: operations already queued on the connection still run ahead of the lease + holder's (with folder concurrent access allowed, selection prefers an idle or new + connection, so a backlog is only possible with the pool at its limit; without it, a busy + connection selected to the folder is taken as is). Returns NULL when every connection + is already reserved - + the pool has nothing left to hand out exclusively. + + The reverse degradation is the one to size for, because a holder cannot detect it. While + the pool is at its limit and every connection is reserved, the ordinary per-operation + selection stops finding a free connection and shares the least busy reserved one: that + operation runs on somebody's leased connection and SELECTs its own mailbox there, which + is exactly the cross-talk a lease exists to prevent. The holder is given no signal, so + exclusivity holds only while maximumConnections exceeds the number of simultaneous + leases, and nothing enforces that - DEFAULT_MAX_CONNECTIONS is 3, so three concurrent + leases are enough to reach it. + Reservation state is guarded, but that only makes it readable - it does not make the + lease safe on its own. Selection reads it from sessionWithMinQueue, which runs wherever + IMAPOperation::start was called, so an acquire racing a start can hand the same + connection to both: the start sees it free, the acquire reserves it, and the operation + is already queued. Serialize acquireConnection and releaseConnection with every + start() on this session, on a queue of your choosing. */ + virtual IMAPAsyncConnection * acquireConnection(String * folder); + /*! Returns a reserved connection to the shared pool and re-arms its idle + auto-disconnect. leaseGeneration is what the connection reported right after + acquireConnection handed it out; a release carrying an older value is refused, because + the connection has been released and leased again since, and going through would clear + the new holder's reservation. With disconnect, tears the socket down first (the + connection object stays pooled and reconnects on next use) - for servers that pin a + mailbox snapshot per connection. Idempotent: releasing a connection that is not reserved + does nothing. Same threading contract as acquireConnection. */ + virtual void releaseConnection(IMAPAsyncConnection * connection, unsigned int leaseGeneration, bool disconnect); virtual void setConnectionLogger(ConnectionLogger * logger); virtual ConnectionLogger * connectionLogger(); @@ -211,6 +251,7 @@ namespace mailcore { time_t mTimeout; bool mAllowsFolderConcurrentAccessEnabled; unsigned int mMaximumConnections; + time_t mAutomaticDisconnectDelay; ConnectionLogger * mConnectionLogger; bool mAutomaticConfigurationDone; IMAPIdentity * mServerIdentity; @@ -233,6 +274,11 @@ namespace mailcore { predicate ( lastFolder() EQUALS TO @param folder ). In case of param folder is NULL the function would search a session among non-selected ones. */ virtual IMAPAsyncConnection * sessionWithMinQueue(bool filterByFolder, String * folder); + // The same pick, allowed to consider connections a lease has reserved - for the fallback + // in availableSession, and nothing else. Reserved connections are skipped otherwise. An + // overload rather than a default argument on the declaration above, so that declaration + // and any override of it stay as they are. + virtual IMAPAsyncConnection * sessionWithMinQueue(bool filterByFolder, String * folder, bool includeReserved); /*! Returns existant or new session with empty operation queue, if it can. Otherwise, returns the session with the minimum size of the operation queue. */ virtual IMAPAsyncConnection * availableSession(); diff --git a/src/c/CCore.h b/src/c/CCore.h index 968fffb45..23d9ef6b1 100644 --- a/src/c/CCore.h +++ b/src/c/CCore.h @@ -14,6 +14,7 @@ #include #include +#include #include #include #include diff --git a/src/c/imap/CIMAPAsyncConnection.cpp b/src/c/imap/CIMAPAsyncConnection.cpp new file mode 100644 index 000000000..39c3a554c --- /dev/null +++ b/src/c/imap/CIMAPAsyncConnection.cpp @@ -0,0 +1,18 @@ +#include +#include + +#include "CIMAPAsyncConnection.h" +#include "CIMAPBaseOperation.h" + +#include "CBase+Private.h" + +#define nativeType mailcore::IMAPAsyncConnection +#define structName CIMAPAsyncConnection + +C_SYNTHESIZE_CONSTRUCTOR() + +C_SYNTHESIZE_FUNC_WITH_SCALAR(bool, isReserved) +C_SYNTHESIZE_FUNC_WITH_SCALAR(unsigned int, leaseGeneration) +C_SYNTHESIZE_FUNC_WITH_SCALAR(unsigned int, operationsCount) +C_SYNTHESIZE_FUNC_WITH_SCALAR(double, lastLoginTime) +C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, disconnectOperation) diff --git a/src/c/imap/CIMAPAsyncConnection.h b/src/c/imap/CIMAPAsyncConnection.h new file mode 100644 index 000000000..d253fb482 --- /dev/null +++ b/src/c/imap/CIMAPAsyncConnection.h @@ -0,0 +1,33 @@ +#ifndef MAILCORE_CIMAP_ASYNC_CONNECTION_H +#define MAILCORE_CIMAP_ASYNC_CONNECTION_H + +#include + +#include "CBase.h" + +#ifdef __cplusplus + +namespace mailcore { + class IMAPAsyncConnection; +} + +extern "C" { +#endif + + // completed in CIMAPBaseOperation.h; including it here would be circular + typedef struct CIMAPBaseOperation CIMAPBaseOperation; + + C_SYNTHESIZE_STRUCT_DEFINITION(CIMAPAsyncConnection, mailcore::IMAPAsyncConnection) + + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, bool, isReserved) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, unsigned int, leaseGeneration) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, unsigned int, operationsCount) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, double, lastLoginTime) + + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncConnection, CIMAPBaseOperation, disconnectOperation) + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/src/c/imap/CIMAPAsyncSession.cpp b/src/c/imap/CIMAPAsyncSession.cpp index c76771b26..3665a5542 100644 --- a/src/c/imap/CIMAPAsyncSession.cpp +++ b/src/c/imap/CIMAPAsyncSession.cpp @@ -2,6 +2,7 @@ #include #include "CIMAPAsyncSession.h" +#include "CIMAPAsyncConnection.h" #include "CIMAPAppendMessageOperation.h" #include "CIMAPCopyMessagesOperation.h" #include "CIMAPFetchContentOperation.h" @@ -20,6 +21,7 @@ C_SYNTHESIZE_STRING(setUsername, username) C_SYNTHESIZE_STRING(setPassword, password) C_SYNTHESIZE_ENUM(ConnectionType, mailcore::ConnectionType, setConnectionType, connectionType) C_SYNTHESIZE_SCALAR(time_t, time_t, setTimeout, timeout) +C_SYNTHESIZE_SCALAR(time_t, time_t, setAutomaticDisconnectDelay, automaticDisconnectDelay) C_SYNTHESIZE_BOOL(setCheckCertificateEnabled, isCheckCertificateEnabled) C_SYNTHESIZE_STRING(setOAuth2Token, OAuth2Token) C_SYNTHESIZE_ENUM(CAuthType, mailcore::AuthType, setAuthType, authType) @@ -86,6 +88,7 @@ C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPIdentity, serverIdentity) C_SYNTHESIZE_FUNC_WITH_SCALAR(bool, isIdleEnabled) C_SYNTHESIZE_FUNC_WITH_SCALAR(bool, isOperationQueueRunning) C_SYNTHESIZE_FUNC_WITH_VOID(cancelAllOperations) +C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPAsyncConnection, acquireConnection, MailCoreString) C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, subscribeFolderOperation, MailCoreString) C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, unsubscribeFolderOperation, MailCoreString) C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, renameFolderOperation, MailCoreString, MailCoreString) @@ -101,6 +104,10 @@ CIMAPCustomCommandOperation CIMAPAsyncSession_customCommandOperation(struct CIMA return CIMAPCustomCommandOperation_new(self.instance->customCommand(command.instance, false)); } +void CIMAPAsyncSession_releaseConnection(struct CIMAPAsyncSession self, CIMAPAsyncConnection connection, unsigned int leaseGeneration, bool disconnect) { + self.instance->releaseConnection(connection.instance, leaseGeneration, disconnect); +} + C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, connectOperation) C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, disconnectOperation) C_SYNTHESIZE_FUNC_WITH_OBJ(CIMAPBaseOperation, noopOperation) diff --git a/src/c/imap/CIMAPAsyncSession.h b/src/c/imap/CIMAPAsyncSession.h index 1fc2941e9..413ae9595 100644 --- a/src/c/imap/CIMAPAsyncSession.h +++ b/src/c/imap/CIMAPAsyncSession.h @@ -22,6 +22,7 @@ #include "CIMAPFolderInfoOperation.h" #include "CIMAPFolderStatusOperation.h" #include "CMessageConstants.h" +#include "CIMAPAsyncConnection.h" #include "CIMAPIdleOperation.h" #include "CIMAPFetchFoldersOperation.h" #include "CIMAPCapabilityOperation.h" @@ -70,6 +71,7 @@ extern "C" { C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, MailCoreString, OAuth2Token, setOAuth2Token) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CAuthType, authType, setAuthType) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, unsigned int, maximumConnections, setMaximumConnections) + C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, time_t, automaticDisconnectDelay, setAutomaticDisconnectDelay) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, bool, allowsFolderConcurrentAccessEnabled, setAllowsFolderConcurrentAccessEnabled) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CIMAPNamespace, defaultNamespace, setDefaultNamespace) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CIMAPIdentity, clientIdentity, setClientIdentity) @@ -85,6 +87,8 @@ extern "C" { C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncSession, bool, isOperationQueueRunning) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, void, cancelAllOperations) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPAsyncConnection, acquireConnection, MailCoreString) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, void, releaseConnection, CIMAPAsyncConnection, unsigned int, bool) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, connectOperation) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, disconnectOperation) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, noopOperation) diff --git a/src/c/imap/CIMAPBaseOperation.cpp b/src/c/imap/CIMAPBaseOperation.cpp index b6768026b..478d27c93 100644 --- a/src/c/imap/CIMAPBaseOperation.cpp +++ b/src/c/imap/CIMAPBaseOperation.cpp @@ -50,6 +50,7 @@ ErrorCode CIMAPBaseOperation_error(struct CIMAPBaseOperation self) { } C_SYNTHESIZE_FUNC_WITH_SCALAR(bool, interruptCurrentCommand) +C_SYNTHESIZE_FUNC_WITH_VOID(setSession, CIMAPAsyncConnection) CIMAPBaseOperation CIMAPBaseOperation_setProgressBlocks(struct CIMAPBaseOperation self, CIMAPProgressBlock itemProgressBlock, CIMAPProgressBlock bodyProgressBlock, const void* userInfo) { CIMAPBaseOperationIMAPCallback *callback = new CIMAPBaseOperationIMAPCallback(userInfo, itemProgressBlock, bodyProgressBlock); diff --git a/src/c/imap/CIMAPBaseOperation.h b/src/c/imap/CIMAPBaseOperation.h index ac711dd64..4331b784d 100644 --- a/src/c/imap/CIMAPBaseOperation.h +++ b/src/c/imap/CIMAPBaseOperation.h @@ -2,6 +2,7 @@ #define MAILCORE_CIMAP_BASE_OPERATION_H #include "COperation.h" +#include "CIMAPAsyncConnection.h" #ifdef __cplusplus class CIMAPBaseOperationIMAPCallback; @@ -36,6 +37,7 @@ extern "C" { C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, ErrorCode, error) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, bool, interruptCurrentCommand) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, void, setSession, CIMAPAsyncConnection) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, CIMAPBaseOperation, setProgressBlocks, CIMAPProgressBlock, CIMAPProgressBlock, const void*) CMAILCORE_EXPORT void CIMAPBaseOperation_retain(CIMAPBaseOperation operation) diff --git a/src/cmake/cmailcore-public-headers.cmake b/src/cmake/cmailcore-public-headers.cmake index b972fedd6..967af4ebe 100644 --- a/src/cmake/cmailcore-public-headers.cmake +++ b/src/cmake/cmailcore-public-headers.cmake @@ -21,6 +21,7 @@ set(CMAILCORE_BASETYPES_HEADERS set(CMAILCORE_IMAP_HEADERS c/imap/CIMAPAppendMessageOperation.h + c/imap/CIMAPAsyncConnection.h c/imap/CIMAPAsyncSession.h c/imap/CIMAPBaseOperation.h c/imap/CIMAPCapabilityOperation.h diff --git a/src/cmake/public-headers.cmake b/src/cmake/public-headers.cmake index 4bd260ee4..bca6261e3 100644 --- a/src/cmake/public-headers.cmake +++ b/src/cmake/public-headers.cmake @@ -89,6 +89,7 @@ set(MAILCORE2_ASYNC_HEADERS async/MCAsync.h async/imap/MCAsyncIMAP.h async/imap/MCIMAPAppendMessageOperation.h + async/imap/MCIMAPAsyncConnection.h async/imap/MCIMAPAsyncSession.h async/imap/MCIMAPCapabilityOperation.h async/imap/MCIMAPCheckAccountOperation.h diff --git a/src/core/imap/MCIMAPSession.cpp b/src/core/imap/MCIMAPSession.cpp index 1be5c3fd8..93b77f63a 100644 --- a/src/core/imap/MCIMAPSession.cpp +++ b/src/core/imap/MCIMAPSession.cpp @@ -5,6 +5,9 @@ #include #include #include +#ifndef _MSC_VER +#include +#endif #include "MCDefines.h" #include "MCIMAPSearchExpression.h" @@ -31,6 +34,13 @@ using namespace mailcore; +static double currentWallClockTime() +{ + struct timeval tv; + gettimeofday(&tv, NULL); + return (double) tv.tv_sec + (double) tv.tv_usec / 1000000.0; +} + class LoadByChunkProgress : public Object, public IMAPProgressCallback { public: LoadByChunkProgress(); @@ -374,6 +384,7 @@ void IMAPSession::init() mConnectionType = ConnectionTypeClear; mCheckCertificateEnabled = true; mVoIPEnabled = true; + mLastLoginTime = 0; mQResyncCompatible = true; mDelimiter = 0; @@ -1022,6 +1033,12 @@ void IMAPSession::login(ErrorCode * pError) MC_SAFE_REPLACE_COPY(String, mLoginResponse, loginResponse); mState = STATE_LOGGEDIN; + // Written from the connection's operation thread, read from the session's dispatch queue: + // on a 32-bit ABI an unlocked double is two stores, and a torn read could report a moment + // in the future - the one direction in which a caller would wrongly believe the view fresh. + LOCK(); + mLastLoginTime = currentWallClockTime(); + UNLOCK(); if (isAutomaticConfigurationEnabled()) { if ((mImap->imap_connection_info != NULL) && (mImap->imap_connection_info->imap_capability != NULL)) { @@ -4398,6 +4415,14 @@ bool IMAPSession::isDisconnected() return mState == STATE_DISCONNECTED; } +double IMAPSession::lastLoginTime() +{ + LOCK(); + double lastLoginTime = mLastLoginTime; + UNLOCK(); + return lastLoginTime; +} + void IMAPSession::setConnectionLogger(ConnectionLogger * logger) { lockConnectionLogger(); diff --git a/src/core/imap/MCIMAPSession.h b/src/core/imap/MCIMAPSession.h index be1fe6004..5282b4cec 100644 --- a/src/core/imap/MCIMAPSession.h +++ b/src/core/imap/MCIMAPSession.h @@ -238,6 +238,12 @@ namespace mailcore { virtual void connectIfNeeded(ErrorCode * pError); virtual void selectIfNeeded(String * folder, ErrorCode * pError); virtual bool isDisconnected(); + + // Wall-clock moment (seconds since the epoch, sub-second resolution) of the last + // successful LOGIN on this session, 0 when it has never logged in. A client that has to + // know whether a pooled connection's mailbox view predates some event of its own + // compares against this instead of tracking disconnects it cannot observe. + virtual double lastLoginTime(); virtual bool isAutomaticConfigurationDone(); virtual void resetAutomaticConfigurationDone(); virtual void applyCapabilities(IndexSet * capabilities); @@ -293,6 +299,7 @@ namespace mailcore { String * mCurrentFolder; MCB_LOCK_TYPE mIdleLock; int mState; + double mLastLoginTime; mailimap * mImap; IMAPProgressCallback * mProgressCallback; unsigned int mProgressItemsCount; diff --git a/src/include/MailCore/CCore.h b/src/include/MailCore/CCore.h index 968fffb45..23d9ef6b1 100644 --- a/src/include/MailCore/CCore.h +++ b/src/include/MailCore/CCore.h @@ -14,6 +14,7 @@ #include #include +#include #include #include #include diff --git a/src/include/MailCore/CIMAPAsyncConnection.h b/src/include/MailCore/CIMAPAsyncConnection.h new file mode 100644 index 000000000..d253fb482 --- /dev/null +++ b/src/include/MailCore/CIMAPAsyncConnection.h @@ -0,0 +1,33 @@ +#ifndef MAILCORE_CIMAP_ASYNC_CONNECTION_H +#define MAILCORE_CIMAP_ASYNC_CONNECTION_H + +#include + +#include "CBase.h" + +#ifdef __cplusplus + +namespace mailcore { + class IMAPAsyncConnection; +} + +extern "C" { +#endif + + // completed in CIMAPBaseOperation.h; including it here would be circular + typedef struct CIMAPBaseOperation CIMAPBaseOperation; + + C_SYNTHESIZE_STRUCT_DEFINITION(CIMAPAsyncConnection, mailcore::IMAPAsyncConnection) + + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, bool, isReserved) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, unsigned int, leaseGeneration) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, unsigned int, operationsCount) + C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncConnection, double, lastLoginTime) + + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncConnection, CIMAPBaseOperation, disconnectOperation) + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/src/include/MailCore/CIMAPAsyncSession.h b/src/include/MailCore/CIMAPAsyncSession.h index 1fc2941e9..413ae9595 100644 --- a/src/include/MailCore/CIMAPAsyncSession.h +++ b/src/include/MailCore/CIMAPAsyncSession.h @@ -22,6 +22,7 @@ #include "CIMAPFolderInfoOperation.h" #include "CIMAPFolderStatusOperation.h" #include "CMessageConstants.h" +#include "CIMAPAsyncConnection.h" #include "CIMAPIdleOperation.h" #include "CIMAPFetchFoldersOperation.h" #include "CIMAPCapabilityOperation.h" @@ -70,6 +71,7 @@ extern "C" { C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, MailCoreString, OAuth2Token, setOAuth2Token) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CAuthType, authType, setAuthType) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, unsigned int, maximumConnections, setMaximumConnections) + C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, time_t, automaticDisconnectDelay, setAutomaticDisconnectDelay) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, bool, allowsFolderConcurrentAccessEnabled, setAllowsFolderConcurrentAccessEnabled) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CIMAPNamespace, defaultNamespace, setDefaultNamespace) C_SYNTHESIZE_PROPERTY_DEFINITION(CIMAPAsyncSession, CIMAPIdentity, clientIdentity, setClientIdentity) @@ -85,6 +87,8 @@ extern "C" { C_SYNTHESIZE_READONLY_PROPERTY_DEFINITION(CIMAPAsyncSession, bool, isOperationQueueRunning) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, void, cancelAllOperations) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPAsyncConnection, acquireConnection, MailCoreString) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, void, releaseConnection, CIMAPAsyncConnection, unsigned int, bool) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, connectOperation) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, disconnectOperation) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPAsyncSession, CIMAPBaseOperation, noopOperation) diff --git a/src/include/MailCore/CIMAPBaseOperation.h b/src/include/MailCore/CIMAPBaseOperation.h index ac711dd64..4331b784d 100644 --- a/src/include/MailCore/CIMAPBaseOperation.h +++ b/src/include/MailCore/CIMAPBaseOperation.h @@ -2,6 +2,7 @@ #define MAILCORE_CIMAP_BASE_OPERATION_H #include "COperation.h" +#include "CIMAPAsyncConnection.h" #ifdef __cplusplus class CIMAPBaseOperationIMAPCallback; @@ -36,6 +37,7 @@ extern "C" { C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, ErrorCode, error) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, bool, interruptCurrentCommand) + C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, void, setSession, CIMAPAsyncConnection) C_SYNTHESIZE_FUNC_DEFINITION(CIMAPBaseOperation, CIMAPBaseOperation, setProgressBlocks, CIMAPProgressBlock, CIMAPProgressBlock, const void*) CMAILCORE_EXPORT void CIMAPBaseOperation_retain(CIMAPBaseOperation operation) diff --git a/src/include/MailCore/MCIMAPAsyncConnection.h b/src/include/MailCore/MCIMAPAsyncConnection.h index 41b3ddd2f..a8c7b2742 100644 --- a/src/include/MailCore/MCIMAPAsyncConnection.h +++ b/src/include/MailCore/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,8 +110,15 @@ 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); @@ -116,12 +126,38 @@ namespace mailcore { 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(); diff --git a/src/include/MailCore/MCIMAPAsyncSession.h b/src/include/MailCore/MCIMAPAsyncSession.h index 76e8503b3..604be5f76 100644 --- a/src/include/MailCore/MCIMAPAsyncSession.h +++ b/src/include/MailCore/MCIMAPAsyncSession.h @@ -93,6 +93,46 @@ namespace mailcore { virtual void setMaximumConnections(unsigned int maxConnections); virtual unsigned int maximumConnections(); + + // How long an idle connection stays open once its queue drains, in seconds. + // Applied to connections created after the change. + virtual void setAutomaticDisconnectDelay(time_t delay); + virtual time_t automaticDisconnectDelay(); + + /*! Reserves a connection for exclusive use: the regular per-operation selection stops + seeing it, so only operations explicitly pointed at it (IMAPOperation::setSession) run + there, and the idle auto-disconnect stands down until release. Exclusivity is + forward-only: operations already queued on the connection still run ahead of the lease + holder's (with folder concurrent access allowed, selection prefers an idle or new + connection, so a backlog is only possible with the pool at its limit; without it, a busy + connection selected to the folder is taken as is). Returns NULL when every connection + is already reserved - + the pool has nothing left to hand out exclusively. + + The reverse degradation is the one to size for, because a holder cannot detect it. While + the pool is at its limit and every connection is reserved, the ordinary per-operation + selection stops finding a free connection and shares the least busy reserved one: that + operation runs on somebody's leased connection and SELECTs its own mailbox there, which + is exactly the cross-talk a lease exists to prevent. The holder is given no signal, so + exclusivity holds only while maximumConnections exceeds the number of simultaneous + leases, and nothing enforces that - DEFAULT_MAX_CONNECTIONS is 3, so three concurrent + leases are enough to reach it. + Reservation state is guarded, but that only makes it readable - it does not make the + lease safe on its own. Selection reads it from sessionWithMinQueue, which runs wherever + IMAPOperation::start was called, so an acquire racing a start can hand the same + connection to both: the start sees it free, the acquire reserves it, and the operation + is already queued. Serialize acquireConnection and releaseConnection with every + start() on this session, on a queue of your choosing. */ + virtual IMAPAsyncConnection * acquireConnection(String * folder); + /*! Returns a reserved connection to the shared pool and re-arms its idle + auto-disconnect. leaseGeneration is what the connection reported right after + acquireConnection handed it out; a release carrying an older value is refused, because + the connection has been released and leased again since, and going through would clear + the new holder's reservation. With disconnect, tears the socket down first (the + connection object stays pooled and reconnects on next use) - for servers that pin a + mailbox snapshot per connection. Idempotent: releasing a connection that is not reserved + does nothing. Same threading contract as acquireConnection. */ + virtual void releaseConnection(IMAPAsyncConnection * connection, unsigned int leaseGeneration, bool disconnect); virtual void setConnectionLogger(ConnectionLogger * logger); virtual ConnectionLogger * connectionLogger(); @@ -211,6 +251,7 @@ namespace mailcore { time_t mTimeout; bool mAllowsFolderConcurrentAccessEnabled; unsigned int mMaximumConnections; + time_t mAutomaticDisconnectDelay; ConnectionLogger * mConnectionLogger; bool mAutomaticConfigurationDone; IMAPIdentity * mServerIdentity; @@ -233,6 +274,11 @@ namespace mailcore { predicate ( lastFolder() EQUALS TO @param folder ). In case of param folder is NULL the function would search a session among non-selected ones. */ virtual IMAPAsyncConnection * sessionWithMinQueue(bool filterByFolder, String * folder); + // The same pick, allowed to consider connections a lease has reserved - for the fallback + // in availableSession, and nothing else. Reserved connections are skipped otherwise. An + // overload rather than a default argument on the declaration above, so that declaration + // and any override of it stay as they are. + virtual IMAPAsyncConnection * sessionWithMinQueue(bool filterByFolder, String * folder, bool includeReserved); /*! Returns existant or new session with empty operation queue, if it can. Otherwise, returns the session with the minimum size of the operation queue. */ virtual IMAPAsyncConnection * availableSession(); diff --git a/src/include/MailCore/MCIMAPSession.h b/src/include/MailCore/MCIMAPSession.h index be1fe6004..5282b4cec 100644 --- a/src/include/MailCore/MCIMAPSession.h +++ b/src/include/MailCore/MCIMAPSession.h @@ -238,6 +238,12 @@ namespace mailcore { virtual void connectIfNeeded(ErrorCode * pError); virtual void selectIfNeeded(String * folder, ErrorCode * pError); virtual bool isDisconnected(); + + // Wall-clock moment (seconds since the epoch, sub-second resolution) of the last + // successful LOGIN on this session, 0 when it has never logged in. A client that has to + // know whether a pooled connection's mailbox view predates some event of its own + // compares against this instead of tracking disconnects it cannot observe. + virtual double lastLoginTime(); virtual bool isAutomaticConfigurationDone(); virtual void resetAutomaticConfigurationDone(); virtual void applyCapabilities(IndexSet * capabilities); @@ -293,6 +299,7 @@ namespace mailcore { String * mCurrentFolder; MCB_LOCK_TYPE mIdleLock; int mState; + double mLastLoginTime; mailimap * mImap; IMAPProgressCallback * mProgressCallback; unsigned int mProgressItemsCount; diff --git a/src/swift/imap/IMAPAsyncConnection.swift b/src/swift/imap/IMAPAsyncConnection.swift new file mode 100644 index 000000000..72aede8bf --- /dev/null +++ b/src/swift/imap/IMAPAsyncConnection.swift @@ -0,0 +1,98 @@ +import Foundation +import CMailCore + +/** + One IMAP connection of an MCOIMAPSession's pool, acquired for exclusive use via + MCOIMAPSession.acquireConnection(folder:). + + While held, the session's regular per-operation connection selection skips this connection, so + the only commands running on it are those explicitly pointed at it with + MCOIMAPBaseOperation.setConnection(_:). Hand it back with + MCOIMAPSession.releaseConnection(_:disconnect:) - a leaked lease permanently degrades the + pool: the connection is never handed out exclusively again and, at the limit, falls back to + being shared. + */ +public class MCOIMAPAsyncConnection: NSObjectCompat { + + internal var connection: CIMAPAsyncConnection + + /// The session this lease came from, held so it cannot be destroyed first: the C++ connection + /// keeps a raw pointer back to its owner and reaches through it on every operation, so an + /// outlived session is a use-after-free rather than a nil check. + private let session: MCOIMAPSession + + /// The reservation this handle was made for. A connection released and acquired again is a + /// different lease on the same object, and the session refuses a release carrying this value + /// once that has happened — otherwise a duplicated cleanup path (a defer plus an explicit + /// release, an error path plus a normal one) would cancel the next holder's lease and tear + /// down the socket underneath it. + internal let leaseGeneration: UInt32 + + internal init(connection: CIMAPAsyncConnection, session: MCOIMAPSession) { + self.connection = connection + self.session = session + self.leaseGeneration = connection.leaseGeneration + self.connection.retain() + } + + /// Returns the lease if its holder never did. A leaked lease is permanent otherwise — nothing + /// in the pool clears a reservation on its own — and the connection would be lost to the pool + /// for the life of the session. Torn down rather than pooled: a holder that lost track of its + /// lease cannot have left the connection in a state anybody should inherit. Best effort: + /// deinit runs on whatever thread drops the last reference, outside the serialisation the + /// release contract asks for; a holder that releases explicitly never gets here. + deinit { + session.releaseConnection(self, disconnect: true) + connection.release() + } + + internal var isReserved: Bool { + return connection.isReserved + } + + /** + Stable identity of the underlying pool connection, constant for the lifetime of the owning + MCOIMAPSession (its pool is never pruned): two handles from separate acquisitions compare + equal here when they lease the same connection. + */ + public var identity: UInt { + return UInt(bitPattern: connection.instance) + } + + internal var operationsCount: UInt32 { + return connection.operationsCount + } + + /** + Wall-clock moment of this connection's last successful LOGIN, nil when it has never logged + in. On a server that pins the mailbox view per connection, this answers the only question + that matters at the start of a lease: whether the view this connection holds was taken + before or after some event of the caller's own. Logins the pool performs on its own - after + its automatic disconnect, a dropped socket, an error retry - move it, so a caller stays + correct without observing them. + + A disconnect does not move it: between the disconnect and the next login the value still + reports the previous login, which reads as older than it is and so errs towards a caller + refreshing a connection that needed no refresh, never the other way. + + Wall clock, so a comparison against a moment the caller recorded the same way is only as + reliable as the clock: a step backwards between the login and the caller's own event can + make the login look later than it was. + */ + public var lastLoginDate: Date? { + let value = connection.lastLoginTime + return value > 0 ? Date(timeIntervalSince1970: value) : nil + } + + /** + Returns an operation that disconnects this connection only: the object stays pooled (and + leased, if it is), and the next command on it logs in from scratch. With a lease on a + server that pins the mailbox view per connection, this is how the holder forces a view no + older than itself — start it before the commands whose freshness matters. + */ + public func disconnectOperation() -> MCOIMAPOperation { + return mailCoreAutoreleasePool { + return MCOIMAPOperation(operation: connection.disconnectOperation()) + } + } +} diff --git a/src/swift/imap/IMAPBaseOperation.swift b/src/swift/imap/IMAPBaseOperation.swift index 84abb17fb..32e44ec42 100644 --- a/src/swift/imap/IMAPBaseOperation.swift +++ b/src/swift/imap/IMAPBaseOperation.swift @@ -47,6 +47,18 @@ public class MCOIMAPBaseOperation : MCOOperation { baseOperation.interruptCurrentCommand() } } + + /** + Pins this operation to the given connection: start() runs it there instead of letting the + session pick a connection. Set it before start(); pair with + MCOIMAPSession.acquireConnection(folder:), which is what keeps other operations off that + connection. + */ + public func setConnection(_ connection: MCOIMAPAsyncConnection) { + mailCoreAutoreleasePool { + baseOperation.setSession(connection.connection) + } + } public func itemProgress(current: UInt32, maximum: UInt32) { diff --git a/src/swift/imap/IMAPSession.swift b/src/swift/imap/IMAPSession.swift index bebfe8bd1..eaf77fe6f 100644 --- a/src/swift/imap/IMAPSession.swift +++ b/src/swift/imap/IMAPSession.swift @@ -143,7 +143,62 @@ public class MCOIMAPSession: NSObjectCompat { get { return session.maximumConnections } set { session.maximumConnections = newValue } } - + + /** + Reserves one connection of the pool for exclusive use: the regular per-operation selection + stops seeing it, so new commands run on it only when explicitly pointed at it with + MCOIMAPBaseOperation.setConnection(_:), and the idle auto-disconnect stands down until + release. Exclusivity is forward-only: operations already queued on the connection still + run ahead of the lease holder's (with folder concurrent access allowed, selection prefers + an idle or new connection, so a backlog is only possible with the pool at its limit; + without it, a busy connection selected to the folder is taken as is). + + Returns nil when every connection is already reserved - callers must then fall back to the + shared pool. + + The reverse degradation is the one to size for, because a holder cannot detect it. While the + pool is at its limit and every connection is reserved, the ordinary per-operation selection + stops finding a free connection and shares the least busy reserved one: that operation runs + on somebody's leased connection and SELECTs its own mailbox there, which is exactly the + cross-talk a lease exists to prevent. The holder is given no signal - no callback, no flag - + so exclusivity is a guarantee only while maximumConnections exceeds the number of + simultaneous leases, and nothing enforces that. The default is DEFAULT_MAX_CONNECTIONS (3), + so three concurrent leases are enough to reach it. Every acquired connection + must be handed back with releaseConnection(_:disconnect:): a leaked lease permanently + degrades the pool — the connection is never handed out exclusively again and, at the + limit, falls back to being shared. + + Reservation state is guarded, but that only makes it readable - it does not make the lease + safe on its own. Selection reads it from within MCOIMAPBaseOperation.start, on whatever + thread calls that, so an acquire racing a start can hand the same connection to both: the + start sees it free, the acquire reserves it, and the operation is already queued. Serialize + acquireConnection and releaseConnection with every start() on this session, on a queue of + your choosing. + */ + public func acquireConnection(folder: String?) -> MCOIMAPAsyncConnection? { + return mailCoreAutoreleasePool { + let connection = session.acquireConnection(folder?.mailCoreString() ?? MailCoreString()) + guard connection.instance != nil else { + return nil + } + return MCOIMAPAsyncConnection(connection: connection, session: self) + } + } + + /** + Returns an acquired connection to the shared pool and re-arms its idle auto-disconnect. + + With disconnect, the socket is torn down first (the connection object stays pooled and + reconnects on next use) - for servers that pin a mailbox snapshot per connection. + Idempotent: releasing a connection that is not reserved does nothing. Same threading + contract as acquireConnection(folder:). + */ + public func releaseConnection(_ connection: MCOIMAPAsyncConnection, disconnect: Bool) { + mailCoreAutoreleasePool { + session.releaseConnection(connection.connection, connection.leaseGeneration, disconnect) + } + } + /** Sets logger callback. The network traffic will be sent to this block. From 5f27dfd20a1b35030752da15983dafe181bed308 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Fri, 11 Sep 2026 16:45:34 +0300 Subject: [PATCH 2/3] Fix: an interrupted IMAP connection reconnects on its next command COR-201 interruptCurrentCommand cancels the libetpan stream, and libetpan never clears a stream's cancelled state - every read and write on it fails from then on. Most command paths notice the stream error and schedule a reconnect through mShouldDisconnect; NOOP and the callers of resultsWithError do not, so a connection cut there stayed "connected" and failed its next command instead. The interrupt now sets the flag itself, under the same lock, and the connection heals lazily through connectIfNeeded whichever command was cut. Co-Authored-By: Claude Fable 5.1 --- src/async/imap/MCIMAPOperation.h | 4 +++- src/core/imap/MCIMAPSession.cpp | 5 +++++ src/include/MailCore/MCIMAPOperation.h | 4 +++- src/swift/imap/IMAPBaseOperation.swift | 6 ++++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/async/imap/MCIMAPOperation.h b/src/async/imap/MCIMAPOperation.h index 8075a733a..a62dee913 100644 --- a/src/async/imap/MCIMAPOperation.h +++ b/src/async/imap/MCIMAPOperation.h @@ -53,7 +53,9 @@ namespace mailcore { Teardown of this connection only - it is left unusable and reconnects on next use, so call it for a command that is being abandoned (cancelled, or given up on), never to hurry up a command whose result still matters. - Returns whether a command was actually interrupted. */ + Returns whether this operation was the one the queue was running at that moment. That may + include a command that finished just as the interrupt landed: its result is intact, but + the stream is cancelled all the same. */ virtual bool interruptCurrentCommand(); virtual void start(); diff --git a/src/core/imap/MCIMAPSession.cpp b/src/core/imap/MCIMAPSession.cpp index 93b77f63a..2eee3a795 100644 --- a/src/core/imap/MCIMAPSession.cpp +++ b/src/core/imap/MCIMAPSession.cpp @@ -3740,6 +3740,11 @@ void IMAPSession::interruptCurrentCommand() LOCK(); if (mImap != NULL && mImap->imap_stream != NULL) { mailstream_cancel(mImap->imap_stream); + // libetpan never clears a stream's cancelled state: every read and write on it fails from + // here on. The next command must reconnect, and connectIfNeeded does that for this flag, + // so the connection stays pooled and heals on its own instead of relying on the caller + // to tear it down. + mShouldDisconnect = true; } UNLOCK(); } diff --git a/src/include/MailCore/MCIMAPOperation.h b/src/include/MailCore/MCIMAPOperation.h index 8075a733a..a62dee913 100644 --- a/src/include/MailCore/MCIMAPOperation.h +++ b/src/include/MailCore/MCIMAPOperation.h @@ -53,7 +53,9 @@ namespace mailcore { Teardown of this connection only - it is left unusable and reconnects on next use, so call it for a command that is being abandoned (cancelled, or given up on), never to hurry up a command whose result still matters. - Returns whether a command was actually interrupted. */ + Returns whether this operation was the one the queue was running at that moment. That may + include a command that finished just as the interrupt landed: its result is intact, but + the stream is cancelled all the same. */ virtual bool interruptCurrentCommand(); virtual void start(); diff --git a/src/swift/imap/IMAPBaseOperation.swift b/src/swift/imap/IMAPBaseOperation.swift index 32e44ec42..d8905829a 100644 --- a/src/swift/imap/IMAPBaseOperation.swift +++ b/src/swift/imap/IMAPBaseOperation.swift @@ -38,8 +38,10 @@ public class MCOIMAPBaseOperation : MCOOperation { is rebuilt on next use, so call it for a command being abandoned, never to hurry up one whose result still matters. - - Returns: whether a command was actually interrupted, i.e. whether this operation was the one - running. `false` means nothing was holding the connection on its behalf. + - Returns: whether this operation was the one the queue was running at that moment. That may + include a command that finished just as the interrupt landed: its result is intact, but the + stream is cancelled all the same. `false` means nothing was holding the connection on this + operation's behalf. */ @discardableResult public func interruptCurrentCommand() -> Bool { From 7cb5b4c671449069a5f79f8835a3e1600bf29163 Mon Sep 17 00:00:00 2001 From: Dmytro Bezverkhnii Date: Fri, 11 Sep 2026 16:47:06 +0300 Subject: [PATCH 3/3] Test: connection lease, pinning, interrupt and idle stand-down COR-201 A local TCP endpoint that greets like an IMAP server, answers the commands a test lists (LOGIN, CAPABILITY, LIST) and blocks on the rest, so acquisition, exclusivity, the all-reserved fallback, a stale release, a dropped handle, the idle timer standing down while reserved and the reconnect after an interrupt are all observable without a server. Darwin only: the endpoint is a POSIX socket. Co-Authored-By: Claude Fable 5.1 --- Package.swift | 2 +- unittest/IMAPConnectionLeaseTests.swift | 526 ++++++++++++++++++++++++ unittest/LeaseTestTCPEndpoint.swift | 213 ++++++++++ 3 files changed, 740 insertions(+), 1 deletion(-) create mode 100644 unittest/IMAPConnectionLeaseTests.swift create mode 100644 unittest/LeaseTestTCPEndpoint.swift diff --git a/Package.swift b/Package.swift index 903136d0a..03dec004b 100644 --- a/Package.swift +++ b/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") ] diff --git a/unittest/IMAPConnectionLeaseTests.swift b/unittest/IMAPConnectionLeaseTests.swift new file mode 100644 index 000000000..61c8bf67f --- /dev/null +++ b/unittest/IMAPConnectionLeaseTests.swift @@ -0,0 +1,526 @@ +// +// IMAPConnectionLeaseTests.swift +// mailcore2 +// +// Tests for IMAPAsyncSession::acquireConnection() / releaseConnection() and for pinning an +// operation to a leased connection. +// + +// Darwin only: the tests need a POSIX listening socket, and the Android job builds the test target +// without running it. Nothing here is platform-specific beyond that socket. +#if canImport(Darwin) + +import Darwin +import Dispatch +import Foundation +import XCTest + +#if SWIFT_PACKAGE +import CMailCore +#endif + +@testable import MailCore + +final class IMAPConnectionLeaseTests: XCTestCase { + + /// Well above every wait below: a command left to its own devices must not be able to finish + /// on its own before the test is done observing it. + private let sessionTimeout: TimeInterval = 60 + + private func makeSession(port: UInt16, maximumConnections: UInt32) -> MCOIMAPSession { + let session = MCOIMAPSession() + session.hostname = "127.0.0.1" + session.port = UInt32(port) + session.connectionType = ConnectionTypeClear + session.username = "user" + session.password = "password" + session.timeout = sessionTimeout + session.maximumConnections = maximumConnections + return session + } + + /// Runs the test body off the main thread while the main thread keeps spinning its run loop. + /// mailcore hands parts of an operation's lifecycle to the main queue and waits for them, so a + /// test that blocks the main thread never gets its operation started in the first place. + private func runOffMainThread(timeout: TimeInterval, _ body: @escaping () -> Void) { + let finished = expectation(description: "test body") + + DispatchQueue.global(qos: .userInitiated).async { + body() + finished.fulfill() + } + + waitForExpectations(timeout: timeout) + } + + private func start(_ operation: MCOIMAPOperation) -> DispatchSemaphore { + let finished = DispatchSemaphore(value: 0) + operation.start { _ in + finished.signal() + } + return finished + } + + /// Enqueueing crosses a couple of threads, so occupancy is polled rather than asserted at one + /// instant. The operation itself cannot finish - the endpoint is silent - so once the count + /// reaches the expected value it stays there. + private func waitForOperationsCount(of connection: MCOIMAPAsyncConnection, + toReach expected: UInt32, + timeout: TimeInterval = 5) -> Bool { + let deadline = Date(timeIntervalSinceNow: timeout) + while Date() < deadline { + if connection.operationsCount >= expected { + return true + } + usleep(20_000) + } + return connection.operationsCount >= expected + } + + /// Acquiring needs no server at all: connections come to life on first use, so lease + /// bookkeeping is observable without a single socket. + func testAcquireReservesUntilReleaseAndExhaustionReturnsNil() { + let session = makeSession(port: 1, maximumConnections: 2) + + guard let first = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 2 connections must satisfy the first lease") + } + XCTAssertTrue(first.isReserved) + + // Non-nil proves it is a different connection: acquiring the already-reserved one again + // is exactly what acquireConnection() must refuse. + guard let second = session.acquireConnection(folder: nil) else { + return XCTFail("A pool with room for 2 connections must satisfy a second lease") + } + XCTAssertTrue(second.isReserved) + + XCTAssertNil(session.acquireConnection(folder: nil), + "With every connection reserved there is nothing left to hand out exclusively") + + session.releaseConnection(second, disconnect: false) + XCTAssertFalse(second.isReserved, "Release must return the connection to the shared pool") + + XCTAssertNotNil(session.acquireConnection(folder: nil), + "A released connection must be acquirable again") + } + + func testPinnedOperationRunsOnTheLeasedConnection() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 2) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 2 connections must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + operation.setConnection(leased) + let finished = self.start(operation) + + XCTAssertTrue(self.waitForOperationsCount(of: leased, toReach: 1), + "A pinned operation must land on the leased connection") + + XCTAssertEqual(finished.wait(timeout: .now() + 2), .timedOut, + "The command was expected to be blocked on the silent socket") + + XCTAssertTrue(operation.interruptCurrentCommand()) + XCTAssertEqual(finished.wait(timeout: .now() + 10), .success, + "interruptCurrentCommand() did not unblock the pinned command") + + session.releaseConnection(leased, disconnect: false) + XCTAssertFalse(leased.isReserved) + } + } + + func testInterruptedConnectionReconnectsOnItsNextCommand() throws { + // LOGIN and what mailcore sends right after it (CAPABILITY, the delimiter LIST) are + // answered, so that the command the interrupt cuts is the NOOP itself: a stream error + // inside any of those already schedules a reconnect on its own, one inside NOOP does not. + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n", + answers: ["LOGIN": "", + "CAPABILITY": "* CAPABILITY IMAP4rev1\r\n", + "LIST": "* LIST (\\Noselect) \"/\" \"\"\r\n"]) + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let connect = session.connectOperation() + connect.setConnection(leased) + XCTAssertEqual(self.start(connect).wait(timeout: .now() + 5), .success, + "The greeting endpoint was expected to let the connect finish") + XCTAssertEqual(endpoint.acceptedClientCount, 1) + + // Nothing answers the NOOP, so it blocks until interrupted. + let noop = session.noopOperation() + noop.setConnection(leased) + let finished = self.start(noop) + XCTAssertTrue(self.waitForOperationsCount(of: leased, toReach: 1)) + XCTAssertEqual(finished.wait(timeout: .now() + 2), .timedOut, + "The NOOP was expected to be blocked on the silent socket") + XCTAssertTrue(noop.interruptCurrentCommand()) + XCTAssertEqual(finished.wait(timeout: .now() + 10), .success, + "interruptCurrentCommand() did not unblock the NOOP") + + // Still "connected" as far as its state goes, but on a stream libetpan will never read + // from again. The next command has to start over, and the endpoint sees a second client. + let reconnect = session.connectOperation() + reconnect.setConnection(leased) + XCTAssertEqual(self.start(reconnect).wait(timeout: .now() + 5), .success) + XCTAssertEqual(endpoint.acceptedClientCount, 2, + "An interrupted connection must reconnect on its next command, on its own") + + session.releaseConnection(leased, disconnect: false) + } + } + + func testUnpinnedOperationAvoidsTheLeasedConnection() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 2) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 2 connections must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + let finished = self.start(operation) + + XCTAssertEqual(finished.wait(timeout: .now() + 2), .timedOut, + "The command was expected to be blocked on the silent socket") + + XCTAssertEqual(leased.operationsCount, 0, + "While the pool has room, a regular operation must not touch the leased connection") + + XCTAssertTrue(operation.interruptCurrentCommand()) + XCTAssertEqual(finished.wait(timeout: .now() + 10), .success) + + session.releaseConnection(leased, disconnect: false) + } + } + + /// The one case where exclusivity gives way: a fully reserved pool at its connection limit + /// shares the least busy reserved connection with regular operations instead of crashing on + /// a NULL session. + /// Every other test leases with folder: nil, which takes acquireConnection's availableSession + /// path and leaves the folder branch — both of the hunks that modify sessionForFolder — with + /// no coverage at all. Those are the hunks that have to behave exactly as upstream does when + /// nothing is reserved, so they are the ones worth pinning. + func testAcquireWithAFolderSkipsReservedConnections() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 2) + + guard let first = session.acquireConnection(folder: "INBOX") else { + return XCTFail("An empty pool with room for 2 connections must satisfy the lease") + } + XCTAssertTrue(first.isReserved) + + // The same folder again: the connection already selected to it is reserved, so the pick + // must create the second one rather than hand the first out twice. + guard let second = session.acquireConnection(folder: "INBOX") else { + return XCTFail("The pool had room for a second connection") + } + XCTAssertTrue(second.isReserved) + XCTAssertNotEqual(first.identity, second.identity, + "a reserved connection must never be handed to a second holder") + + XCTAssertNil(session.acquireConnection(folder: "INBOX"), + "with both connections reserved the pool has nothing left to lease") + XCTAssertNil(session.acquireConnection(folder: "Archive"), + "and a different folder does not change that") + + session.releaseConnection(first, disconnect: false) + session.releaseConnection(second, disconnect: false) + } + + /// A release is refused for a connection that belongs to another session's pool: it would + /// clear a reservation that session is relying on and queue a disconnect on a connection this + /// one has no claim to. + func testReleaseIgnoresAConnectionFromAnotherSession() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let owner = makeSession(port: endpoint.port, maximumConnections: 1) + let stranger = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = owner.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + stranger.releaseConnection(leased, disconnect: true) + + XCTAssertTrue(leased.isReserved, "another session must not be able to end this lease") + XCTAssertNil(owner.acquireConnection(folder: nil), + "the owning pool must still consider its only connection leased") + + owner.releaseConnection(leased, disconnect: false) + } + + func testExhaustedPoolSharesTheLeasedConnection() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + let finished = self.start(operation) + + XCTAssertTrue(self.waitForOperationsCount(of: leased, toReach: 1), + "With the whole pool reserved, a regular operation must share the leased connection") + + XCTAssertEqual(finished.wait(timeout: .now() + 2), .timedOut, + "The command was expected to be blocked on the silent socket") + + XCTAssertTrue(operation.interruptCurrentCommand()) + XCTAssertEqual(finished.wait(timeout: .now() + 10), .success) + + session.releaseConnection(leased, disconnect: true) + } + } + + func testAcquirePrefersAnIdleConnectionOverABusyOne() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 2) + + runOffMainThread(timeout: 30) { + // Occupy the first connection with a command stuck on the silent socket. + let blocked = session.connectOperation() + let blockedFinished = self.start(blocked) + XCTAssertEqual(blockedFinished.wait(timeout: .now() + 2), .timedOut, + "The command was expected to be blocked on the silent socket") + + let leased = session.acquireConnection(folder: nil) + if let leased = leased { + XCTAssertEqual(leased.operationsCount, 0, + "With room in the pool, a lease must get an idle or new connection, not the busy one") + } + else { + XCTFail("A pool with room for 2 connections must satisfy the lease") + } + + XCTAssertTrue(blocked.interruptCurrentCommand()) + XCTAssertEqual(blockedFinished.wait(timeout: .now() + 10), .success) + + if let leased = leased { + session.releaseConnection(leased, disconnect: false) + } + } + } + + /// Also exercises the session -> connection plumbing of automaticDisconnectDelay: with the + /// default 30s the timer could not fire inside the observation windows at all. + func testAutomaticDisconnectStandsDownWhileReservedAndReleaseRearmsIt() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + session.session.automaticDisconnectDelay = 1 + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + operation.setConnection(leased) + let finished = self.start(operation) + XCTAssertEqual(finished.wait(timeout: .now() + 5), .success, + "The greeting endpoint was expected to let the connect finish") + + // The queue has drained, so the 1s idle timer is armed. Reserved, the connection + // must survive well past the timer period: the timer stands down instead of firing. + XCTAssertFalse(endpoint.waitForClientDisconnect(timeout: 2.5), + "The idle auto-disconnect must stand down while the connection is reserved") + + session.releaseConnection(leased, disconnect: false) + + // Release re-arms the idle timer, which then closes the pooled socket. + XCTAssertTrue(endpoint.waitForClientDisconnect(timeout: 5), + "After release the re-armed auto-disconnect was expected to fire") + } + } + + func testReleaseWithDisconnectClosesTheConnection() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + operation.setConnection(leased) + let finished = self.start(operation) + XCTAssertEqual(finished.wait(timeout: .now() + 5), .success, + "The greeting endpoint was expected to let the connect finish") + + session.releaseConnection(leased, disconnect: true) + XCTAssertFalse(leased.isReserved) + XCTAssertTrue(endpoint.waitForClientDisconnect(timeout: 5), + "Release with disconnect was expected to close the socket") + } + } + + /// The release that matters is not the duplicated one, but the duplicated one that lands + /// AFTER somebody else has taken the same connection. Without a lease token the second + /// release passes every check — the connection is reserved, by this session — and clears the + /// new holder's reservation while queueing a disconnect under it. + func testStaleReleaseDoesNotCancelTheNextLease() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let first = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + session.releaseConnection(first, disconnect: false) + + guard let second = session.acquireConnection(folder: nil) else { + return XCTFail("The released connection must be available again") + } + XCTAssertTrue(second.isReserved) + + // The first holder's cleanup runs a second time — a defer after an explicit release, an + // error path after a normal one. + session.releaseConnection(first, disconnect: true) + + XCTAssertTrue(second.isReserved, "a stale release must not free the lease that replaced it") + XCTAssertNil(session.acquireConnection(folder: nil), + "the pool must still consider its only connection leased") + + // The refusal is the C++ primitive's, not the Swift handle's: the same stale release + // straight through the C bridge, with the generation the first lease reported. + session.session.releaseConnection(second.connection, first.leaseGeneration, true) + XCTAssertTrue(second.isReserved, "the session itself must refuse a release with an older lease generation") + + session.releaseConnection(second, disconnect: false) + } + + /// Nothing in the pool clears a reservation on its own, so a holder that loses track of its + /// lease would cost the pool that connection for the life of the session. The handle returns + /// the lease when it goes away. + func testDroppingTheHandleReturnsTheLease() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + do { + guard let leaked = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + XCTAssertTrue(leaked.isReserved) + XCTAssertNil(session.acquireConnection(folder: nil), "the only connection is leased") + } + + guard let reacquired = session.acquireConnection(folder: nil) else { + return XCTFail("A dropped handle must have returned its lease to the pool") + } + session.releaseConnection(reacquired, disconnect: false) + } + + func testReleaseIsIdempotent() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + operation.setConnection(leased) + let finished = self.start(operation) + XCTAssertEqual(finished.wait(timeout: .now() + 5), .success, + "The greeting endpoint was expected to let the connect finish") + + session.releaseConnection(leased, disconnect: false) + // The second release must be a no-op: its disconnect would otherwise land on a + // connection that is back in the shared pool and may serve other operations. + session.releaseConnection(leased, disconnect: true) + + XCTAssertFalse(endpoint.waitForClientDisconnect(timeout: 2), + "A repeated release must not tear down a pooled connection") + } + } + + /// The freshness question a leaseholder actually asks is "was this connection's view taken + /// before my signal", and a connection that has never logged in has no view at all: its + /// first command logs in and therefore sees the current state. Reporting nil here is what + /// lets a caller skip a teardown it used to pay for, having no way to tell a fresh pool + /// connection from a stale one. + /// + /// acquireConnection does no I/O, so this asserts on a connection that provably never + /// reached the wire. The other half - that a real LOGIN is recorded and a later one moves + /// the value forward - is not unit-testable here: this suite has no IMAP server, and a fake + /// answering just enough to get through LOGIN would encode mailcore's own post-login + /// sequence in a test. It is verified against the live server instead. + func testNeverConnectedConnectionReportsNoLoginTime() throws { + let endpoint = try LeaseTestTCPEndpoint() + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + defer { session.releaseConnection(leased, disconnect: false) } + + XCTAssertNil(leased.lastLoginDate, + "A pool connection that has never logged in must not claim a login moment") + } + + func testConnectionScopedDisconnectTearsTheSocketButKeepsTheLease() throws { + let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n") + defer { endpoint.stop() } + + let session = makeSession(port: endpoint.port, maximumConnections: 1) + + guard let leased = session.acquireConnection(folder: nil) else { + return XCTFail("An empty pool with room for 1 connection must satisfy the lease") + } + + runOffMainThread(timeout: 30) { + let operation = session.connectOperation() + operation.setConnection(leased) + let finished = self.start(operation) + XCTAssertEqual(finished.wait(timeout: .now() + 5), .success, + "The greeting endpoint was expected to let the connect finish") + + let disconnect = self.start(leased.disconnectOperation()) + XCTAssertEqual(disconnect.wait(timeout: .now() + 5), .success) + + XCTAssertTrue(endpoint.waitForClientDisconnect(timeout: 5), + "The connection-scoped disconnect was expected to close the socket") + XCTAssertTrue(leased.isReserved, + "Refreshing tears the socket, not the lease") + + session.releaseConnection(leased, disconnect: false) + } + } +} + +#endif diff --git a/unittest/LeaseTestTCPEndpoint.swift b/unittest/LeaseTestTCPEndpoint.swift new file mode 100644 index 000000000..5d6dda444 --- /dev/null +++ b/unittest/LeaseTestTCPEndpoint.swift @@ -0,0 +1,213 @@ +// +// LeaseTestTCPEndpoint.swift +// mailcore2 +// +// Shared test helper. +// + +// Darwin only, like the tests that use it: it needs a POSIX listening socket, and the Android job +// builds the test target without running it. +#if canImport(Darwin) + +import Darwin +import Dispatch +import Foundation + +/// A TCP endpoint that accepts connections and never answers a command. +/// +/// Without a greeting, a client sits in its first read until the socket timeout expires - so a +/// command sent here can only end by being interrupted, which makes both interruption and queue +/// occupancy observable in a test. With a greeting, an IMAP connect completes (embed the +/// capabilities in the banner so the client does not follow up with a CAPABILITY command) and the +/// connection then idles, which makes disconnect behavior observable: mailcore tears a connection +/// down by closing the socket, reported here through waitForClientDisconnect(). +final class LeaseTestTCPEndpoint { + + private let listeningSocket: Int32 + private let greeting: String? + private let answers: [String: String] + private let acceptQueue = DispatchQueue(label: "LeaseTestTCPEndpoint.accept") + private let lock = NSLock() + private var acceptedSockets: [Int32] = [] + private var openClientCount = 0 + private var everAcceptedClient = false + private var isClosed = false + + let port: UInt16 + + /// Pass an IMAP banner (e.g. "* OK [CAPABILITY IMAP4rev1] ready\r\n") to let connects finish; + /// pass nil to stay silent so that every command blocks. `answers` maps a command name + /// (LOGIN, LIST, ...) to the untagged lines to send before its tagged OK, so a test can walk + /// the client to a chosen state; any command not listed still blocks. + init(greeting: String? = nil, answers: [String: String] = [:]) throws { + self.greeting = greeting + self.answers = answers + + // Everything below works on a local descriptor: a closure that touched `listeningSocket` + // would capture self before `port` is initialized. + let fileDescriptor = socket(AF_INET, SOCK_STREAM, 0) + guard fileDescriptor >= 0 else { + throw NSError(domain: "LeaseTestTCPEndpoint", code: Int(errno), userInfo: nil) + } + + var reuse: Int32 = 1 + setsockopt(fileDescriptor, SOL_SOCKET, SO_REUSEADDR, &reuse, socklen_t(MemoryLayout.size)) + + var address = sockaddr_in() + address.sin_family = sa_family_t(AF_INET) + address.sin_port = 0 // any free port + address.sin_addr = in_addr(s_addr: inet_addr("127.0.0.1")) + + let bound = withUnsafePointer(to: &address) { pointer -> Int32 in + return pointer.withMemoryRebound(to: sockaddr.self, capacity: 1) { sockaddrPointer in + return bind(fileDescriptor, sockaddrPointer, socklen_t(MemoryLayout.size)) + } + } + + guard bound == 0, listen(fileDescriptor, 8) == 0 else { + close(fileDescriptor) + throw NSError(domain: "LeaseTestTCPEndpoint", code: Int(errno), userInfo: nil) + } + + var boundAddress = sockaddr_in() + var length = socklen_t(MemoryLayout.size) + let named = withUnsafeMutablePointer(to: &boundAddress) { pointer -> Int32 in + return pointer.withMemoryRebound(to: sockaddr.self, capacity: 1) { sockaddrPointer in + return getsockname(fileDescriptor, sockaddrPointer, &length) + } + } + + guard named == 0 else { + close(fileDescriptor) + throw NSError(domain: "LeaseTestTCPEndpoint", code: Int(errno), userInfo: nil) + } + + listeningSocket = fileDescriptor + port = UInt16(bigEndian: boundAddress.sin_port) + + acceptQueue.async { [weak self] in + self?.acceptConnections() + } + } + + private func acceptConnections() { + while true { + let accepted = accept(listeningSocket, nil, nil) + guard accepted >= 0 else { + return // the listening socket was closed + } + + lock.lock() + let closed = isClosed + if closed { + lock.unlock() + Darwin.close(accepted) + return + } + acceptedSockets.append(accepted) + openClientCount += 1 + everAcceptedClient = true + lock.unlock() + + if let greeting = greeting { + greeting.utf8CString.withUnsafeBufferPointer { buffer in + // -1 for the terminating NUL of a CString; loop out partial writes + var sent = 0 + let total = buffer.count - 1 + while sent < total { + let written = send(accepted, buffer.baseAddress! + sent, total - sent, 0) + guard written > 0 else { + return + } + sent += written + } + } + } + + // Incoming commands are read and discarded - never answered, except the ones listed in + // `answers` - so EOF, i.e. the client closing its socket, is the only other thing this + // loop reports. The reader owns the descriptor: closing it from stop() while recv() blocks + // on it would let the fd number be reused and the loop read somebody else's + // descriptor; stop() only shuts the socket down, which wakes recv(), and the close + // happens here. + let answers = self.answers + DispatchQueue.global().async { [weak self] in + var buffer = [UInt8](repeating: 0, count: 1024) + var pending = "" + while true { + let received = recv(accepted, &buffer, buffer.count, 0) + guard received > 0 else { + break + } + guard answers.isEmpty == false else { + continue + } + pending += String(decoding: buffer[0..= 2, let untagged = answers[words[1].uppercased()] else { + continue + } + let reply = Array((untagged + "\(words[0]) OK \(words[1]) completed\r\n").utf8) + _ = reply.withUnsafeBufferPointer { send(accepted, $0.baseAddress!, $0.count, 0) } + } + } + Darwin.close(accepted) + guard let self = self else { + return + } + self.lock.lock() + self.openClientCount -= 1 + self.lock.unlock() + } + } + } + + /// How many clients connected so far, closed ones included. + var acceptedClientCount: Int { + lock.lock() + defer { lock.unlock() } + return acceptedSockets.count + } + + /// Waits until at least one client was accepted and every accepted client has closed its + /// socket. Returns false when clients are still connected after the timeout. + func waitForClientDisconnect(timeout: TimeInterval) -> Bool { + let deadline = Date(timeIntervalSinceNow: timeout) + while Date() < deadline { + lock.lock() + let disconnected = everAcceptedClient && openClientCount == 0 + lock.unlock() + if disconnected { + return true + } + usleep(50_000) + } + return false + } + + func stop() { + lock.lock() + guard !isClosed else { + lock.unlock() + return + } + isClosed = true + let sockets = acceptedSockets + acceptedSockets = [] + lock.unlock() + + // shutdown() wakes the blocked accept()/recv() calls without invalidating the fd + // numbers under them; each descriptor is then closed by the thread that owns it + // (accepted sockets by their readers, the listening one right here after the wakeup). + shutdown(listeningSocket, SHUT_RDWR) + Darwin.close(listeningSocket) + for accepted in sockets { + shutdown(accepted, SHUT_RDWR) + } + } +} + +#endif