From c152e07b3762169d9ce29ea4290c3cbd463fc016 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Thu, 7 May 2026 08:59:16 +0200 Subject: [PATCH 1/2] fix(imap): log number of connections during CLI sync AI-assisted: Claude Code (Claude Sonnet 4.6) Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/Command/SyncAccount.php | 10 +++++++++- lib/IMAP/HordeImapClient.php | 10 +++++++++- lib/IMAP/IMAPClientFactory.php | 14 +++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/Command/SyncAccount.php b/lib/Command/SyncAccount.php index 64f81fa27d..06471c70a7 100644 --- a/lib/Command/SyncAccount.php +++ b/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("Batch of new messages sync'ed. " . $mbs . 'MB of memory in use'); $this->sync($account, $force, $output); } + + foreach ($this->clientFactory->getLoginStats() as $host => $count) { + $consoleLogger->debug(sprintf("%d IMAP connection(s) to %s", $count, $host)); + } } } diff --git a/lib/IMAP/HordeImapClient.php b/lib/IMAP/HordeImapClient.php index 46051fcc66..a962fed53b 100644 --- a/lib/IMAP/HordeImapClient.php +++ b/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] diff --git a/lib/IMAP/IMAPClientFactory.php b/lib/IMAP/IMAPClientFactory.php index 3ec6bd42d6..2829bd54d6 100644 --- a/lib/IMAP/IMAPClientFactory.php +++ b/lib/IMAP/IMAPClientFactory.php @@ -27,6 +27,9 @@ use function json_encode; class IMAPClientFactory { + /** @var array */ + 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 */ + public function getLoginStats(): array { + return $this->loginCounts; + } } From 13147527b829913ad72f0f8193fc220d1e5e1548 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Thu, 7 May 2026 09:02:53 +0200 Subject: [PATCH 2/2] perf(imap): reuse client connection during initial sync AI-assisted: Claude Code (Claude Sonnet 4.6) Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/Service/Sync/ImapToDbSynchronizer.php | 89 +++++++++---------- .../Service/Sync/ImapToDbSynchronizerTest.php | 12 +-- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/lib/Service/Sync/ImapToDbSynchronizer.php b/lib/Service/Sync/ImapToDbSynchronizer.php index a48805dd41..8b5361cf2c 100644 --- a/lib/Service/Sync/ImapToDbSynchronizer.php +++ b/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); diff --git a/tests/Unit/Service/Sync/ImapToDbSynchronizerTest.php b/tests/Unit/Service/Sync/ImapToDbSynchronizerTest.php index 98c3d04692..cc11db3ba6 100644 --- a/tests/Unit/Service/Sync/ImapToDbSynchronizerTest.php +++ b/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' => [],