perf(imap): reuse client connection during initial sync

lib/Command/SyncAccount.php

@@ -12,6 +12,7 @@
 use OCA\Mail\Account;
 use OCA\Mail\Exception\IncompleteSyncException;
 use OCA\Mail\Exception\ServiceException;
+use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\IMAP\MailboxSync;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\Sync\ImapToDbSynchronizer;
@@ -34,17 +35,20 @@ final class SyncAccount extends Command {
 	private MailboxSync $mailboxSync;
 	private ImapToDbSynchronizer $syncService;
 	private LoggerInterface $logger;
+	private IMAPClientFactory $clientFactory;
 
 	public function __construct(AccountService $service,
 		MailboxSync $mailboxSync,
 		ImapToDbSynchronizer $messageSync,
-		LoggerInterface $logger) {
+		LoggerInterface $logger,
+		IMAPClientFactory $clientFactory) {
 		parent::__construct();
 
 		$this->accountService = $service;
 		$this->mailboxSync = $mailboxSync;
 		$this->syncService = $messageSync;
 		$this->logger = $logger;
+		$this->clientFactory = $clientFactory;
 	}
 
 	/**
@@ -95,5 +99,9 @@ private function sync(Account $account, bool $force, OutputInterface $output): v
 			$output->writeln("<info>Batch of new messages sync'ed. " . $mbs . 'MB of memory in use</info>');
 			$this->sync($account, $force, $output);
 		}
+
+		foreach ($this->clientFactory->getLoginStats() as $host => $count) {
+			$consoleLogger->debug(sprintf("%d IMAP connection(s) to %s", $count, $host));
+		}
 	}
 }

lib/IMAP/HordeImapClient.php

@@ -28,6 +28,12 @@ class HordeImapClient extends Horde_Imap_Client_Socket {
 	private ?IMemcache $rateLimiterCache = null;
 	private ?ITimeFactory $timeFactory = null;
 	private ?string $hash = null;
+	private IMAPClientFactory $factory;
+
+	public function __construct(array $params, IMAPClientFactory $factory) {
+		parent::__construct($params);
+		$this->factory = $factory;
+	}
 
 	public function enableRateLimiter(
 		IMemcache $cache,
@@ -57,7 +63,9 @@ public function login() {
 	private const RATE_LIMIT_WINDOW = 3 * 60 * 60;
 
 	protected function imapLogin() {
-		return parent::_login();
+		$result = parent::_login();
+		$this->factory->recordLogin($this->_params['hostspec']);
+		return $result;
 	}
 
 	#[\Override]

lib/IMAP/IMAPClientFactory.php

@@ -27,6 +27,9 @@
 use function json_encode;
 
 class IMAPClientFactory {
+	/** @var array<string, int> */
+	private array $loginCounts = [];
+
 	/** @var ICrypto */
 	private $crypto;
 
@@ -134,7 +137,7 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
 			$params['debug'] = $this->config->getSystemValue('datadirectory') . '/' . $fn;
 		}
 
-		$client = new HordeImapClient($params);
+		$client = new HordeImapClient($params, $this);
 
 		$rateLimitingCache = $this->cacheFactory->createDistributed('mail_imap_ratelimit');
 		if ($rateLimitingCache instanceof IMemcache) {
@@ -143,4 +146,13 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
 
 		return $client;
 	}
+
+	public function recordLogin(string $host): void {
+		$this->loginCounts[$host] = ($this->loginCounts[$host] ?? 0) + 1;
+	}
+
+	/** @return array<string, int> */
+	public function getLoginStats(): array {
+		return $this->loginCounts;
+	}
 }

lib/Service/Sync/ImapToDbSynchronizer.php

@@ -335,57 +335,56 @@ private function runInitialSync(
 			$logger
 		);
 
-		// Need a client without a cache
-		$client->logout();
-		$client = $this->clientFactory->getClient($account, false);
-
-		$highestKnownUid = $this->dbMapper->findHighestUid($mailbox);
+		// Use a no-cache client for findAll. Horde accumulates cache state on
+		// every iteration of the findAll loop, causing a memory leak on large
+		// mailboxes (commit e50c214ff). Do NOT logout $client — the caller owns
+		// it and we need it below for getSyncToken.
+		$noCacheClient = $this->clientFactory->getClient($account, false);
 		try {
-			$imapMessages = $this->imapMapper->findAll(
-				$client,
-				$mailbox->getName(),
-				self::MAX_NEW_MESSAGES,
-				$highestKnownUid ?? 0,
-				$logger,
-				$perf,
-				$account->getUserId(),
-			);
-			$perf->step(sprintf('fetch %d messages from IMAP', count($imapMessages)));
-		} catch (Horde_Imap_Client_Exception $e) {
-			throw new ServiceException('Can not get messages from mailbox ' . $mailbox->getName() . ': ' . $e->getMessage(), 0, $e);
-		}
-
-		foreach (array_chunk($imapMessages['messages'], 500) as $chunk) {
-			$messages = array_map(static fn (IMAPMessage $imapMessage) => $imapMessage->toDbMessage($mailbox->getId(), $account->getMailAccount()), $chunk);
-			$this->dbMapper->insertBulk($account, ...$messages);
-			$perf->step(sprintf('persist %d messages in database', count($chunk)));
-			// Free the memory
-			unset($messages);
-		}
+			$highestKnownUid = $this->dbMapper->findHighestUid($mailbox);
+			try {
+				$imapMessages = $this->imapMapper->findAll(
+					$noCacheClient,
+					$mailbox->getName(),
+					self::MAX_NEW_MESSAGES,
+					$highestKnownUid ?? 0,
+					$logger,
+					$perf,
+					$account->getUserId(),
+				);
+				$perf->step(sprintf('fetch %d messages from IMAP', count($imapMessages)));
+			} catch (Horde_Imap_Client_Exception $e) {
+				throw new ServiceException('Can not get messages from mailbox ' . $mailbox->getName() . ': ' . $e->getMessage(), 0, $e);
+			}
 
