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

[dbezverkhnii]

Linear: COR-201. Consumed by readdle/spark-core-mono#8026.

What

Connection lease. IMAPAsyncSession::acquireConnection(folder) reserves one connection of the pool for exclusive use: 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(connection, disconnect) 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. Returns NULL when every connection is reserved; in that state, and only then, ordinary selection falls back to sharing the least busy reserved connection (documented on the API, with the threading contract: serialize acquire/release with start()).

Last login time. IMAPSession::lastLoginTime() — 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, without tracking disconnects it cannot observe.

Swift / C bridge. MCOIMAPAsyncConnection handle (a dropped handle returns its lease from deinit, best effort), MCOIMAPSession.acquireConnection(folder:) / releaseConnection(_:disconnect:), MCOIMAPBaseOperation.setConnection(_:), lastLoginDate, identity.

Behaviour change for existing callers — one, deliberate

interruptCurrentCommand() now sets mShouldDisconnect. libetpan never clears a stream's cancelled state, so a connection whose command was cut had to reconnect before its next command anyway; most command paths noticed the stream error and scheduled that, but NOOP and the callers of resultsWithError did not, and a connection cut there stayed "connected" and failed its next command once. Now every interrupted connection reconnects on its next command, whichever command was cut. Covered by a test that fails on the old code.

Everything else is additive: a client that does not call the new API sees the same selection, the same idle timer (30 s default unchanged) and the same headers.

Test-only knobs

The idle disconnect delay is configurable (setAutomaticDisconnectDelay, C++ and C layer) so the stand-down while reserved can be observed in a test; operationsCount / isReserved on the Swift handle are internal. Kept deliberately — there is no cheaper way to make the timer observable.

Verification

