[geary/wip/789924-network-transition-redux: 61/64] Ensure account synchroniser re-uses client sessions properly.

[Michael Gratton <mjog src gnome org>]

commit 3f0b8c98c43933eeac1770fa49c4b21fd2b4bc21
Author: Michael James Gratton <mike vee net>
Date:   Tue Feb 27 02:05:58 2018 +1100

    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.

 .../gmail/imap-engine-gmail-folder.vala            |    2 +-
 .../imap-engine-account-synchronizer.vala          |   16 ++++++++-
 .../imap-engine/imap-engine-generic-account.vala   |   23 +++++--------
 .../imap-engine/imap-engine-minimal-folder.vala    |   33 +++++++++++---------
 .../imap-engine-revokable-committed-move.vala      |    2 +-
 5 files changed, 43 insertions(+), 33 deletions(-)
---
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala 
b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
index c4ab545..0469d01 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
@@ -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,
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala 
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index 18419f3..4230c67 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -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) {
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 48103ea..7257deb 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -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);
+            }
         }
     }
 
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index e7c2ced..ec5a371 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -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();
+                }
+            });
     }
 
 }
diff --git a/src/engine/imap-engine/imap-engine-revokable-committed-move.vala 
b/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
index a7ab922..e5a9c55 100644
--- a/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
+++ b/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
@@ -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();
         }