Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle IMAP disconnects more explicitly #6174

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Apr 4, 2022

This changes the IMAP client usages to be used as a resource that is
freed after finished use. Previously we just memoized connections to the
same account to lower the number of connections, but that has still
shown to cause too many open connections during tests but possibly also
in production.

  • Move IMAP client usage into try-(catch-)finally patterns
  • Drop the IMAP client factory memoization
  • Open follow-up ticket to diagnose if we have potential reconnects because of this change. If we previously used the memoized connection in two sections of a code there is now a little overhead. It might not be worth the optimization but should be checked nevertheless. IMAP connection reuse #6175

@miaulalala
Copy link
Contributor

mail/lib/Account.php

Lines 185 to 190 in 689ef95

public function getMailbox($folderId) {
return new Mailbox(
$this->getImapConnection(),
new Horde_Imap_Client_Mailbox($folderId)
);
}

Is still using the deprecated getImapConnection method.

Refctor?

@ChristophWurst
Copy link
Member Author

\OCA\Mail\Account::getMailbox is deprecated itself. I wouldn't refactor this anymore but try to get rid of those usages.

This changes the IMAP client usages to be used as a *resource* that is
freed after finished use. Previously we just memoized connections to the
same account to lower the number of connections, but that has still
shown to cause too many open connections during tests but possibly also
in production.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the refactor/imap-client-clean-logout-resource branch from 689ef95 to a23e823 Compare April 4, 2022 09:07
@miaulalala miaulalala merged commit b391116 into main Apr 4, 2022
@miaulalala miaulalala deleted the refactor/imap-client-clean-logout-resource branch April 4, 2022 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants