[geary/wip/789924-network-transition-redux: 59/64] Remove GenericAccount's IMAP account session attribute.



commit 9cedfaf0d829f9b80d841a48e4e0b54dec362d37
Author: Michael James Gratton <mike vee net>
Date:   Tue Feb 27 01:03:21 2018 +1100

    Remove GenericAccount's IMAP account session attribute.
    
    For most of the time, GenericAccount does not need an open client
    session, yet it always claims one from the pool on open and hangs on to
    it. This means that the account synchroniser for example cannot use the
    same session to perform sync, requiring an additional session to be
    opened.
    
    This patch removes the held session with methods to claim and release
    account sessions instances for those code paths that need it, allowing
    the previously claimed session to be re-used when not in use.
    
    * src/engine/imap-engine/imap-engine-generic-account.vala
      (GenericAccount): Remove remote_open_lock and remote_session, just
      claim new client sessions from the pool as needed instead. Add
      release_account_session to allow claimed sessions to be returned to the
      pool. Update uses of remote_session to claim and release instead.
    
    * src/engine/imap/transport/imap-client-session-manager.vala
      (ClientSessionManager): Update ready signal to include a parameter
      indicating if ready or not, so GenericAccount can still block
      claim_account_session callers when not ready. Rework all code that sets
      is_ready to use a common notify method that both does that and fires
      the signal, and rework all code paths that stop the pool to use a
      common code path that uniformly resets timers, notifies no longer
      ready, and drops all client sessions.
    
    * src/engine/imap/api/imap-account-session.vala: Remove distinction
      between fetch_folder_async and fetch_folder_cached_async
      since account session objects are now short-lived.

 .../imap-engine/imap-engine-generic-account.vala   |  242 +++++++++-----------
 .../imap-engine/imap-engine-minimal-folder.vala    |    6 +-
 src/engine/imap/api/imap-account-session.vala      |   80 +++----
 .../transport/imap-client-session-manager.vala     |  122 ++++++-----
 4 files changed, 203 insertions(+), 247 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index aefed4b..86acee9 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -35,13 +35,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
     private bool open = false;
     private Cancellable? open_cancellable = null;
+    private Nonblocking.Semaphore? remote_ready_lock = null;
 
     private Geary.SearchFolder? search_folder { get; private set; default = null; }
 
-    private Nonblocking.Mutex remote_open_lock = new Nonblocking.Mutex();
-    private Nonblocking.Semaphore? remote_ready_lock = null;
-    private Imap.AccountSession? remote_session { get; private set; default = null; }
-
     private Gee.HashMap<FolderPath, MinimalFolder> folder_map = new Gee.HashMap<
         FolderPath, MinimalFolder>();
     private Gee.HashMap<FolderPath, Folder> local_only = new Gee.HashMap<FolderPath, Folder>();
@@ -167,11 +164,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         this.queue_operation(
             new LoadFolders(this, this.local, get_supported_special_folders())
         );
-
-        // If the pool is already ready, let's go get a session!
-        if (this.session_pool.is_ready) {
-            this.open_remote_session.begin(cancellable);
-        }
     }
 
     public override async void close_async(Cancellable? cancellable = null) throws Error {
@@ -179,6 +171,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
             return;
 
         // Stop trying to re-use IMAP server connections
+        this.remote_ready_lock.reset();
         this.session_pool.discard_returned_sessions = true;
 
         // Halt internal tasks early so they stop using local and
@@ -209,8 +202,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
         // Close remote infrastructure
 
-        yield close_remote_session(cancellable);
-        this.remote_ready_lock = null;
         try {
             yield this.session_pool.close_async(cancellable);
         } catch (Error err) {
@@ -219,6 +210,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
                   this.session_pool.to_string()
             );
         }
+        this.remote_ready_lock = null;
 
         // Close local infrastructure
 
@@ -304,15 +296,13 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     /**
-     * Returns a valid IMAP account session when one is available.
-     *
-     * Implementations may use this to acquire an IMAP session for
-     * performing account-related work. The call will wait until a
-     * connection is established then return the session.
+     * Claims a new IMAP account session from the pool.
      *
-     * The session returned is guaranteed to be open upon return,
-     * however may close afterwards due to this account closing, or
-     * the network connection going away.
+     * A new IMAP client session will be retrieved from the pool,
+     * connecting if needed, and used for a new account session. This
+     * call will wait until the pool is ready to provide sessions. The
+     * session must be returned via {@link release_account_session}
+     * after use.
      *
      * The account must have been opened before calling this method.
      */
