[geary] Improved connection reestablishment for IMAP folders: Bug #713972
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Improved connection reestablishment for IMAP folders: Bug #713972
- Date: Fri, 23 Jan 2015 01:20:50 +0000 (UTC)
commit ed5914e468ca2d6f472cd9b851f88665faab4cf6
Author: Jim Nelson <jim yorba org>
Date: Thu Jan 22 17:11:01 2015 -0800
Improved connection reestablishment for IMAP folders: Bug #713972
This patch drastically improves network connection handling and
reestablishment. In particular, the ImapEngine.Folder's open_count
wasn't always properly maintained (in the engine and the client)
causing connection reestablishment issues. Also, prior code attempted
to deduce when to reestablish a connection based on the type of Error
encountered. Now, as long as the Folder's open_count is greater than
zero, reestablishment will be attempted (with a back-off algorithm and
special cases in the ClientSessionManager preventing continual login,
i.e. when credentials are bad).
ClientSessionManager also has a back-off algorithm when it's adjusting
its session pool and uses the NetworkMonitor to detect when the host
is simply unavailable due to the network being unavailable (to prevent
continuous retries).
In addition to the above bug, this patch solves the following tickets:
Bug #714595
Bug #725959
Bug #712960
src/client/application/geary-controller.vala | 7 +-
src/engine/app/app-conversation-monitor.vala | 27 +--
src/engine/imap-db/imap-db-folder.vala | 35 +++
.../imap-engine-account-synchronizer.vala | 29 ++-
.../imap-engine/imap-engine-generic-account.vala | 4 +-
.../imap-engine/imap-engine-minimal-folder.vala | 232 +++++++++++++-------
.../replay-ops/imap-engine-replay-disconnect.vala | 5 +-
src/engine/imap/api/imap-account.vala | 17 ++-
src/engine/imap/imap-error.vala | 4 +
.../transport/imap-client-session-manager.vala | 125 +++++++++--
10 files changed, 352 insertions(+), 133 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 27affab..2bae336 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1275,13 +1275,10 @@ public class GearyController : Geary.BaseObject {
Cancellable? conversation_cancellable = (current_is_inbox ?
inbox_cancellables.get(folder.account) : cancellable_folder);
- // stop monitoring for conversations and close the folder (but only if not an inbox,
- // which we leave open for notifications)
+ // stop monitoring for conversations and close the folder
if (current_conversations != null) {
- yield current_conversations.stop_monitoring_async(!current_is_inbox, null);
+ yield current_conversations.stop_monitoring_async(null);
current_conversations = null;
- } else if (current_folder != null && !current_is_inbox) {
- yield current_folder.close_async();
}
// re-enable copy/move to the last selected folder
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 14bf509..ecad09f 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -328,12 +328,11 @@ public class Geary.App.ConversationMonitor : BaseObject {
* If null is supplied as the Cancellable, no cancellable is used; pass the original Cancellable
* here to use that.
*/
- public async void stop_monitoring_async(bool close_folder, Cancellable? cancellable) throws Error {
- yield stop_monitoring_internal_async(close_folder, false, cancellable);
+ public async void stop_monitoring_async(Cancellable? cancellable) throws Error {
+ yield stop_monitoring_internal_async(false, cancellable);
}
- private async void stop_monitoring_internal_async(bool close_folder, bool retrying,
- Cancellable? cancellable) throws Error {
+ private async void stop_monitoring_internal_async(bool retrying, Cancellable? cancellable) throws Error {
if (!is_monitoring)
return;
@@ -350,17 +349,15 @@ public class Geary.App.ConversationMonitor : BaseObject {
folder.account.email_locally_complete.disconnect(on_account_email_locally_complete);
Error? close_err = null;
- if (close_folder) {
- try {
- yield folder.close_async(cancellable);
- } catch (Error err) {
- // throw, but only after cleaning up (which is to say, if close_async() fails,
- // then the Folder is still treated as closed, which is the best that can be
- // expected; it definitely shouldn't still be considered open).
- debug("Unable to close monitored folder %s: %s", folder.to_string(), err.message);
-
- close_err = err;
- }
+ try {
+ yield folder.close_async(cancellable);
+ } catch (Error err) {
+ // throw, but only after cleaning up (which is to say, if close_async() fails,
+ // then the Folder is still treated as closed, which is the best that can be
+ // expected; it definitely shouldn't still be considered open).
+ debug("Unable to close monitored folder %s: %s", folder.to_string(), err.message);
+
+ close_err = err;
}
notify_monitoring_stopped(retrying);
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index e1b5abe..1919515 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -173,6 +173,41 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
properties.set_select_examine_message_count(count);
}
+ public async Imap.StatusData fetch_status_data(ListFlags flags, Cancellable? cancellable) throws Error {
+ Imap.StatusData? status_data = null;
+ yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
+ Db.Statement stmt = cx.prepare("""
+ SELECT uid_next, uid_validity, unread_count
+ FROM FolderTable
+ WHERE id = ?
+ """);
+ stmt.bind_rowid(0, folder_id);
+
+ Db.Result result = stmt.exec(cancellable);
+ if (result.finished)
+ return Db.TransactionOutcome.DONE;
+
+ int messages = do_get_email_count(cx, flags, cancellable);
+ Imap.UID? uid_next = !result.is_null_for("uid_next")
+ ? new Imap.UID(result.int64_for("uid_next"))
+ : null;
+ Imap.UIDValidity? uid_validity = !result.is_null_for("uid_validity")
+ ? new Imap.UIDValidity(result.int64_for("uid_validity"))
+ : null;
+
+ // Note that recent is not stored
+ status_data = new Imap.StatusData(new Imap.MailboxSpecifier.from_folder_path(path, null),
+ messages, 0, uid_next, uid_validity, result.int_for("unread_count"));
+
+ return Db.TransactionOutcome.DONE;
+ }, cancellable);
+
+ if (status_data == null)
+ throw new EngineError.NOT_FOUND("%s STATUS not found in database", path.to_string());
+
+ return status_data;
+ }
+
// Returns a Map with the created or merged email as the key and the result of the operation
// (true if created, false if merged) as the value. Note that every email
// object passed in's EmailIdentifier will be fully filled out by this
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index 66d6aa3..de64620 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -7,6 +7,7 @@
private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
private const int FETCH_DATE_RECEIVED_CHUNK_COUNT = 25;
private const int SYNC_DELAY_SEC = 10;
+ private const int RETRY_SYNC_DELAY_SEC = 60;
public GenericAccount account { get; private set; }
@@ -80,7 +81,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
// treat as an availability check (i.e. as if the account had just opened) because
// just because this value has changed doesn't mean the contents in the folders
// have changed
- delayed_send_all(account.list_folders(), true);
+ delayed_send_all(account.list_folders(), true, SYNC_DELAY_SEC);
} catch (Error err) {
debug("Unable to schedule re-sync for %s due to prefetch time changing: %s",
account.to_string(), err.message);
@@ -96,7 +97,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
foreach (Folder folder in available)
unavailable_paths.remove(folder.path);
- delayed_send_all(available, true);
+ delayed_send_all(available, true, SYNC_DELAY_SEC);
}
if (unavailable != null) {
@@ -108,7 +109,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
}
private void on_folders_contents_altered(Gee.Collection<Folder> altered) {
- delayed_send_all(altered, false);
+ delayed_send_all(altered, false, SYNC_DELAY_SEC);
}
private void on_email_sent() {
@@ -121,8 +122,8 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
}
}
- private void delayed_send_all(Gee.Collection<Folder> folders, bool reason_available) {
- Timeout.add_seconds(SYNC_DELAY_SEC, () => {
+ private void delayed_send_all(Gee.Collection<Folder> folders, bool reason_available, int sec) {
+ Timeout.add_seconds(sec, () => {
// remove any unavailable folders
Gee.ArrayList<Folder> trimmed_folders = new Gee.ArrayList<Folder>();
foreach (Folder folder in folders) {
@@ -334,16 +335,24 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
debug("Unable to open %s: %s", folder.to_string(), err.message);
+ // retry later
+ delayed_send_all(iterate<Folder>(folder).to_array_list(), false, RETRY_SYNC_DELAY_SEC);
+
return true;
}
+ bool not_cancelled = true;
try {
yield sync_folder_async(folder, epoch, oldest_local, oldest_local_id);
} catch (Error err) {
- if (err is IOError.CANCELLED)
- return false;
-
- debug("Error background syncing folder %s: %s", folder.to_string(), err.message);
+ if (err is IOError.CANCELLED) {
+ not_cancelled = false;
+ } else {
+ debug("Error background syncing folder %s: %s", folder.to_string(), err.message);
+
+ // retry later
+ delayed_send_all(iterate<Folder>(folder).to_array_list(), false, RETRY_SYNC_DELAY_SEC);
+ }
// fallthrough and close
}
@@ -355,7 +364,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
debug("Error closing %s: %s", folder.to_string(), err.message);
}
- return true;
+ return not_cancelled;
}
private async void sync_folder_async(MinimalFolder folder, DateTime epoch, DateTime? oldest_local,
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index ff441f2..9c4361c 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -340,7 +340,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
bool folder_created;
Imap.Folder remote_folder = yield remote.fetch_folder_async(folder.path,
- out folder_created, cancellable);
+ out folder_created, null, cancellable);
if (!folder_created) {
int unseen_count = yield remote.fetch_unseen_count_async(folder.path, cancellable);
@@ -517,7 +517,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
continue;
Imap.Folder remote_folder = (Imap.Folder) yield remote.fetch_folder_async(folder,
- null, cancellable);
+ null, null, cancellable);
yield local.clone_folder_async(remote_folder, cancellable);
}
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 0f5213d..66d39b7 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -7,8 +7,8 @@
private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport.Copy,
Geary.FolderSupport.Mark, Geary.FolderSupport.Move {
private const int FORCE_OPEN_REMOTE_TIMEOUT_SEC = 10;
- private const int DEFAULT_REESTABLISH_DELAY_MSEC = 10;
- private const int MAX_REESTABLISH_DELAY_MSEC = 30000;
+ private const int DEFAULT_REESTABLISH_DELAY_MSEC = 500;
+ private const int MAX_REESTABLISH_DELAY_MSEC = 60 * 1000;
public override Account account { get { return _account; } }
@@ -48,6 +48,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
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();
public MinimalFolder(GenericAccount account, Imap.Account remote, ImapDB.Account local,
ImapDB.Folder local_folder, SpecialFolderType special_folder_type) {
@@ -70,6 +72,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
if (open_count > 0)
warning("Folder %s destroyed without closing", to_string());
+ cancel_remote_open_timer();
+
local_folder.email_complete.disconnect(on_email_complete);
}
@@ -144,8 +148,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
}
}
- private async bool normalize_folders(Geary.Imap.Folder remote_folder, Geary.Folder.OpenFlags open_flags,
- Cancellable? cancellable) throws Error {
+ private async bool normalize_folders(Geary.Imap.Folder remote_folder, Cancellable? cancellable)
+ throws Error {
debug("%s: Begin normalizing remote and local folders", to_string());
Geary.Imap.FolderProperties local_properties = local_folder.get_properties();
@@ -488,8 +492,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
if (!remote_opened) {
debug("wait_for_open_async %s: opening remote on demand...", to_string());
- remote_opened = true;
- open_remote_async.begin(open_flags, null);
+ start_remote_open_now();
}
if (!yield remote_semaphore.wait_for_result_async(cancellable))
@@ -499,25 +502,23 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
public override async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null)
throws Error {
if (open_count++ > 0) {
- // even if opened or opening, respect the NO_DELAY flag
+ // even if opened or opening, or if forcing a re-open, respect the NO_DELAY flag
if (open_flags.is_all_set(OpenFlags.NO_DELAY)) {
- cancel_remote_open_timer();
- wait_for_open_async.begin();
+ // add NO_DELAY flag if it forces an open
+ if (!remote_opened)
+ this.open_flags |= OpenFlags.NO_DELAY;
+
+ start_remote_open_now();
}
- debug("Not opening %s: already open (open_count=%d)", to_string(), open_count);
+ debug("Not opening %s: already open", to_string());
return false;
}
+ // first open gets to name the flags, but see note above
this.open_flags = open_flags;
- open_internal(open_flags, cancellable);
-
- return true;
- }
-
- private void open_internal(Folder.OpenFlags open_flags, Cancellable? cancellable) {
remote_semaphore = new Geary.Nonblocking.ReportingSemaphore<bool>(false);
// start the replay queue
@@ -535,16 +536,32 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// meaning that folder normalization never happens and unsolicited notifications never
// arrive
if (open_flags.is_all_set(OpenFlags.NO_DELAY))
- wait_for_open_async.begin();
+ start_remote_open_now();
else
start_remote_open_timer();
+
+ return true;
}
private void start_remote_open_timer() {
if (open_remote_timer_id != 0)
Source.remove(open_remote_timer_id);
- open_remote_timer_id = Timeout.add_seconds(FORCE_OPEN_REMOTE_TIMEOUT_SEC, on_open_remote_timeout);
+ open_remote_timer_id = Timeout.add_seconds(FORCE_OPEN_REMOTE_TIMEOUT_SEC, () => {
+ start_remote_open_now();
+
+ 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() {
@@ -555,18 +572,29 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
open_remote_timer_id = 0;
}
- private bool on_open_remote_timeout() {
- open_remote_timer_id = 0;
+ // 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,
+ // 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) {
+ int token;
+ try {
+ token = yield open_mutex.claim_async(cancellable);
+ } catch (Error err) {
+ return;
+ }
- // remote was not forced open due to caller, so open now
- wait_for_open_async.begin();
+ yield open_remote_locked_async(cancellable);
- return false;
+ try {
+ open_mutex.release(ref token);
+ } catch (Error err) {
+ }
}
- private async void open_remote_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable) {
- cancel_remote_open_timer();
-
+ // Should only be called when open_mutex is locked, i.e. use open_remote_async()
+ private async void open_remote_locked_async(Cancellable? cancellable) {
// watch for folder closing before this call got a chance to execute
if (open_count == 0)
return;
@@ -580,16 +608,20 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// carefully back out and possibly retry
Imap.Folder? opening_folder = null;
try {
+ debug("Fetching STATUS for remote %s from local", to_string());
+ Imap.StatusData local_status = yield local_folder.fetch_status_data(
+ ImapDB.Folder.ListFlags.NONE, cancellable);
+
debug("Fetching information for remote folder %s", to_string());
- opening_folder = yield remote.fetch_folder_async(local_folder.get_path(),
- null, cancellable);
+ opening_folder = yield remote.fetch_folder_async(local_folder.get_path(), null, local_status,
+ cancellable);
debug("Opening remote folder %s", opening_folder.to_string());
yield opening_folder.open_async(cancellable);
// allow subclasses to examine the opened folder and resolve any vital
// inconsistencies
- if (yield normalize_folders(opening_folder, open_flags, cancellable)) {
+ if (yield normalize_folders(opening_folder, cancellable)) {
// update flags, properties, etc.
yield local.update_folder_select_examine_async(opening_folder, cancellable);
@@ -612,15 +644,22 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
try {
yield opening_folder.close_async(null);
} catch (Error err) {
- debug("Error closing remote folder %s: %s", opening_folder.to_string(), err.message);
+ debug("%s: Error closing remote folder %s: %s", to_string(), opening_folder.to_string(),
+ err.message);
+
+ // fall through
}
// stop before starting the close
opening_monitor.notify_finish();
+ // normalize_folders() returning false indicates a soft error, but hard in the sense
+ // that opening cannot proceed, even with a connection retry
+ open_count = 0;
+
// schedule immediate close
close_internal_async.begin(CloseReason.LOCAL_CLOSE, CloseReason.REMOTE_CLOSE, false,
- false, cancellable);
+ cancellable);
return;
}
@@ -645,24 +684,25 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
}
Folder.CloseReason remote_reason;
- bool force_reestablishment;
if (hard_failure) {
// hard failure, retry
debug("Hard failure opening or preparing remote folder %s, retrying: %s", to_string(),
open_err.message);
remote_reason = CloseReason.REMOTE_ERROR;
- force_reestablishment = true;
} else {
// soft failure, treat as failure to open
- debug("Soft failure opening or preparing remote folder %s: %s", to_string(),
+ debug("Soft failure opening or preparing remote folder %s, closing: %s", to_string(),
open_err.message);
notify_open_failed(
is_cancellation ? Folder.OpenFailed.CANCELLED : Folder.OpenFailed.REMOTE_FAILED,
open_err);
remote_reason = CloseReason.REMOTE_CLOSE;
- force_reestablishment = false;
+
+ // clear open_count to ensure that close_internal_async() doesn't attempt to
+ // reestablish the connection
+ open_count = 0;
}
// be sure to close opening_folder if it was fetched or opened
@@ -670,15 +710,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
if (opening_folder != null)
yield opening_folder.close_async(null);
} catch (Error err) {
- debug("Error closing remote folder %s: %s", opening_folder.to_string(), err.message);
+ debug("%s: Error closing remote folder %s: %s", to_string(), opening_folder.to_string(),
+ err.message);
}
// stop before starting the close
opening_monitor.notify_finish();
- // schedule immediate close and force reestablishment
- close_internal_async.begin(CloseReason.LOCAL_CLOSE, remote_reason, force_reestablishment,
- false, null);
+ // schedule immediate close (and possible connection reestablishment)
+ close_internal_async.begin(CloseReason.LOCAL_CLOSE, remote_reason, false, null);
return;
}
@@ -694,7 +734,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
? remote_count
: yield local_folder.get_email_count_async(ImapDB.Folder.ListFlags.NONE, cancellable);
} catch (Error count_err) {
- debug("Unable to fetch count from local folder: %s", count_err.message);
+ debug("Unable to fetch count from local folder %s: %s", to_string(), count_err.message);
count = 0;
}
@@ -704,8 +744,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
try {
remote_semaphore.notify_result(remote_folder != null, null);
} catch (Error notify_err) {
- debug("Unable to fire semaphore notifying remote folder ready/not ready: %s",
- notify_err.message);
+ debug("%s: Unable to fire semaphore notifying remote folder ready/not ready: %s",
+ to_string(), notify_err.message);
// do this now rather than wait for close_internal_async() to execute to ensure that
// any replay operations already queued don't attempt to run
@@ -715,7 +755,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// schedule immediate close
close_internal_async.begin(CloseReason.LOCAL_CLOSE, CloseReason.REMOTE_CLOSE, false,
- false, cancellable);
+ cancellable);
return;
}
@@ -735,13 +775,35 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
if (remote_folder != null)
_properties.remove(remote_folder.properties);
- yield close_internal_async(CloseReason.LOCAL_CLOSE, CloseReason.REMOTE_CLOSE, false, true,
+ yield close_internal_async(CloseReason.LOCAL_CLOSE, CloseReason.REMOTE_CLOSE, true,
cancellable);
}
- // NOTE: This bypasses open_count and forces the Folder closed.
+ // Close the remote connection and, if open_count is zero, the Folder itself. A Mutex is used
+ // to prevent concurrency.
+ //
+ // NOTE: This bypasses open_count and forces the Folder closed, reestablishing a connection if
+ // open_count is greater than zero
internal async void close_internal_async(Folder.CloseReason local_reason, Folder.CloseReason
remote_reason,
- bool force_reestablish, bool flush_pending, Cancellable? cancellable) {
+ bool flush_pending, Cancellable? cancellable) {
+ int token;
+ try {
+ token = yield close_mutex.claim_async(cancellable);
+ } catch (Error err) {
+ return;
+ }
+
+ yield close_internal_locked_async(local_reason, remote_reason, flush_pending, cancellable);
+
+ try {
+ close_mutex.release(ref token);
+ } catch (Error err) {
+ }
+ }
+
+ // Should only be called when close_mutex is locked, i.e. use close_internal_async()
+ private async void close_internal_locked_async(Folder.CloseReason local_reason,
+ Folder.CloseReason remote_reason, bool flush_pending, Cancellable? cancellable) {
cancel_remote_open_timer();
// only flushing pending ReplayOperations if this is a "clean" close, not forced due to
@@ -780,7 +842,16 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
if (flush_pending)
closing_remote_folder = clear_remote_folder();
- if (closing_remote_folder != null || force_reestablish) {
+ // now treat remote as closed, i.e. a call to open_async() will reinitiate opening and not fall
+ // through (unless open_count is > 0) ... do this before close_remote_folder_async() since
+ // it's *possible* for it to loop back to open_async() before returning
+ remote_opened = false;
+
+ // use background call to (a) close remote if necessary and/or (b) reestablish connection if
+ // necessary ... store reestablish condition now, before scheduling close_remote_folder_async(),
+ // as it touches open_count
+ bool reestablish = open_count > 0;
+ if (closing_remote_folder != null || reestablish) {
// to avoid keeping the caller waiting while the remote end closes (i.e. drops the
// connection or performs an IMAP CLOSE operation), close it in the background and
// reestablish connection there, if necessary
@@ -793,19 +864,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// closed. Also not a problem, as GenericAccount does that internally. However, this
// might be an issue if GenericAccount removes this folder due to a user command or
// detection on the server, so this background op keeps a reference to the Folder
- close_remote_folder_async.begin(this, closing_remote_folder, remote_reason,
- force_reestablish);
+ close_remote_folder_async.begin(this, closing_remote_folder);
}
- remote_opened = false;
-
- // if remote reason is an error, then close_remote_folder_async() will be performing
- // reestablishment, so go no further
- if ((remote_reason.is_error() && closing_remote_folder != null) || force_reestablish)
+ // if reestablishing in close_remote_folder_async(), go no further
+ if (reestablish)
return;
// forced closed one way or another, so reset state
- open_count = 0;
+ open_flags = OpenFlags.NONE;
reestablish_delay_msec = DEFAULT_REESTABLISH_DELAY_MSEC;
// use remote_reason even if remote_folder was null; it could be that the error occurred
@@ -847,7 +914,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// See note in close_async() for why this method is static and uses an owned ref
private static async void close_remote_folder_async(owned MinimalFolder folder,
- owned Imap.Folder? remote_folder, Folder.CloseReason remote_reason, bool force_reestablish) {
+ owned Imap.Folder? remote_folder) {
// force the remote closed; if due to a remote disconnect and plan on reopening, *still*
// need to do this ... don't set remote_folder to null, as that will make some code paths
// think the folder is closing or closed when in fact it will be re-opening in a moment
@@ -860,31 +927,37 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// fallthrough
}
+ if (folder.open_count <= 0) {
+ debug("Not reestablishing connection to %s: closed", folder.to_string());
+
+ return;
+ }
+
// reestablish connection (which requires renormalizing the remote with the local) if
// close was in error
- if (remote_reason.is_error() || force_reestablish) {
- debug("Reestablishing broken connection to %s in %dms", folder.to_string(),
- folder.reestablish_delay_msec);
+ debug("Reestablishing broken connection to %s in %dms", folder.to_string(),
+ folder.reestablish_delay_msec);
+
+ yield Scheduler.sleep_ms_async(folder.reestablish_delay_msec);
+
+ if (folder.open_count <= 0) {
+ debug("Not reestablishing connection to %s: closed after delay", folder.to_string());
- yield Scheduler.sleep_ms_async(folder.reestablish_delay_msec);
+ return;
+ }
+
+ try {
+ // double timer now, reset to init value when cleanly opened
+ folder.reestablish_delay_msec = (folder.reestablish_delay_msec * 2).clamp(
+ DEFAULT_REESTABLISH_DELAY_MSEC, MAX_REESTABLISH_DELAY_MSEC);
- try {
- if (folder.open_count > 0) {
- // double now, reset to init value when cleanly opened
- folder.reestablish_delay_msec = (folder.reestablish_delay_msec * 2).clamp(
- DEFAULT_REESTABLISH_DELAY_MSEC, MAX_REESTABLISH_DELAY_MSEC);
-
- // since open_async() increments open_count, artificially decrement here to
- // prevent driving the value up
- folder.open_count--;
-
- yield folder.open_async(OpenFlags.NO_DELAY, null);
- } else {
- debug("%s: Not reestablishing broken connection, folder was closed", folder.to_string());
- }
- } catch (Error err) {
- debug("Error reestablishing broken connection to %s: %s", folder.to_string(), err.message);
- }
+ // since open_async() increments open_count, artificially decrement here to
+ // prevent driving the value up
+ folder.open_count = Numeric.int_floor(folder.open_count - 1, 0);
+
+ yield folder.open_async(OpenFlags.NO_DELAY, null);
+ } catch (Error err) {
+ debug("Error reestablishing broken connection to %s: %s", folder.to_string(), err.message);
}
}
@@ -1405,5 +1478,10 @@ 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());
+ }
}
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-disconnect.vala
b/src/engine/imap-engine/replay-ops/imap-engine-replay-disconnect.vala
index d277b9c..2a20990 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-replay-disconnect.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-disconnect.vala
@@ -35,10 +35,9 @@ private class Geary.ImapEngine.ReplayDisconnect : Geary.ImapEngine.ReplayOperati
// we want to encourage, so use the Idle queue to schedule close_internal_async
Idle.add(() => {
// ReplayDisconnect is only used when remote disconnects, so never flush pending, the
- // connection is down or going down, but always force reestablish connection, since
- // it wasn't our idea
+ // connection is down or going down
owner.close_internal_async.begin(Geary.Folder.CloseReason.LOCAL_CLOSE, remote_reason,
- true, false, null);
+ false, null);
return false;
});
diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala
index b7de768..42bbf71 100644
--- a/src/engine/imap/api/imap-account.vala
+++ b/src/engine/imap/api/imap-account.vala
@@ -219,8 +219,10 @@ private class Geary.Imap.Account : BaseObject {
}
}
+ // By supplying fallback STATUS, the Folder may be fetched if a network error occurs; if null,
+ // the network error is thrown
public async Imap.Folder fetch_folder_async(FolderPath path, out bool created,
- Cancellable? cancellable) throws Error {
+ StatusData? fallback_status_data, Cancellable? cancellable) throws Error {
check_open();
created = false;
@@ -245,7 +247,18 @@ private class Geary.Imap.Account : BaseObject {
Imap.Folder folder;
if (!mailbox_info.attrs.is_no_select) {
- StatusData status = yield fetch_status_async(folder_path, StatusDataType.all(), cancellable);
+ StatusData status;
+ try {
+ status = yield fetch_status_async(folder_path, StatusDataType.all(), cancellable);
+ } catch (Error err) {
+ if (fallback_status_data == null)
+ throw err;
+
+ debug("Unable to fetch STATUS for %s, using fallback from local: %s",
folder_path.to_string(),
+ err.message);
+
+ status = fallback_status_data;
+ }
folder = new Imap.Folder(folder_path, session_mgr, status, mailbox_info);
} else {
diff --git a/src/engine/imap/imap-error.vala b/src/engine/imap/imap-error.vala
index c88cc32..f9e4e32 100644
--- a/src/engine/imap/imap-error.vala
+++ b/src/engine/imap/imap-error.vala
@@ -52,5 +52,9 @@ public errordomain Geary.ImapError {
* This indicates a local time out, not one reported by the server.
*/
TIMED_OUT,
+ /**
+ * Network is unavailable.
+ */
+ UNAVAILABLE
}
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala
b/src/engine/imap/transport/imap-client-session-manager.vala
index c821d64..c84ccce 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -5,8 +5,9 @@
*/
public class Geary.Imap.ClientSessionManager : BaseObject {
- public const int DEFAULT_MIN_POOL_SIZE = 1;
- private const int AUTHORIZED_SESSION_ERROR_RETRY_TIMEOUT_MSEC = 1000;
+ private const int DEFAULT_MIN_POOL_SIZE = 1;
+ private const int AUTHORIZED_SESSION_ERROR_MIN_RETRY_TIMEOUT_SEC = 1;
+ private const int AUTHORIZED_SESSION_ERROR_MAX_RETRY_TIMEOUT_SEC = 10;
public bool is_open { get; private set; default = false; }
@@ -44,7 +45,18 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
*/
public int min_pool_size { get; set; default = DEFAULT_MIN_POOL_SIZE; }
+ /**
+ * Indicates if the { link Endpoint} the { link ClientSessionManager} connects to is reachable,
+ * according to the NetworkMonitor.
+ *
+ * By default, this is true, optimistic the network is reachable. It is updated even if the
+ * { link ClientSessionManager} is not open, maintained for the lifetime of the object.
+ */
+ public bool is_endpoint_reachable { get; private set; default = true; }
+
private AccountInformation account_information;
+ private Endpoint endpoint;
+ private NetworkMonitor network_monitor;
private Gee.HashSet<ClientSession> sessions = new Gee.HashSet<ClientSession>();
private int pending_sessions = 0;
private Nonblocking.Mutex sessions_mutex = new Nonblocking.Mutex();
@@ -52,16 +64,28 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
private bool authentication_failed = false;
private bool untrusted_host = false;
private uint authorized_session_error_retry_timeout_id = 0;
+ private int authorized_session_retry_sec = AUTHORIZED_SESSION_ERROR_MIN_RETRY_TIMEOUT_SEC;
+ private bool checking_reachable = false;
public signal void login_failed();
public ClientSessionManager(AccountInformation account_information) {
this.account_information = account_information;
+ // NOTE: This works because AccountInformation guarantees the IMAP endpoint not to change
+ // for the lifetime of the AccountInformation object; if this ever changes, will need to
+ // refactor for that
+ endpoint = account_information.get_imap_endpoint();
+ network_monitor = NetworkMonitor.get_default();
account_information.notify["imap-credentials"].connect(on_imap_credentials_notified);
- account_information.get_imap_endpoint().untrusted_host.connect(on_imap_untrusted_host);
- account_information.get_imap_endpoint().notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect(
- on_imap_trust_untrusted_host);
+ endpoint.untrusted_host.connect(on_imap_untrusted_host);
+ endpoint.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect(on_imap_trust_untrusted_host);
+
+ network_monitor.network_changed.connect(on_network_changed);
+ network_monitor.notify["network-available"].connect(on_network_available_changed);
+
+ // get this started now
+ check_endpoint_reachable(null);
}
~ClientSessionManager() {
@@ -69,9 +93,11 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
warning("Destroying opened ClientSessionManager");
account_information.notify["imap-credentials"].disconnect(on_imap_credentials_notified);
- account_information.get_imap_endpoint().untrusted_host.disconnect(on_imap_untrusted_host);
- account_information.get_imap_endpoint().notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].disconnect(
- on_imap_trust_untrusted_host);
+ endpoint.untrusted_host.disconnect(on_imap_untrusted_host);
+ endpoint.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].disconnect(on_imap_trust_untrusted_host);
+
+ network_monitor.network_changed.disconnect(on_network_changed);
+ network_monitor.notify["network-available"].disconnect(on_network_available_changed);
}
public async void open_async(Cancellable? cancellable) throws Error {
@@ -145,8 +171,13 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
return;
}
- while ((sessions.size + pending_sessions) < min_pool_size && !authentication_failed && is_open &&
!untrusted_host)
+ while ((sessions.size + pending_sessions) < min_pool_size
+ && !authentication_failed
+ && is_open
+ && !untrusted_host
+ && is_endpoint_reachable) {
schedule_new_authorized_session();
+ }
try {
sessions_mutex.release(ref token);
@@ -167,15 +198,17 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
try {
create_new_authorized_session.end(result);
} catch (Error err) {
- debug("Unable to create authorized session to %s: %s",
- account_information.get_imap_endpoint().to_string(), err.message);
+ debug("Unable to create authorized session to %s: %s", endpoint.to_string(), err.message);
- // try again after a slight delay
+ // try again after a slight delay and bump up delay
if (authorized_session_error_retry_timeout_id != 0)
Source.remove(authorized_session_error_retry_timeout_id);
- authorized_session_error_retry_timeout_id
- = Timeout.add(AUTHORIZED_SESSION_ERROR_RETRY_TIMEOUT_MSEC,
- on_authorized_session_error_retry_timeout);
+
+ authorized_session_error_retry_timeout_id = Timeout.add_seconds(
+ authorized_session_retry_sec, on_authorized_session_error_retry_timeout);
+
+ authorized_session_retry_sec = (authorized_session_retry_sec * 2).clamp(
+ AUTHORIZED_SESSION_ERROR_MIN_RETRY_TIMEOUT_SEC,
AUTHORIZED_SESSION_ERROR_MAX_RETRY_TIMEOUT_SEC);
}
}
@@ -194,9 +227,12 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
throw new ImapError.UNAUTHENTICATED("Invalid ClientSessionManager credentials");
if (untrusted_host)
- throw new ImapError.UNAUTHENTICATED("Untrusted host %s",
account_information.get_imap_endpoint().to_string());
+ throw new ImapError.UNAUTHENTICATED("Untrusted host %s", endpoint.to_string());
- ClientSession new_session = new ClientSession(account_information.get_imap_endpoint());
+ if (!is_endpoint_reachable)
+ throw new ImapError.UNAVAILABLE("Host at %s is unreachable", endpoint.to_string());
+
+ ClientSession new_session = new ClientSession(endpoint);
// add session to pool before launching all the connect activity so error cases can properly
// back it out
@@ -244,6 +280,9 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
throw err;
}
+ // reset delay
+ authorized_session_retry_sec = AUTHORIZED_SESSION_ERROR_MIN_RETRY_TIMEOUT_SEC;
+
// do this after logging in
new_session.enable_keepalives(selected_keepalive_sec, unselected_keepalive_sec,
selected_with_idle_keepalive_sec);
@@ -448,7 +487,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
private void on_imap_trust_untrusted_host() {
// fired when the trust_untrusted_host property changes, indicating if the user has agreed
// to ignore the trust problems and continue connecting
- if (untrusted_host && account_information.get_imap_endpoint().trust_untrusted_host == Trillian.TRUE)
{
+ if (untrusted_host && endpoint.trust_untrusted_host == Trillian.TRUE) {
untrusted_host = false;
if (is_open)
@@ -456,11 +495,59 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
}
}
+ private void on_network_changed() {
+ // Always check if reachable because IMAP server could be on localhost. (This is a Linux
+ // program, after all...)
+ check_endpoint_reachable(null);
+ }
+
+ private void on_network_available_changed() {
+ // If network is available and endpoint is reachable, do nothing more, all is good,
+ // otherwise check (see note in on_network_changed)
+ if (network_monitor.network_available && is_endpoint_reachable)
+ return;
+
+ check_endpoint_reachable(null);
+ }
+
+ private void check_endpoint_reachable(Cancellable? cancellable) {
+ if (checking_reachable)
+ return;
+
+ debug("Checking if IMAP host %s reachable...", endpoint.to_string());
+
+ checking_reachable = true;
+ check_endpoint_reachable_async.begin(cancellable);
+ }
+
+ // Use check_endpoint_reachable to properly schedule
+ private async void check_endpoint_reachable_async(Cancellable? cancellable) {
+ try {
+ is_endpoint_reachable = yield network_monitor.can_reach_async(endpoint.remote_address,
+ cancellable);
+ message("IMAP host %s considered %s", endpoint.to_string(),
+ is_endpoint_reachable ? "reachable" : "unreachable");
+ } catch (Error err) {
+ // If cancelled, don't change anything
+ if (err is IOError.CANCELLED)
+ return;
+
+ message("Error determining if IMAP host %s is reachable, treating as unreachable: %s",
+ endpoint.to_string(), err.message);
+ is_endpoint_reachable = false;
+ } finally {
+ checking_reachable = false;
+ }
+
+ if (is_endpoint_reachable)
+ adjust_session_pool.begin();
+ }
+
/**
* Use only for debugging and logging.
*/
public string to_string() {
- return account_information.get_imap_endpoint().to_string();
+ return endpoint.to_string();
}
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
