[geary/wip/789924-network-transition: 8/15] Only attempt to obtain an ImapClientSession when some are available.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/789924-network-transition: 8/15] Only attempt to obtain an ImapClientSession when some are available.
- Date: Sun, 12 Nov 2017 11:11:19 +0000 (UTC)
commit 575d3f1d99052f3fa02b2482371bb64041ac63a1
Author: Michael James Gratton <mike vee net>
Date: Thu Nov 9 17:12:48 2017 +1100
Only attempt to obtain an ImapClientSession when some are available.
This commit really does two things:
1. Ensures that users of ImapClientSession don't attempt to get one
when they shouldn't.
2. Ensures that users of ImapClientSession are notified when one is
available.
This doesn't update the background synchronizer however, that will happen
next.
* src/engine/imap/transport/imap-client-session-manager.vala
(ImapClientSessionManager): add is_ready property and ready
signal. Keep these up to date as auth and connectivity changes.
* src/engine/imap/api/imap-account.vala (Account): Also add is_ready
property and ready signal, proxy these through from the client session
manager so we don't need to expose that implementation detail to uses
of the class.
* src/engine/imap-engine/imap-engine-generic-account.vala
(GenericAccount): Simply the process for calling the folder enumeration
process, and make sure it gets cancelled if running and the account is
closed. Ensure that it doesn't attempt anything remote if the remote
account is not ready. When the remote account becomes ready, launch an
enumeration right away.
* src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
Simply the process for opening the rempte folder. Ensure that it isn't
attemptted before the remote account is ready. Do open the remote
folder when the account becomes ready.
.../imap-engine/imap-engine-generic-account.vala | 158 ++++++++++----------
.../imap-engine/imap-engine-minimal-folder.vala | 85 +++++------
src/engine/imap/api/imap-account.vala | 31 +++-
.../transport/imap-client-session-manager.vala | 39 ++++-
4 files changed, 174 insertions(+), 139 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index fdadb40..14af15c 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -28,25 +28,32 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
private Gee.HashMap<FolderPath, uint> refresh_unseen_timeout_ids
= new Gee.HashMap<FolderPath, uint>();
private Gee.HashSet<Geary.Folder> in_refresh_unseen = new Gee.HashSet<Geary.Folder>();
- private uint refresh_folder_timeout_id = 0;
- private bool in_refresh_enumerate = false;
- private Cancellable refresh_cancellable = new Cancellable();
private bool awaiting_credentials = false;
+ private Cancellable? enumerate_folder_cancellable = null;
+ private TimeoutManager refresh_folder_timer;
private Gee.Map<Geary.SpecialFolderType, Gee.List<string>> special_search_names =
new Gee.HashMap<Geary.SpecialFolderType, Gee.List<string>>();
+
public GenericAccount(string name, Geary.AccountInformation information,
Imap.Account remote, ImapDB.Account local) {
base (name, information);
-
+
this.remote = remote;
+ this.remote.ready.connect(on_remote_ready);
+
this.local = local;
this.remote.login_failed.connect(on_login_failed);
- this.local.email_sent.connect(on_email_sent);
this.local.contacts_loaded.connect(() => { contacts_loaded(); });
-
+ this.local.email_sent.connect(on_email_sent);
+
+ this.refresh_folder_timer = new TimeoutManager.seconds(
+ REFRESH_FOLDER_LIST_SEC,
+ () => { this.enumerate_folders_async.begin(); }
+ );
+
search_upgrade_monitor = local.search_index_monitor;
db_upgrade_monitor = local.upgrade_monitor;
db_vacuum_monitor = local.vacuum_monitor;
@@ -159,8 +166,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
// IMAP password before attempting a connection. This might have to be
// reworked when we allow passwordless logins.
if (!information.imap_credentials.is_complete())
- yield information.fetch_passwords_async(ServiceFlag.IMAP);
-
+ yield information.get_passwords_async(ServiceFlag.IMAP);
+
// need to back out local.open_async() if remote fails
try {
yield remote.open_async(cancellable);
@@ -174,22 +181,25 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
throw err;
}
-
- open = true;
-
- notify_opened();
+ this.open = true;
+
+ notify_opened();
notify_folders_available_unavailable(sort_by_path(local_only.values), null);
-
- // schedule an immediate sweep of the folders; once this is finished, folders will be
- // regularly enumerated
- reschedule_folder_refresh(true);
+
+ this.enumerate_folders_async.begin();
}
-
+
public override async void close_async(Cancellable? cancellable = null) throws Error {
if (!open)
return;
+ Cancellable folder_cancellable = this.enumerate_folder_cancellable;
+ if (folder_cancellable != null) {
+ folder_cancellable.cancel();
+ }
+ this.refresh_folder_timer.reset();
+
notify_folders_available_unavailable(null, sort_by_path(local_only.values));
notify_folders_available_unavailable(null, sort_by_path(folder_map.values));
@@ -375,73 +385,61 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
in_refresh_unseen.remove(folder);
}
}
-
- private void reschedule_folder_refresh(bool immediate) {
- if (in_refresh_enumerate)
+
+ private async void enumerate_folders_async()
+ throws Error {
+ check_open();
+
+ if (this.enumerate_folder_cancellable != null) {
return;
-
- cancel_folder_refresh();
-
- refresh_folder_timeout_id = immediate
- ? Idle.add(on_refresh_folders)
- : Timeout.add_seconds(REFRESH_FOLDER_LIST_SEC, on_refresh_folders);
- }
-
- private void cancel_folder_refresh() {
- if (refresh_folder_timeout_id != 0) {
- Source.remove(refresh_folder_timeout_id);
- refresh_folder_timeout_id = 0;
}
- }
-
- private bool on_refresh_folders() {
- in_refresh_enumerate = true;
- enumerate_folders_async.begin(refresh_cancellable, on_refresh_completed);
-
- refresh_folder_timeout_id = 0;
-
- return false;
- }
-
- private void on_refresh_completed(Object? source, AsyncResult result) {
+ Cancellable? cancellable = this.enumerate_folder_cancellable = new Cancellable();
+ this.refresh_folder_timer.reset();
+
+ debug("%s: Enumerating folders", to_string());
+
+ bool successful = false;
try {
- enumerate_folders_async.end(result);
+ // enumerate local folders first
+ Gee.HashMap<FolderPath, ImapDB.Folder> local_folders = yield enumerate_local_folders_async(
+ null, cancellable
+ );
+
+ // convert to a list of Geary.Folder ... build_folder() also reports new folders, so this
+ // gets the word out quickly (local_only folders have already been reported)
+ Gee.Collection<Geary.Folder> existing_list = new Gee.ArrayList<Geary.Folder>();
+ existing_list.add_all(build_folders(local_folders.values));
+ existing_list.add_all(local_only.values);
+
+ if (this.remote.is_ready && !cancellable.is_cancelled()) {
+ // build a map of all existing folders
+ Gee.HashMap<FolderPath, Geary.Folder> existing_folders
+ = Geary.traverse<Geary.Folder>(existing_list).to_hash_map<FolderPath>(f => f.path);
+
+ // now that all local have been enumerated and
+ // reported (this is important to assist startup of
+ // the UI), enumerate the remote folders
+ bool remote_folders_suspect;
+ Gee.HashMap<FolderPath, Imap.Folder>? remote_folders = yield enumerate_remote_folders_async(
+ null, out remote_folders_suspect, cancellable);
+
+ // pair the local and remote folders and make sure everything is up-to-date
+ yield update_folders_async(existing_folders, remote_folders, remote_folders_suspect,
cancellable);
+
+ successful = true;
+ }
} catch (Error err) {
- if (!(err is IOError.CANCELLED))
- debug("Refresh of account %s folders did not complete: %s", to_string(), err.message);
+ if (!(err is IOError.CANCELLED)) {
+ report_problem(Geary.Account.Problem.CONNECTION_FAILURE, err);
+ }
+ }
+
+ this.enumerate_folder_cancellable = null;
+ if (successful) {
+ this.refresh_folder_timer.start();
}
-
- in_refresh_enumerate = false;
- reschedule_folder_refresh(false);
- }
-
- private async void enumerate_folders_async(Cancellable? cancellable) throws Error {
- check_open();
-
- // enumerate local folders first
- Gee.HashMap<FolderPath, ImapDB.Folder> local_folders = yield enumerate_local_folders_async(
- null, cancellable);
-
- // convert to a list of Geary.Folder ... build_folder() also reports new folders, so this
- // gets the word out quickly (local_only folders have already been reported)
- Gee.Collection<Geary.Folder> existing_list = new Gee.ArrayList<Geary.Folder>();
- existing_list.add_all(build_folders(local_folders.values));
- existing_list.add_all(local_only.values);
-
- // build a map of all existing folders
- Gee.HashMap<FolderPath, Geary.Folder> existing_folders
- = Geary.traverse<Geary.Folder>(existing_list).to_hash_map<FolderPath>(f => f.path);
-
- // now that all local have been enumerated and reported (this is important to assist
- // startup of the UI), enumerate the remote folders
- bool remote_folders_suspect;
- Gee.HashMap<FolderPath, Imap.Folder>? remote_folders = yield enumerate_remote_folders_async(
- null, out remote_folders_suspect, cancellable);
-
- // pair the local and remote folders and make sure everything is up-to-date
- yield update_folders_async(existing_folders, remote_folders, remote_folders_suspect, cancellable);
}
-
+
private async Gee.HashMap<FolderPath, ImapDB.Folder> enumerate_local_folders_async(
Geary.FolderPath? parent, Cancellable? cancellable) throws Error {
check_open();
@@ -964,6 +962,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
do_login_failed_async.begin(credentials, response, () => { awaiting_credentials = false; });
}
+ private void on_remote_ready() {
+ this.enumerate_folders_async.begin();
+ }
+
private async void do_login_failed_async(Geary.Credentials? credentials, Geary.Imap.StatusResponse?
response) {
bool reask_password = true;
try {
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index b0b3f72..959b522 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -43,12 +43,13 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
private Folder.OpenFlags open_flags = OpenFlags.NONE;
private int open_count = 0;
private bool remote_opened = false;
+ private bool remote_ready = false;
+ private TimeoutManager remote_open_timer;
private Nonblocking.ReportingSemaphore<bool> remote_semaphore =
new Nonblocking.ReportingSemaphore<bool>(false);
private Nonblocking.Semaphore closed_semaphore = new Nonblocking.Semaphore();
private ReplayQueue replay_queue;
private int remote_count = -1;
- private uint open_remote_timer_id = 0;
private int reestablish_delay_msec = DEFAULT_REESTABLISH_DELAY_MSEC;
private Nonblocking.Mutex open_mutex = new Nonblocking.Mutex();
private Nonblocking.Mutex close_mutex = new Nonblocking.Mutex();
@@ -77,8 +78,11 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
public MinimalFolder(GenericAccount account, Imap.Account remote, ImapDB.Account local,
ImapDB.Folder local_folder, SpecialFolderType special_folder_type) {
- _account = account;
+ this._account = account;
this.remote = remote;
+ this.remote_open_timer = new TimeoutManager.seconds(
+ FORCE_OPEN_REMOTE_TIMEOUT_SEC, () => { start_open_remote(); }
+ );
this.local = local;
this.local_folder = local_folder;
_special_folder_type = special_folder_type;
@@ -92,16 +96,13 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
local_folder.email_complete.connect(on_email_complete);
}
-
+
~MinimalFolder() {
if (open_count > 0)
warning("Folder %s destroyed without closing", to_string());
-
- cancel_remote_open_timer();
-
- local_folder.email_complete.disconnect(on_email_complete);
+ this.local_folder.email_complete.disconnect(on_email_complete);
}
-
+
protected virtual void notify_closing(Gee.List<ReplayOperation> final_ops) {
closing(final_ops);
}
@@ -520,8 +521,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// an open_async, it's reset at close time
if (!remote_opened) {
debug("wait_for_open_async %s: opening remote on demand...", to_string());
-
- start_remote_open_now();
+ start_open_remote();
}
if (!yield remote_semaphore.wait_for_result_async(cancellable))
@@ -536,8 +536,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// add NO_DELAY flag if it forces an open
if (!remote_opened)
this.open_flags |= OpenFlags.NO_DELAY;
-
- start_remote_open_now();
+
+ start_open_remote();
}
debug("Not opening %s: already open", to_string());
@@ -565,49 +565,32 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// second account Inbox they don't manipulate), no remote connection will ever be made,
// meaning that folder normalization never happens and unsolicited notifications never
// arrive
- if (open_flags.is_all_set(OpenFlags.NO_DELAY))
- start_remote_open_now();
- else
- start_remote_open_timer();
-
+ this.remote.ready.connect(on_remote_ready);
+ if (open_flags.is_all_set(OpenFlags.NO_DELAY)) {
+ start_open_remote();
+ } else {
+ this.remote_open_timer.start();
+ }
return true;
}
- private void start_remote_open_timer() {
- if (this.open_remote_timer_id != 0)
- Source.remove(this.open_remote_timer_id);
-
- this.open_remote_timer_id = Timeout.add_seconds(FORCE_OPEN_REMOTE_TIMEOUT_SEC, () => {
- start_remote_open_now();
- this.open_remote_timer_id = 0;
- return false;
- });
- }
-
- private void start_remote_open_now() {
- if (remote_opened)
- return;
-
- cancel_remote_open_timer();
- remote_opened = true;
-
- open_remote_async.begin(null);
- }
-
- private void cancel_remote_open_timer() {
- if (this.open_remote_timer_id == 0)
- return;
-
- Source.remove(this.open_remote_timer_id);
- this.open_remote_timer_id = 0;
+ private void start_open_remote() {
+ if (!this.remote_opened &&
+ !this.remote_open_timer.is_running &&
+ this.remote_ready) {
+ this.remote_open_timer.reset();
+ this.remote_opened = true;
+ this.open_remote_async.begin(null);
+ }
}
// Open the remote connection using a Mutex to prevent concurrency.
//
- // start_remote_open_now() *should* prevent more than one open from occurring at the same time,
+ // start_open_remote() *should* prevent more than one open from occurring at the same time,
// but it's still wise to use a nonblocking primitive to prevent it if that does occur to at
// least keep Folder state cogent.
private async void open_remote_async(Cancellable? cancellable) {
+ debug("%s: Opening remote folder", to_string());
int token;
try {
token = yield open_mutex.claim_async(cancellable);
@@ -822,8 +805,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
bool flush_pending, Cancellable? cancellable) {
// make sure no open is waiting in the wings to start; close_internal_locked_async() will
// reestablish a connection if necessary
- cancel_remote_open_timer();
-
+ this.remote_open_timer.reset();
+
int token;
try {
token = yield close_mutex.claim_async(cancellable);
@@ -1571,10 +1554,14 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
return ret;
}
-
+
public override string to_string() {
return "%s (open_count=%d remote_opened=%s)".printf(base.to_string(), open_count,
remote_opened.to_string());
}
-}
+ private void on_remote_ready() {
+ this.remote_ready = true;
+ start_open_remote();
+ }
+}
diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala
index 14c1b0e..da8a066 100644
--- a/src/engine/imap/api/imap-account.vala
+++ b/src/engine/imap/api/imap-account.vala
@@ -19,8 +19,16 @@
*/
private class Geary.Imap.Account : BaseObject {
+ /** Determines if the IMAP account has been opened. */
public bool is_open { get; private set; default = false; }
-
+
+ /**
+ * Determines if the IMAP account has a working connection.
+ *
+ * See {@link ClientSessionManager.is_open} for more details.
+ */
+ public bool is_ready { get { return this.session_mgr.is_ready; } }
+
private string name;
private AccountInformation account_information;
private ClientSessionManager session_mgr;
@@ -36,15 +44,22 @@ private class Geary.Imap.Account : BaseObject {
private Imap.MailboxSpecifier? inbox_specifier = null;
private string hierarchy_delimiter = null;
-
+ /**
+ * Fired after opening when the account has a working connection.
+ *
+ * This may be fired multiple times, see @{link
+ * ClientSessionManager.ready} for details.
+ */
+ public signal void ready();
+
public signal void login_failed(Geary.Credentials? cred, StatusResponse? response);
-
+
public Account(Geary.AccountInformation account_information) {
name = "IMAP Account for %s".printf(account_information.imap_credentials.to_string());
this.account_information = account_information;
this.session_mgr = new ClientSessionManager(account_information);
-
- session_mgr.login_failed.connect(on_login_failed);
+ this.session_mgr.ready.connect(on_session_ready);
+ this.session_mgr.login_failed.connect(on_login_failed);
}
private void check_open() throws Error {
@@ -607,7 +622,11 @@ private class Geary.Imap.Account : BaseObject {
private void on_login_failed(StatusResponse? response) {
login_failed(account_information.imap_credentials, response);
}
-
+
+ private void on_session_ready() {
+ ready();
+ }
+
public string to_string() {
return name;
}
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala
b/src/engine/imap/transport/imap-client-session-manager.vala
index 625c0e2..02f84a4 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -12,9 +12,19 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
private const int POOL_RETRY_MIN_TIMEOUT_SEC = 1;
private const int POOL_RETRY_MAX_TIMEOUT_SEC = 10;
+ /** Determines if the manager has been opened. */
public bool is_open { get; private set; default = false; }
/**
+ * Determines if the manager has a working connection.
+ *
+ * This will be true once at least one connection has been
+ * established, and after the server has become reachable again
+ * after being unreachable.
+ */
+ public bool is_ready { get; private set; default = false; }
+
+ /**
* Set to zero or negative value if keepalives should be disabled when a connection has not
* selected a mailbox. (This is not recommended.)
*
@@ -60,6 +70,16 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
private TimeoutManager pool_start;
private TimeoutManager pool_retry;
+ /**
+ * Fired after when the manager has a working connection.
+ *
+ * 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.
+ */
+ public signal void ready();
+
+ /** Fired when an authentication error occurs opening a session. */
public signal void login_failed(StatusResponse? response);
public ClientSessionManager(AccountInformation account_information) {
@@ -111,7 +131,8 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
if (!is_open)
return;
- is_open = false;
+ this.is_open = false;
+ this.is_ready = false;
this.endpoint.connectivity.notify["is-reachable"].disconnect(on_connectivity_change);
@@ -259,6 +280,11 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
throw err;
}
+ if (!this.is_ready) {
+ this.is_ready = true;
+ ready();
+ }
+
// reset delay
this.pool_retry.interval = POOL_RETRY_MIN_TIMEOUT_SEC;
@@ -458,15 +484,14 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
private void on_disconnected(ClientSession session, ClientSession.DisconnectReason reason) {
force_disconnect.begin(session, false);
}
-
+
private void on_login_failed(ClientSession session, StatusResponse? response) {
- authentication_failed = true;
-
+ this.is_ready = false;
+ this.authentication_failed = true;
login_failed(response);
-
session.disconnect_async.begin();
}
-
+
// Only call with sessions mutex locked
private void locked_add_session(ClientSession session) {
sessions.add(session);
@@ -525,6 +550,8 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
if (is_reachable) {
this.pool_start.start();
} else {
+ // Get a ready signal again once we are back online
+ this.is_ready = false;
this.pool_start.reset();
this.pool_retry.reset();
this.force_disconnect_all.begin();
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