@@ -321,7 +311,31 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         check_open();
         debug("%s: Acquiring account session", this.to_string());
         yield this.remote_ready_lock.wait_async(cancellable);
-        return this.remote_session;
+        Imap.ClientSession client =
+            yield this.session_pool.claim_authorized_session_async(cancellable);
+        return new Imap.AccountSession(this.information.id, client);
+    }
+
+    /**
+     * Returns an IMAP account session to the pool for re-use.
+     */
+    public void release_account_session(Imap.AccountSession session) {
+        debug("%s: Releasing account 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 account session: %s",
+                              to_string(),
+                              err.message);
+                    }
+                }
+            );
+        }
     }
 
     /**
@@ -444,7 +458,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
         check_open();
 
-        return yield ensure_special_folder_async(special, cancellable);
+        Geary.Folder? folder = get_special_folder(special);
+        if (folder == null) {
+            Imap.AccountSession account = yield claim_account_session();
+            try {
+                folder = yield ensure_special_folder_async(account, special, cancellable);
+            } finally {
+                release_account_session(account);
+            }
+        }
+        return folder;
     }
 
     public override async void send_email_async(Geary.ComposedEmail composed,
@@ -638,16 +661,12 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     /**
-     * Returns the folder for the given special type, creating it if needed.
+     * Locates a special folder, creating it if needed.
      */
