[geary] Handle reconnects inside of ImapEngine.GenericFolder
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Handle reconnects inside of ImapEngine.GenericFolder
- Date: Tue, 26 Nov 2013 01:22:57 +0000 (UTC)
commit 1a48ccc564f4701352e033baa420ccae0b06c4e9
Author: Jim Nelson <jim yorba org>
Date: Mon Nov 25 17:19:09 2013 -0800
Handle reconnects inside of ImapEngine.GenericFolder
For historical reasons, reestablishing connections when a folder's
session died was handled inside of ConversationMonitor. This broke
at some point and Geary failed to reconnect when the session dropped.
This patch puts this logic into ImapEngine.GenericFolder, where it
belongs, as well as fixes a problem in ClientSession that caused
issues when the session was closed by the server (via a BYE response).
This patch fixes bug #713609 and bug #714532. I also believe this
fixes bug #713078. Additionally, with the reconnect logic now in
GenericFolder, bug #714671 is solved.
src/engine/app/app-conversation-monitor.vala | 111 --------------------
.../imap-engine/imap-engine-generic-folder.vala | 29 ++++--
src/engine/imap/transport/imap-client-session.vala | 60 ++++-------
3 files changed, 42 insertions(+), 158 deletions(-)
---
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index f60bef5..55e20a6 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -44,9 +44,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
private Geary.Email.Field required_fields;
private Geary.Folder.OpenFlags open_flags;
private Cancellable? cancellable_monitor = null;
- private bool retry_connection = false;
- private int64 last_retry_time = 0;
- private uint retry_id = 0;
private bool reseed_notified = false;
private int _min_window_count = 0;
private ConversationOperationQueue operation_queue = new ConversationOperationQueue();
@@ -188,8 +185,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
this.open_flags = open_flags;
this.required_fields = required_fields | REQUIRED_FIELDS;
_min_window_count = min_window_count;
-
- folder.account.information.notify["imap-credentials"].connect(on_imap_credentials_notified);
}
~ConversationMonitor() {
@@ -198,8 +193,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
// Manually detach all the weak refs in the Conversation objects
conversations.clear_owners();
-
- folder.account.information.notify["imap-credentials"].disconnect(on_imap_credentials_notified);
}
protected virtual void notify_monitoring_started() {
@@ -291,7 +284,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
folder.email_inserted.connect(on_folder_email_inserted);
folder.email_removed.connect(on_folder_email_removed);
folder.opened.connect(on_folder_opened);
- folder.closed.connect(on_folder_closed);
folder.account.email_flags_changed.connect(on_account_email_flags_changed);
folder.account.email_locally_complete.connect(on_account_email_locally_complete);
// TODO: handle removed email
@@ -305,7 +297,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
folder.email_inserted.disconnect(on_folder_email_inserted);
folder.email_removed.disconnect(on_folder_email_removed);
folder.opened.disconnect(on_folder_opened);
- folder.closed.disconnect(on_folder_closed);
folder.account.email_flags_changed.disconnect(on_account_email_flags_changed);
folder.account.email_locally_complete.disconnect(on_account_email_locally_complete);
@@ -343,9 +334,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
private async void stop_monitoring_internal_async(bool close_folder, bool retrying,
Cancellable? cancellable) throws Error {
- // always unschedule, as Timeout will hold a reference to this object
- unschedule_retry();
-
if (!is_monitoring)
return;
@@ -358,7 +346,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
folder.email_inserted.disconnect(on_folder_email_inserted);
folder.email_removed.disconnect(on_folder_email_removed);
folder.opened.disconnect(on_folder_opened);
- folder.closed.disconnect(on_folder_closed);
folder.account.email_flags_changed.disconnect(on_account_email_flags_changed);
folder.account.email_locally_complete.disconnect(on_account_email_locally_complete);
@@ -755,104 +742,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
operation_queue.add(new ReseedOperation(this, state.to_string()));
}
- private void on_folder_closed(Folder.CloseReason reason) {
- debug("Folder %s close reason %s", folder.to_string(), reason.to_string());
-
- // watch for errors; these indicate a retry should occur
- if (reason.is_error() && reestablish_connections)
- retry_connection = true;
-
- // wait for the folder to be completely closed before retrying
- if (reason != Folder.CloseReason.FOLDER_CLOSED)
- return;
-
- if (!retry_connection) {
- debug("Folder %s closed normally, not reestablishing connection", folder.to_string());
-
- stop_monitoring_internal_async.begin(false, false, null);
-
- return;
- }
-
- // reset
- retry_connection = false;
-
- debug("Folder %s closed due to error, reestablishing connection to continue monitoring
conversations",
- folder.to_string());
-
- schedule_retry();
- }
-
- private async void do_restart_monitoring_async() {
- last_retry_time = get_monotonic_time();
-
- try {
- debug("Restarting conversation monitoring of folder %s, stopping previous monitoring...",
- folder.to_string());
- yield stop_monitoring_internal_async(false, true, null);
- } catch (Error stop_err) {
- debug("Error closing folder %s while reestablishing connection: %s", folder.to_string(),
- stop_err.message);
- }
-
- // TODO: Get smarter about this, especially since this might be an authentication error
- // and not a hard error
- debug("Restarting conversation monitoring of folder %s...", folder.to_string());
- try {
- if (!yield start_monitoring_async(cancellable_monitor))
- debug("Unable to restart monitoring of %s: already monitoring", folder.to_string());
- else
- debug("Reestablished connection to %s, continuing to monitor conversations",
- folder.to_string());
- } catch (Error start_err) {
- debug("Unable to reestablish connection to %s, retrying in %d seconds: %s", folder.to_string(),
- RETRY_CONNECTION_SEC, start_err.message);
-
- schedule_retry();
- }
- }
-
- // If we've got a pending retry and the folder's account's password is
- // updated, cancel the pending retry and retry now. This prevents the user
- // waiting while nothing happens after they type in their password.
- private void on_imap_credentials_notified() {
- if (retry_id == 0)
- return;
- unschedule_retry();
- do_restart_monitoring_async.begin();
- }
-
- private void schedule_retry() {
- if (retry_id != 0)
- return;
-
- // Number of us in the future we can schedule a retry, so we have a
- // minimum of RETRY_CONNECTION_SEC s between retries.
- int64 next_retry_time = RETRY_CONNECTION_SEC * 1000000 - (get_monotonic_time() - last_retry_time);
-
- // If it's been enough time since last retry we can retry immediately
- // (note we schedule on ms, translated from us).
- if (next_retry_time <= 0)
- do_restart_monitoring_async.begin();
- else
- retry_id = Timeout.add((uint) (next_retry_time / 1000), on_delayed_retry);
- }
-
- private void unschedule_retry() {
- if (retry_id != 0) {
- Source.remove(retry_id);
- retry_id = 0;
- }
- }
-
- private bool on_delayed_retry() {
- retry_id = 0;
-
- do_restart_monitoring_async.begin();
-
- return false;
- }
-
/**
* Attempts to load enough conversations to fill min_window_count.
*/
diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala
b/src/engine/imap-engine/imap-engine-generic-folder.vala
index d39b089..026a1f4 100644
--- a/src/engine/imap-engine/imap-engine-generic-folder.vala
+++ b/src/engine/imap-engine/imap-engine-generic-folder.vala
@@ -443,6 +443,13 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
}
this.open_flags = open_flags;
+
+ open_internal(cancellable);
+
+ return true;
+ }
+
+ private void open_internal(Cancellable? cancellable) {
remote_semaphore = new Geary.Nonblocking.ReportingSemaphore<bool>(false);
// start the replay queue
@@ -460,8 +467,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
// meaning that folder normalization never happens and unsolicited notifications never
// arrive
start_remote_open_timer();
-
- return true;
}
private void start_remote_open_timer() {
@@ -590,9 +595,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
// NOTE: This bypasses open_count and forces the Folder closed.
internal async void close_internal_async(Folder.CloseReason local_reason, Folder.CloseReason
remote_reason,
Cancellable? cancellable) {
- // force closed
- open_count = 0;
-
cancel_remote_open_timer();
// only flushing pending ReplayOperations if this is a "clean" close, not forced due to
@@ -640,6 +642,21 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
closing_remote_folder.close_async.begin(cancellable);
}
+ remote_opened = false;
+
+ // reestablish connection (which requires renormalizing the remote with the local) if
+ // close was in error
+ if (remote_reason.is_error()) {
+ debug("Reestablishing broken connect to %s", to_string());
+
+ open_internal(null);
+
+ return;
+ }
+
+ // forced closed one way or another
+ open_count = 0;
+
// use remote_reason even if remote_folder was null; it could be that the error occurred
// while opening and remote_folder was yet unassigned ... also, need to call this every
// time, even if remote was not fully opened, as some callers rely on order of signals
@@ -648,8 +665,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
// see above note for why this must be called every time
notify_closed(local_reason);
- remote_opened = false;
-
notify_closed(CloseReason.FOLDER_CLOSED);
debug("Folder %s closed", to_string());
diff --git a/src/engine/imap/transport/imap-client-session.vala
b/src/engine/imap/transport/imap-client-session.vala
index df7c8d4..85ddfeb 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -335,7 +335,7 @@ public class Geary.Imap.ClientSession : BaseObject {
new Geary.State.Mapping(State.LOGGING_OUT, Event.CLOSE_MAILBOX, on_late_command),
new Geary.State.Mapping(State.LOGGING_OUT, Event.LOGOUT, Geary.State.nop),
new Geary.State.Mapping(State.LOGGING_OUT, Event.DISCONNECT, on_disconnect),
- new Geary.State.Mapping(State.LOGGING_OUT, Event.RECV_STATUS, on_recv_disconnecting_status),
+ new Geary.State.Mapping(State.LOGGING_OUT, Event.RECV_STATUS, on_dropped_response),
new Geary.State.Mapping(State.LOGGING_OUT, Event.RECV_COMPLETION,
on_logging_out_recv_completion),
new Geary.State.Mapping(State.LOGGING_OUT, Event.RECV_ERROR, on_disconnected),
new Geary.State.Mapping(State.LOGGING_OUT, Event.SEND_ERROR, on_send_error),
@@ -347,7 +347,7 @@ public class Geary.Imap.ClientSession : BaseObject {
new Geary.State.Mapping(State.LOGGED_OUT, Event.CLOSE_MAILBOX, on_late_command),
new Geary.State.Mapping(State.LOGGED_OUT, Event.LOGOUT, on_late_command),
new Geary.State.Mapping(State.LOGGED_OUT, Event.DISCONNECT, on_disconnect),
- new Geary.State.Mapping(State.LOGGED_OUT, Event.RECV_STATUS, on_recv_disconnecting_status),
+ new Geary.State.Mapping(State.LOGGED_OUT, Event.RECV_STATUS, on_dropped_response),
new Geary.State.Mapping(State.LOGGED_OUT, Event.RECV_COMPLETION, on_dropped_response),
new Geary.State.Mapping(State.LOGGED_OUT, Event.RECV_ERROR, on_disconnected),
new Geary.State.Mapping(State.LOGGED_OUT, Event.SEND_ERROR, on_send_error),
@@ -574,22 +574,21 @@ public class Geary.Imap.ClientSession : BaseObject {
private void drop_connection() {
unschedule_keepalive();
- if (cx == null)
- return;
-
- cx.connected.disconnect(on_network_connected);
- cx.disconnected.disconnect(on_network_disconnected);
- cx.sent_command.disconnect(on_network_sent_command);
- cx.send_failure.disconnect(on_network_send_error);
- cx.received_status_response.disconnect(on_received_status_response);
- cx.received_server_data.disconnect(on_received_server_data);
- cx.received_bytes.disconnect(on_received_bytes);
- cx.received_bad_response.disconnect(on_received_bad_response);
- cx.recv_closed.disconnect(on_received_closed);
- cx.receive_failure.disconnect(on_network_receive_failure);
- cx.deserialize_failure.disconnect(on_network_receive_failure);
-
- cx = null;
+ if (cx != null) {
+ cx.connected.disconnect(on_network_connected);
+ cx.disconnected.disconnect(on_network_disconnected);
+ cx.sent_command.disconnect(on_network_sent_command);
+ cx.send_failure.disconnect(on_network_send_error);
+ cx.received_status_response.disconnect(on_received_status_response);
+ cx.received_server_data.disconnect(on_received_server_data);
+ cx.received_bytes.disconnect(on_received_bytes);
+ cx.received_bad_response.disconnect(on_received_bad_response);
+ cx.recv_closed.disconnect(on_received_closed);
+ cx.receive_failure.disconnect(on_network_receive_failure);
+ cx.deserialize_failure.disconnect(on_network_receive_failure);
+
+ cx = null;
+ }
// if there are any outstanding commands waiting for responses, wake them up now
if (waiting_for_completion.size > 0) {
@@ -901,15 +900,12 @@ public class Geary.Imap.ClientSession : BaseObject {
}
private void on_keepalive_completed(Object? source, AsyncResult result) {
- StatusResponse response;
try {
- response = send_command_async.end(result);
+ StatusResponse response = send_command_async.end(result);
Logging.debug(Logging.Flag.PERIODIC, "[%s] Keepalive result: %s", to_string(),
response.to_string());
} catch (Error err) {
debug("[%s] Keepalive error: %s", to_string(), err.message);
-
- return;
}
}
@@ -1014,11 +1010,8 @@ public class Geary.Imap.ClientSession : BaseObject {
break;
case Status.BYE:
- // this is REMOTE_ERROR because it occurs in a state where the client didn't
- // expect to go away (see on_recv_disconnecting_status)
- fsm.do_post_transition(() => { disconnected(DisconnectReason.REMOTE_ERROR); });
-
- return State.BROKEN;
+ debug("[%s] Received BYE from server: %s", to_string(), status_response.to_string());
+ break;
default:
debug("[%s] Received error from server: %s", to_string(), status_response.to_string());
@@ -1028,19 +1021,6 @@ public class Geary.Imap.ClientSession : BaseObject {
return state;
}
- private uint on_recv_disconnecting_status(uint state, uint event, void *user, Object? object) {
- StatusResponse status_response = (StatusResponse) object;
-
- switch (status_response.status) {
- case Status.BYE:
- fsm.do_post_transition(() => { disconnected(DisconnectReason.REMOTE_CLOSE); });
-
- return State.BROKEN;
- }
-
- return state;
- }
-
//
// select/examine
//
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
