Ensure account synchroniser re-uses client sessions properly.

This ensures that when an account synchroniser account op completes and
its folder is closing completely, that it waits for this to complete
fully. Thus when the next op executes, it can re-use the same client
session.

This, along with recent other recent changes on the branch, gets the
total client session count on app start down to 2 simultaneous
connections only, instead of 3 or 4.

* src/engine/imap-engine/imap-engine-account-synchronizer.vala
  (RefreshFolderSync): If closing the folder causes it to be completely
  closed, wait for that to happen so the session is released to the pool
  before the next op runs.

* src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
  Make close_remote_session async so it can wait for the session to be
  fully released, including returning the session to the pool. Update
  call sites.

* src/engine/imap-engine/imap-engine-generic-account.vala
  (GenericAccount): Make release_folder_session async so that
  MinimalFolder can wait for it to be fully released, including returning
  the session from Selected state. Update call sites.
This commit is contained in:
Michael James Gratton 2018-02-27 02:05:58 +11:00
parent 9070380fc8
commit 3f0b8c98c4
5 changed files with 43 additions and 33 deletions

View file

@ -76,7 +76,7 @@ private class Geary.ImapEngine.GmailFolder : MinimalFolder, FolderSupport.Archiv
try {
yield imap_trash.remove_email_async(Imap.MessageSet.uid_sparse(uids), cancellable);
} finally {
account.release_folder_session(imap_trash);
yield account.release_folder_session(imap_trash);
}
debug("%s: Successfully true-removed %d/%d emails", folder.to_string(), uids.size,

View file

@ -103,17 +103,29 @@ private class Geary.ImapEngine.RefreshFolderSync : FolderOperation {
public override async void execute(Cancellable cancellable)
throws Error {
bool opened = false;
bool closed = false;
try {
yield this.folder.open_async(Folder.OpenFlags.NONE, cancellable);
opened = true;
yield this.folder.wait_for_remote_async(cancellable);
debug("Synchronising : %s", this.folder.to_string());
yield sync_folder(cancellable);
} finally {
if (opened) {
try {
// don't pass in the Cancellable; really need this
// to complete in all cases
yield this.folder.close_async();
closed = yield this.folder.close_async();
if (closed) {
// If the folder was actually closing, wait
// for it here to completely close so that its
// session has a chance to exit IMAP Selected
// state when released, allowing the next sync
// op to reuse the same session. Here we
// definitely want to use the cancellable so
// the wait can be interrupted.
yield this.folder.wait_for_close_async(cancellable);
}
} catch (Error err) {
debug(
"%s: Error closing folder %s: %s",
@ -216,7 +228,7 @@ private class Geary.ImapEngine.CheckFolderSync : RefreshFolderSync {
next_epoch = prefetch_max_epoch;
}
debug("%s *** syncing to: %s", this.account.to_string(), next_epoch.to_string());
debug("Synchronising %s to: %s", this.account.to_string(), next_epoch.to_string());
if (local_count < this.folder.properties.email_total &&
next_epoch.compare(prefetch_max_epoch) >= 0) {

View file

@ -394,23 +394,18 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
/**
* Returns an IMAP folder session to the pool for cleanup and re-use.
*/
public void release_folder_session(Imap.FolderSession session) {
public async void release_folder_session(Imap.FolderSession session) {
debug("%s: Releasing folder session", this.to_string());
Imap.ClientSession? old_session = session.close();
if (old_session != null) {
this.session_pool.release_session_async.begin(
old_session,
(obj, res) => {
try {
this.session_pool.release_session_async.end(res);
} catch (Error err) {
debug("%s: Error releasing %s session: %s",
to_string(),
session.folder.path.to_string(),
err.message);
}
}
);
try {
yield this.session_pool.release_session_async(old_session);
} catch (Error err) {
debug("%s: Error releasing %s session: %s",
to_string(),
session.folder.path.to_string(),
err.message);
}
}
}

View file

@ -717,7 +717,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
/**
* Unhooks the IMAP folder session and returns it to the account.
*/
internal void close_remote_session(Folder.CloseReason remote_reason) {
internal async void close_remote_session(Folder.CloseReason remote_reason) {
// Since the remote session has is/has gone away, we need to
// let waiters know. In the case of the folder being closed,
// notify that no more remotes will ever come back, otherwise
@ -737,14 +737,13 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Imap.FolderSession session = this.remote_session;
this.remote_session = null;
if (session != null) {
session.appended.disconnect(on_remote_appended);
session.updated.disconnect(on_remote_updated);
session.removed.disconnect(on_remote_removed);
session.disconnected.disconnect(on_remote_disconnected);
this._properties.remove(session.folder.properties);
this._account.release_folder_session(session);
yield this._account.release_folder_session(session);
notify_closed(remote_reason);
}
@ -828,7 +827,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
}
// Actually close the remote folder
close_remote_session(remote_reason);
yield close_remote_session(remote_reason);
// Since both the remote session and replay queue have shut
// down, we can reset the folder's internal state.
@ -931,7 +930,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// problem and should not try to use the folder further,
// unless the open was simply cancelled. So clean up, and
// force the folder closed.
this._account.release_folder_session(session);
yield this._account.release_folder_session(session);
if (!(err is IOError.CANCELLED)) {
Folder.CloseReason local_reason = CloseReason.LOCAL_ERROR;
Folder.CloseReason remote_reason = CloseReason.REMOTE_CLOSE;
@ -962,7 +961,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
} catch (Error err) {
// Database failed, which is also a pretty serious
// problem, so handle as per above.
this._account.release_folder_session(session);
yield this._account.release_folder_session(session);
if (!(err is IOError.CANCELLED)) {
notify_open_failed(Folder.OpenFailed.LOCAL_ERROR, err);
yield close_internal(
@ -1515,15 +1514,19 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Geary.Folder.CloseReason remote_reason = is_error
? Geary.Folder.CloseReason.REMOTE_ERROR
: Geary.Folder.CloseReason.REMOTE_CLOSE;
close_remote_session(remote_reason);
// If an error occurred, but the folder is still open and so
// is the pool, try re-establishing the connection.
if (is_error &&
this._account.session_pool.is_ready &&
!this.open_cancellable.is_cancelled()) {
this.open_remote_session.begin();
}
this.close_remote_session.begin(
remote_reason,
(obj, res) => {
this.close_remote_session.end(res);
// Once closed, if we are closing because an error
// occurred, but the folder is still open and so is
// the pool, try re-establishing the connection.
if (is_error &&
this._account.session_pool.is_ready &&
!this.open_cancellable.is_cancelled()) {
this.open_remote_session.begin();
}
});
}
}

View file

@ -44,7 +44,7 @@ private class Geary.ImapEngine.RevokableCommittedMove : Revokable {
this.account.update_folder(target);
} finally {
if (session != null) {
this.account.release_folder_session(session);
yield this.account.release_folder_session(session);
}
set_invalid();
}