-    internal async Geary.Folder ensure_special_folder_async(Geary.SpecialFolderType special,
+    internal async Geary.Folder ensure_special_folder_async(Imap.AccountSession remote,
+                                                            Geary.SpecialFolderType special,
                                                             Cancellable? cancellable)
         throws Error {
-        Geary.Folder? folder = get_special_folder(special);
-        if (folder != null)
-            return folder;
-
-        Imap.AccountSession account = yield claim_account_session();
         MinimalFolder? minimal_folder = null;
         Geary.FolderPath? path = information.get_special_folder_path(special);
         if (path != null) {
@@ -656,7 +675,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
             // This is the first time we're turning a non-special folder into a special one.
             // After we do this, we'll record which one we picked in the account info.
             Geary.FolderPath root =
-                yield account.get_default_personal_namespace(cancellable);
+                yield remote.get_default_personal_namespace(cancellable);
             Gee.List<string> search_names = special_search_names.get(special);
             foreach (string search_name in search_names) {
                 Geary.FolderPath search_path = root.get_child(search_name);
@@ -683,7 +702,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         } else {
             debug("Creating %s to use as special folder %s", path.to_string(), special.to_string());
             // TODO: ignore error due to already existing.
-            yield account.create_folder_async(path, special, cancellable);
+            yield remote.create_folder_async(path, special, cancellable);
             minimal_folder = (MinimalFolder) yield fetch_folder_async(path, cancellable);
         }
 
@@ -773,76 +792,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     /**
-     * Establishes a new account session with the IMAP server.
-     */
-    private async void open_remote_session(Cancellable cancellable) {
-        try {
-            int token = yield this.remote_open_lock.claim_async(cancellable);
-            if (this.remote_session != null) {
-                return;
-            }
-
-            try {
-                check_open();
-                debug("%s: Opening remote session", to_string());
-                Imap.ClientSession client =
-                    yield this.session_pool.claim_authorized_session_async(
-                        cancellable
-                    );
-
-                this.remote_session = new Imap.AccountSession(
-                    this.information.id, client
-                );
-                this.remote_session.disconnected.connect(on_remote_disconnect);
-
-                this.remote_ready_lock.notify();
-            } catch (Error err) {
-                notify_imap_problem(ProblemType.CONNECTION_ERROR, err);
-            }
-
-            this.remote_open_lock.release(ref token);
-
-            // Now we have a valid remote session again, update our idea
-            // of what the remote folders are in case they have changed
-            update_remote_folders();
-        } catch (Error err) {
-            // Oh well
-        }
-    }
-
-    /**
-     * Drops the current account session, if any.
-     */
-    private async void close_remote_session(Cancellable cancellable) {
-        try {
-            int token = yield this.remote_open_lock.claim_async(cancellable);
-            if (this.remote_session == null) {
-                return;
-            }
-
-            try {
-                this.remote_ready_lock.reset();
-
-                Imap.ClientSession? old_session = this.remote_session.close();
-
-                this.remote_session.disconnected.connect(on_remote_disconnect);
-                this.remote_session = null;
-
-                if (old_session != null) {
-                    yield this.session_pool.release_session_async(old_session);
-                }
-            } catch (Error err) {
-                debug("%s: Error closing remote session: %s",
-                      this.to_string(), err.message);
-            }
-
-            this.remote_open_lock.release(ref token);
-        } catch (Error err) {
-            // Oh well
-        }
-    }
-
-    /**
      * Hooks up and queues an {@link UpdateRemoteFolders} operation.
      */
     private void update_remote_folders() {
@@ -995,13 +944,20 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
     }
 
-    private void on_pool_session_ready() {
-        // Now have a valid session, so credentials must be good
-        this.authentication_failures = 0;
-        this.open_remote_session.begin(this.open_cancellable);
+    private void on_pool_session_ready(bool is_ready) {
+        if (is_ready) {
+            // Now have a valid session, so credentials must be good
+            this.authentication_failures = 0;
+            this.remote_ready_lock.blind_notify();
+            update_remote_folders();
+        } else {
+            this.remote_ready_lock.reset();
+            this.refresh_folder_timer.reset();
+        }
     }
 
     private void on_pool_connection_failed(Error error) {
+        this.remote_ready_lock.reset();
         if (error is ImapError.UNAUTHENTICATED) {
             // This is effectively a login failure
             on_pool_login_failed(null);
@@ -1011,6 +967,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     private void on_pool_login_failed(Geary.Imap.StatusResponse? response) {
+        this.remote_ready_lock.reset();
         this.authentication_failures++;
         if (this.authentication_failures >= Geary.Account.AUTH_ATTEMPTS_MAX) {
             // We have tried auth too many times, so bail out
@@ -1063,10 +1020,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
     }
 
-    private void on_remote_disconnect(Imap.ClientSession.DisconnectReason reason) {
-        this.close_remote_session.begin(this.open_cancellable);
-    }
-
 }
 
 
@@ -1171,23 +1124,29 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
     }
 
     public override async void execute(Cancellable cancellable) throws Error {
-        Imap.AccountSession remote =
-            yield ((GenericAccount) this.account).claim_account_session(cancellable);
-
         Gee.Map<FolderPath, Geary.Folder> existing_folders =
             Geary.traverse<Geary.Folder>(this.account.list_folders())
             .to_hash_map<FolderPath>(f => f.path);
         Gee.Map<FolderPath, Imap.Folder> remote_folders =
             new Gee.HashMap<FolderPath, Imap.Folder>();
 
-        bool is_suspect = yield enumerate_remote_folders_async(
-            remote, remote_folders, null, cancellable
+        GenericAccount account = (GenericAccount) this.account;
+        Imap.AccountSession remote = yield account.claim_account_session(
+            cancellable
         );
+        try {
+            bool is_suspect = yield enumerate_remote_folders_async(
+                remote, remote_folders, null, cancellable
+            );
 
-        // pair the local and remote folders and make sure everything is up-to-date
-        yield update_folders_async(
-            remote, existing_folders, remote_folders, is_suspect, cancellable
-        );
+            // pair the local and remote folders and make sure
+            // everything is up-to-date
+            yield update_folders_async(
+                remote, existing_folders, remote_folders, is_suspect, cancellable
+            );
+        } finally {
+            account.release_account_session(remote);
+        }
     }
 
     private async bool enumerate_remote_folders_async(Imap.AccountSession remote,
@@ -1343,9 +1302,11 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
         // Ensure each of the important special folders we need already exist
         foreach (Geary.SpecialFolderType special in this.specials) {
             try {
-                yield this.generic_account.ensure_special_folder_async(
-                    special, cancellable
-                );
+                if (this.generic_account.get_special_folder(special) == null) {
+                    yield this.generic_account.ensure_special_folder_async(
+                        remote, special, cancellable
+                    );
+                }
             } catch (Error e) {
                 warning("Unable to ensure special folder %s: %s", special.to_string(), e.message);
             }
@@ -1369,31 +1330,36 @@ internal class Geary.ImapEngine.RefreshFolderUnseen : FolderOperation {
     }
 
     public override async void execute(Cancellable cancellable) throws Error {
-        Imap.AccountSession remote =
-            yield ((GenericAccount) this.account).claim_account_session(cancellable);
-
+        GenericAccount account = (GenericAccount) this.account;
         if (this.folder.get_open_state() == Geary.Folder.OpenState.CLOSED) {
-            Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
-                folder.path,
-                true,
+            Imap.AccountSession? remote = yield account.claim_account_session(
                 cancellable
             );
+            try {
+                Imap.Folder remote_folder = yield remote.fetch_folder_async(
+                    folder.path,
+                    cancellable
+                );
 
-            // Although this is called when the folder is closed, we
-            // can safely use local_folder since we are only using its
-            // properties, and the properties were loaded when the
-            // folder was first instantiated.
-            ImapDB.Folder local_folder = ((MinimalFolder) this.folder).local_folder;
+                // Implementation-specific hack: Although this is called
+                // when the MinimalFolder is closed, we can safely use
+                // local_folder since we are only using its properties,
+                // and the properties were loaded when the folder was
+                // first instantiated.
+                ImapDB.Folder local_folder = ((MinimalFolder) this.folder).local_folder;
 
-            if (remote_folder.properties.have_contents_changed(
-                    local_folder.get_properties(),
-                    this.folder.to_string())) {
+                if (remote_folder.properties.have_contents_changed(
+                        local_folder.get_properties(),
+                        this.folder.to_string())) {
 
-                yield local_folder.update_folder_status(
-                    remote_folder.properties, false, true, cancellable
-                );
+                    yield local_folder.update_folder_status(
+                        remote_folder.properties, false, true, cancellable
+                    );
 
-                ((GenericAccount) this.account).update_folder(this.folder);
+                    ((GenericAccount) this.account).update_folder(this.folder);
+                }
+            } finally {
+                account.release_account_session(remote);
             }
         }
     }
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 23f87b2..66cd17f 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1503,8 +1503,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         );
     }
 
-    private void on_remote_ready() {
-        this.open_remote_session.begin();
+    private void on_remote_ready(bool is_ready) {
+        if (is_ready) {
+            this.open_remote_session.begin();
+        }
     }
 
     private void on_remote_disconnected(Imap.ClientSession.DisconnectReason reason) {
diff --git a/src/engine/imap/api/imap-account-session.vala b/src/engine/imap/api/imap-account-session.vala
index 50cbf9a..e5e633c 100644
--- a/src/engine/imap/api/imap-account-session.vala
+++ b/src/engine/imap/api/imap-account-session.vala
@@ -107,67 +107,45 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
     }
 
     /**
-     * Returns a single folder from the server.
-     *
-     * The folder is not cached by the account and hence will not be
-     * used my multiple callers or containers.  This is useful for
-     * one-shot operations on the server.
-     */
-    public async Imap.Folder fetch_folder_async(FolderPath path, Cancellable? cancellable)
-        throws Error {
-        ClientSession session = claim_session();
-
-        Gee.List<MailboxInformation>? mailboxes = yield send_list_async(
-            session, path, false, cancellable
-        );
-        if (mailboxes.is_empty)
-            throw_not_found(path);
-
-        MailboxInformation mailbox_info = mailboxes.get(0);
-        Imap.FolderProperties? props = null;
-        if (!mailbox_info.attrs.is_no_select) {
-            StatusData status = yield send_status_async(
-                session,
-                mailbox_info.mailbox,
-                StatusDataType.all(),
-                cancellable
-            );
-            props = new Imap.FolderProperties.selectable(
-                mailbox_info.attrs,
-                status,
-                session.capabilities
-            );
-        } else {
-            props = new Imap.FolderProperties.not_selectable(mailbox_info.attrs);
-        }
-
-        return new Imap.Folder(path, props);
-    }
-
-    /**
      * Returns a single folder, from the account's cache or fetched fresh.
      *
      * If the folder has previously been retrieved, that is returned
      * instead of fetching it again. If not, it is fetched from the
      * server and cached for future use.
      */
-    public async Imap.Folder fetch_folder_cached_async(FolderPath path,
-                                                       bool refresh_status,
-                                                       Cancellable? cancellable)
+    public async Imap.Folder fetch_folder_async(FolderPath path,
+                                                Cancellable? cancellable)
         throws Error {
         ClientSession session = claim_session();
         Imap.Folder? folder = this.folders.get(path);
         if (folder == null) {
-            folder = yield fetch_folder_async(path, cancellable);
-            this.folders.set(path, folder);
-        } else if (refresh_status && !folder.properties.attrs.is_no_select) {
-            StatusData status = yield send_status_async(
-                session,
-                session.get_mailbox_for_path(path),
-                { StatusDataType.UNSEEN, StatusDataType.MESSAGES },
-                cancellable
+            Gee.List<MailboxInformation>? mailboxes = yield send_list_async(
+                session, path, false, cancellable
             );
-            folder.properties.update_status(status);
+            if (mailboxes.is_empty) {
+                throw_not_found(path);
+            }
+
+            MailboxInformation mailbox_info = mailboxes.get(0);
+            Imap.FolderProperties? props = null;
+            if (!mailbox_info.attrs.is_no_select) {
+                StatusData status = yield send_status_async(
+                    session,
+                    mailbox_info.mailbox,
+                    StatusDataType.all(),
+                    cancellable
+                );
+                props = new Imap.FolderProperties.selectable(
+                    mailbox_info.attrs,
+                    status,
+                    session.capabilities
+                );
+            } else {
+                props = new Imap.FolderProperties.not_selectable(mailbox_info.attrs);
+            }
+
+            folder = new Imap.Folder(path, props);
+            this.folders.set(path, folder);
         }
         return folder;
     }
@@ -182,7 +160,7 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
      * folders found, and hence should be used with care.
      */
     public async Gee.List<Imap.Folder> fetch_child_folders_async(FolderPath? parent, Cancellable? 
cancellable)
-    throws Error {
+        throws Error {
         ClientSession session = claim_session();
         Gee.List<Imap.Folder> children = new Gee.ArrayList<Imap.Folder>();
         Gee.List<MailboxInformation> mailboxes = yield send_list_async(session, parent, true, cancellable);
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala 
b/src/engine/imap/transport/imap-client-session-manager.vala
index 14ecec5..61d6fbc 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -96,13 +96,14 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
     private bool untrusted_host = false;
 
     /**
-     * Fired after when the manager has a working connection.
+     * Fired when the manager's ready state changes.
      *
-     * This will be fired both after opening if online and once at
-     * least one connection has been established, and after the server
-     * has become reachable again after being unreachable.
+     * This will be fired after opening if online and once at least
+     * one connection has been established, after the server has
+     * become reachable again after being unreachable, and if the
+     * server becomes unreachable.
      */
-    public signal void ready();
+    public signal void ready(bool is_ready);
 
     /** Fired when a network or non-auth error occurs opening a session. */
     public signal void connection_failed(Error err);
@@ -129,7 +130,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
 
         this.pool_stop = new TimeoutManager.seconds(
             POOL_STOP_TIMEOUT_SEC,
-            () => { this.force_disconnect_all.begin(); }
+            () => { this.close_pool.begin(); }
         );
     }
 
@@ -163,16 +164,12 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
             return;
 
         this.is_open = false;
-        this.is_ready = false;
-
-        this.pool_start.reset();
-        this.pool_stop.reset();
         this.pool_cancellable.cancel();
 
                this.endpoint.connectivity.notify["is-reachable"].disconnect(on_connectivity_change);
         this.endpoint.connectivity.address_error_reported.disconnect(on_connectivity_error);
 
-        yield force_disconnect_all();
+        yield close_pool();
 
         // TODO: This isn't the best (deterministic) way to deal with this, but it's easy and works
         // for now
@@ -222,7 +219,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
     public async ClientSession claim_authorized_session_async(Cancellable? cancellable)
         throws Error {
         check_open();
-        debug("[%s] Claiming session from %d of %d free",
+        debug("[%s] Claiming session with %d of %d free",
               this.id, this.free_queue.size, this.all_sessions.size);
 
         if (this.authentication_failed)
@@ -237,11 +234,11 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         ClientSession? claimed = null;
         while (claimed == null) {
             // This isn't racy since this is class is not accessed by
-            // multiple threads. Don't wait for it though because we
-            // only want to kick off establishing the connection, and
-            // wait for it via the queue.
+            // multiple threads. Don't wait for it to return though
+            // because we only want to kick off establishing the
+            // connection, and wait for it via the queue.
             if (this.free_queue.size == 0) {
-                check_pool.begin();
+                this.check_pool.begin(true);
             }
 
             claimed = yield this.free_queue.receive(cancellable);
@@ -310,36 +307,45 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
             throw new EngineError.OPEN_REQUIRED("ClientSessionManager is not open");
     }
 
-    private async void check_pool() {
+    private async void check_pool(bool is_claiming = false) {
         debug("[%s] Checking session pool with %d of %d free",
               this.id, this.free_queue.size, this.all_sessions.size);
 
-        while (this.is_open &&
-               !this.authentication_failed &&
-               !this.untrusted_host &&
-               this.endpoint.connectivity.is_reachable.is_certain()) {
-            // Open pool sessions serially to avoid hammering the server
-            try {
-                ClientSession free = yield this.create_new_authorized_session(
-                    this.pool_cancellable
-                );
-                yield this.sessions_mutex.execute_locked(() => {
-                        this.all_sessions.add(free);
-                    });
-
-                this.free_queue.send(free);
-            } catch (Error err) {
-                debug("[%s] Error adding free session pool: %s",
-                      this.id, err.message);
-                break;
+        this.pool_start.reset();
+
+        if (this.is_open &&
+            !this.authentication_failed &&
+            !this.untrusted_host &&
+            this.endpoint.connectivity.is_reachable.is_certain()) {
+
+            int needed = this.min_pool_size - this.all_sessions.size;
+            if (needed <= 0 && is_claiming) {
+                needed = 1;
             }
 
-            if (this.all_sessions.size >= this.min_pool_size) {
-                break;
+            // Open as many as needed in parallel
+            while (needed > 0) {
+                add_pool_session.begin();
+                needed--;
             }
         }
     }
 
+    private async void add_pool_session() {
+        try {
+            ClientSession free = yield this.create_new_authorized_session(
+                this.pool_cancellable
+            );
+            yield this.sessions_mutex.execute_locked(() => {
+                    this.all_sessions.add(free);
+                });
+            this.free_queue.send(free);
+        } catch (Error err) {
+            debug("[%s] Error adding new session to the pool: %s",
+                  this.id, err.message);
+        }
+    }
+
     /** Determines if a session is valid, disposing of it if not. */
     private async bool check_session(ClientSession target, bool allow_selected) {
         bool valid = false;
@@ -422,26 +428,31 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         // already done so.
         if (!this.is_ready) {
             debug("[%s] Became ready", this.id);
-            this.is_ready = true;
-            ready();
+            notify_ready(true);
         }
 
         return new_session;
     }
 
-    /** Disconnects all sessions in the pool. */
-    private async void force_disconnect_all()
-        throws Error {
-        debug("[%s] Dropping and disconnecting %d sessions",
+    private async void close_pool() {
+        debug("[%s] Closing the pool, disconnecting %d sessions",
               this.id, this.all_sessions.size);
 
+        this.pool_start.reset();
+        this.pool_stop.reset();
+        notify_ready(false);
+
         // Take a copy and work off that while scheduling disconnects,
         // since as they disconnect they'll remove themselves from the
         // sessions list and cause the loop below to explode.
         ClientSession[]? to_close = null;
-        yield this.sessions_mutex.execute_locked(() => {
-                to_close = this.all_sessions.to_array();
-            });
+        try {
+            yield this.sessions_mutex.execute_locked(() => {
+                    to_close = this.all_sessions.to_array();
+                });
+        } catch (Error err) {
+            debug("Error occurred copying sessions: %s", err.message);
+        }
 
         // Disconnect all existing sessions at once. Don't block
         // waiting for any since we don't want to delay closing the
@@ -482,6 +493,11 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         return removed;
     }
 
+    private void notify_ready(bool is_ready) {
+        this.is_ready = is_ready;
+        ready(is_ready);
+    }
+
     private void on_disconnected(ClientSession session, ClientSession.DisconnectReason reason) {
         this.remove_session_async.begin(
             session,
@@ -497,16 +513,14 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
     }
 
     private void on_login_failed(ClientSession session, StatusResponse? response) {
-        this.is_ready = false;
         this.authentication_failed = true;
         login_failed(response);
-        session.disconnect_async.begin();
+        this.close_pool.begin();
     }
 
     private void on_imap_untrusted_host() {
-        // this is called any time trust issues are detected, so immediately clutch in to stop
-        // retries
-        untrusted_host = true;
+        this.untrusted_host = true;
+        this.close_pool.begin();
     }
 
     private void on_imap_trust_untrusted_host() {
@@ -526,18 +540,14 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
             this.pool_start.start();
             this.pool_stop.reset();
                } else {
-            // Get a ready signal again once we are back online
-            this.is_ready = false;
             this.pool_start.reset();
             this.pool_stop.start();
         }
        }
 
        private void on_connectivity_error(Error error) {
-        this.is_ready = false;
-        this.pool_start.reset();
-        this.pool_stop.start();
         connection_failed(error);
+        this.close_pool.begin();
        }
 
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]