• swift test --filter IMAPConnectionLeaseTests: 15 tests against a local TCP endpoint that greets like an IMAP server, answers the commands a test lists and blocks on the rest (Darwin only; the Android job builds the test target without running it).
• Each commit builds on its own.
• src/include/MailCore/* copies are byte-identical to their sources; upstream declarations and doc blocks untouched (the sessionWithMinQueue variant is an overload, not a default argument).
• Windows: MCIMAPSession.cpp includes MCWin32.h first, which maps gettimeofday; the sys/time.h guard mirrors MCLog.cpp. Not built on Windows here — the prebuilt is the check.
• Reviewed zero-context: no blockers; the doc findings are folded in.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core IMAP pool selection, connection lifecycle, and interrupt/reconnect behavior; callers must serialize lease acquire/release with operation start() to avoid races, and must size maximumConnections above concurrent leases to keep exclusivity.

Overview
Adds an IMAP connection lease so callers can hold one pooled connection exclusively: acquireConnection / releaseConnection (C++, C, Swift), with lease generation so stale or duplicate releases cannot clear someone else's reservation. While leased, normal operation routing skips that connection; work runs there only via setConnection / setSession. Idle auto-disconnect is suppressed for the lease, uses a configurable delay (default 30s unchanged), and is re-armed on release; the idle timer is synchronized with reservation state so it cannot disconnect under an active holder.

Pool routing now ignores reserved connections (and avoids rewriting folder affinity on a shared fallback). When the pool is at maximumConnections and every connection is leased, unpinned operations still get the least-busy reserved connection—a documented exclusivity limit.

Also exposes lastLoginTime / lastLoginDate for judging whether a connection's mailbox view predates client events, plus a new CIMAPAsyncConnection wrapper and MCOIMAPAsyncConnection handle (including best-effort lease return from deinit).

Existing callers: interruptCurrentCommand() now sets mShouldDisconnect so the next command reconnects after libetpan cancels the stream (fixes NOOP and similar paths that previously stayed "connected" and failed). interruptCurrentCommand return-value docs are clarified.

Tests: 15 Darwin-only lease tests with a local LeaseTestTCPEndpoint fake IMAP server.

^{Reviewed by Cursor Bugbot for commit 7cb5b4c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[dbezverkhnii]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[Copilot]

🟡 Changes recommended

Critical lease-timer and packaging/Xcode integration issues, along with stale lease-token validation, remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds exclusive pooled IMAP connection leasing, operation pinning, login timestamps, Swift/C APIs, and reconnect handling.

Changes:

• Implements reservation-aware selection and idle-disconnect control.
• Exposes lease and connection metadata through Swift and C.
• Adds local TCP lease and reconnect tests.

File summaries

File	Summary
unittest/LeaseTestTCPEndpoint.swift	Local TCP IMAP test endpoint.
unittest/IMAPConnectionLeaseTests.swift	Tests leasing, pinning, timer behavior, and reconnects.
src/swift/imap/IMAPSession.swift	Swift lease APIs; critical Xcode source registration is missing.
src/swift/imap/IMAPBaseOperation.swift	Swift operation connection pinning.
src/swift/imap/IMAPAsyncConnection.swift	Swift connection lease handle.
src/include/MailCore/MCIMAPSession.h	Public login timestamp API and interrupt documentation.
src/include/MailCore/MCIMAPOperation.h	Public operation documentation.
src/include/MailCore/MCIMAPAsyncSession.h	Public native lease declarations.
src/include/MailCore/MCIMAPAsyncConnection.h	Public connection metadata declarations.
src/include/MailCore/CIMAPBaseOperation.h	Public C pinning API.
src/include/MailCore/CIMAPAsyncSession.h	Public C lease API.
src/include/MailCore/CIMAPAsyncConnection.h	Public C connection handle.
src/include/MailCore/CCore.h	Public C aggregate header wiring.
src/core/imap/MCIMAPSession.h	Session timestamp and interrupt declarations; lifecycle documentation needs updating.
src/core/imap/MCIMAPSession.cpp	Login timestamp and reconnect behavior; successful-login coverage is missing.
src/cmake/public-headers.cmake	Public C++ header registration.
src/cmake/cmailcore-public-headers.cmake	Public C header registration.
src/c/imap/CIMAPBaseOperation.h	C pinning declaration.
src/c/imap/CIMAPBaseOperation.cpp	C pinning implementation.
src/c/imap/CIMAPAsyncSession.h	C lease declarations.
src/c/imap/CIMAPAsyncSession.cpp	C bridge lacks native lease-generation validation.
src/c/imap/CIMAPAsyncConnection.h	C connection declarations.
src/c/imap/CIMAPAsyncConnection.cpp	Critical legacy Xcode project entries are missing.
src/c/CCore.h	C aggregate header wiring.
src/async/imap/MCIMAPOperation.h	Native operation documentation.
src/async/imap/MCIMAPAsyncSession.h	Native session declarations; critical Windows header packaging entry is missing.
src/async/imap/MCIMAPAsyncSession.cpp	Lease selection and release; critical timer race and stale-generation validation remain.
src/async/imap/MCIMAPAsyncConnection.h	Reservation state declarations.
src/async/imap/MCIMAPAsyncConnection.cpp	Reservation and idle-disconnect implementation.
Package.swift	Registers Swift sources.
configure-headers.sh	Updates generated header wiring.

Review details

Suppressed comments (3)

src/c/imap/CIMAPAsyncSession.cpp:109

• This C bridge passes only the underlying pointer to the native release method, so a copied stale CIMAPAsyncConnection handle has no lease generation attached. After the same pooled object is reacquired, releasing the old C handle can therefore clear the new holder's reservation and disconnect its socket; carry a lease token/generation through this API instead of delegating the raw pointer alone.

void CIMAPAsyncSession_releaseConnection(struct CIMAPAsyncSession self, CIMAPAsyncConnection connection, bool disconnect) {
    self.instance->releaseConnection(connection.instance, disconnect);
}

src/core/imap/MCIMAPSession.cpp:3747

• The intentional reconnect-on-next-command behavior makes the public IMAPSession::interruptCurrentCommand documentation stale: both MCIMAPSession.h copies still say the caller must explicitly disconnect and that a later connect() builds the stream. Please update those public comments so callers do not follow the old lifecycle contract.

        // 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;

src/core/imap/MCIMAPSession.cpp:1041

• The new timestamp is only asserted for the never-logged-in case. No repository test verifies that a successful LOGIN sets mLastLoginTime (or that a later LOGIN advances it), so a regression in this assignment or the success path would pass the suite even though this is the freshness signal exposed to callers. The local endpoint already answers LOGIN/CAPABILITY/LIST and can drive this case.

    LOCK();
    mLastLoginTime = currentWallClockTime();
    UNLOCK();

• Files reviewed: 31/31 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cursor]

Stale Bugbot comment from a previous run.

[dbezverkhnii]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[dbezverkhnii]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 7cb5b4c. Configure here.}