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;
+ }
}
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' => [],