-		if (!$imapMessages['all']) {
-			// We might need more attempts to fill the cache
-			$loggingMailboxId = $account->getId() . ':' . $mailbox->getName();
-			$total = $imapMessages['total'];
-			$cached = count($this->dbMapper->findAllUids($mailbox));
-			$perf->step('find number of cached UIDs');
+			foreach (array_chunk($imapMessages['messages'], 500) as $chunk) {
+				$messages = array_map(static fn (IMAPMessage $imapMessage) => $imapMessage->toDbMessage($mailbox->getId(), $account->getMailAccount()), $chunk);
+				$this->dbMapper->insertBulk($account, ...$messages);
+				$perf->step(sprintf('persist %d messages in database', count($chunk)));
+				// Free the memory
+				unset($messages);
+			}
 
-			$perf->end();
-			throw new IncompleteSyncException("Initial sync is not complete for $loggingMailboxId ($cached of $total messages cached).");
-		}
+			if (!$imapMessages['all']) {
+				// We might need more attempts to fill the cache
+				$loggingMailboxId = $account->getId() . ':' . $mailbox->getName();
+				$total = $imapMessages['total'];
+				$cached = count($this->dbMapper->findAllUids($mailbox));
+				$perf->step('find number of cached UIDs');
 
-		// Get the sync token from a cache-enabled client. The non-cache client
-		// used above does not activate CONDSTORE/QRESYNC, so its sync token
-		// lacks HIGHESTMODSEQ. Without it, the first partial sync falls into
-		// Horde's "no MODSEQ" fallback that resolves ALL UIDs, causing OOM on
-		// large mailboxes.
-		$client->logout();
-		$cacheClient = $this->clientFactory->getClient($account);
-		try {
-			$syncToken = $cacheClient->getSyncToken($mailbox->getName());
+				$perf->end();
+				throw new IncompleteSyncException("Initial sync is not complete for $loggingMailboxId ($cached of $total messages cached).");
+			}
 		} finally {
-			$cacheClient->logout();
+			$noCacheClient->logout();
 		}
+
+		// Use the cache-enabled $client passed in for the sync token. The no-cache
+		// client does not activate CONDSTORE/QRESYNC (commit 7980d9e40), so its
+		// token lacks HIGHESTMODSEQ — causing the first partial sync to resolve ALL
+		// UIDs and run OOM on large mailboxes. Do NOT logout $client here; the
+		// caller (syncAccount) owns and closes it.
+		$syncToken = $client->getSyncToken($mailbox->getName());
 		$mailbox->setSyncNewToken($syncToken);
 		$mailbox->setSyncChangedToken($syncToken);
 		$mailbox->setSyncVanishedToken($syncToken);

tests/Unit/Service/Sync/ImapToDbSynchronizerTest.php

@@ -81,18 +81,14 @@ public function testInitialSyncGetsSyncTokenFromCacheClient(): void {
 		$initialClient = $this->createMock(Horde_Imap_Client_Socket::class);
 		$initialClient->method('__get')->with('capability')->willReturn($capability);
 		$noCacheClient = $this->createStub(Horde_Imap_Client_Socket::class);
-		$cacheClient = $this->createMock(Horde_Imap_Client_Socket::class);
-		$cacheClient->expects($this->once())
+		$initialClient->expects($this->once())
 			->method('getSyncToken')
 			->with('INBOX')
 			->willReturn('dG9rZW5XaXRoSA==');
-		$cacheClient->expects($this->once())->method('logout');
-		$this->clientFactory->expects($this->exactly(2))
+		$this->clientFactory->expects($this->once())
 			->method('getClient')
-			->willReturnMap([
-				[$account, false, $noCacheClient],
-				[$account, true, $cacheClient],
-			]);
+			->with($account, false)
+			->willReturn($noCacheClient);
 		$this->dbMapper->method('findHighestUid')->willReturn(null);
 		$this->imapMapper->method('findAll')->willReturn([
 			'messages' => [],