[geary] Clean up network monitor handling.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Clean up network monitor handling.
- Date: Tue, 26 Sep 2017 08:30:11 +0000 (UTC)
commit 0b156120a76ece5c721f3755ebb3dfeb64406026
Author: Michael James Gratton <mike vee net>
Date: Tue Sep 26 15:34:23 2017 +1000
Clean up network monitor handling.
Also works around Bug 788170 when running under flatpak.
* src/engine/imap/transport/imap-client-session-manager.vala: Break out
code that uses Glib.NetworkMonitor into a seperate util class, and hook
that up.
* src/engine/util/util-connectivity-manager.vala: New util class that
coalases network change events, to ensure the engine is only notified
once when an enpoint becomes reachable or unreachable.
src/CMakeLists.txt | 1 +
.../transport/imap-client-session-manager.vala | 112 +++++------------
src/engine/util/util-connectivity-manager.vala | 132 ++++++++++++++++++++
3 files changed, 167 insertions(+), 78 deletions(-)
---
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 64c541e..1e4213f 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -291,6 +291,7 @@ engine/state/state-mapping.vala
engine/util/util-ascii.vala
engine/util/util-collection.vala
+engine/util/util-connectivity-manager.vala
engine/util/util-converter.vala
engine/util/util-files.vala
engine/util/util-generic-capabilities.vala
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala
b/src/engine/imap/transport/imap-client-session-manager.vala
index 45b9338..6bba536 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -47,16 +47,16 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
/**
* Indicates if the {@link Endpoint} the {@link ClientSessionManager} connects to is reachable,
- * according to the NetworkMonitor.
+ * according to NetworkMonitor.
*
- * By default, this is true, optimistic the network is reachable. It is updated even if the
+ * By default, this is false, pessimistic that 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; }
-
+ public bool is_endpoint_reachable { get; private set; default = false; }
+
private AccountInformation account_information;
private Endpoint endpoint;
- private NetworkMonitor network_monitor;
+ private ConnectivityManager connectivity;
private Gee.HashSet<ClientSession> sessions = new Gee.HashSet<ClientSession>();
private int pending_sessions = 0;
private Nonblocking.Mutex sessions_mutex = new Nonblocking.Mutex();
@@ -65,41 +65,36 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
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(StatusResponse? response);
-
+
public ClientSessionManager(AccountInformation account_information) {
this.account_information = account_information;
+ this.account_information.notify["imap-credentials"].connect(on_imap_credentials_notified);
+
// 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);
- 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);
+ this.endpoint = account_information.get_imap_endpoint();
+ this.endpoint.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect(on_imap_trust_untrusted_host);
+ this.endpoint.untrusted_host.connect(on_imap_untrusted_host);
+
+ this.connectivity = new ConnectivityManager(this.endpoint);
+ this.connectivity.notify["is-reachable"].connect(on_connectivity_change);
+ this.connectivity.check_reachable.begin();
}
-
+
~ClientSessionManager() {
if (is_open)
warning("Destroying opened ClientSessionManager");
-
+
account_information.notify["imap-credentials"].disconnect(on_imap_credentials_notified);
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);
+ this.connectivity.cancel_check();
+ this.connectivity = null;
}
-
+
public async void open_async(Cancellable? cancellable) throws Error {
if (is_open)
throw new EngineError.ALREADY_OPEN("ClientSessionManager already open");
@@ -156,9 +151,9 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
// TODO: Need a more thorough and bulletproof system for maintaining a pool of ready
// authorized sessions.
private async void adjust_session_pool() {
- if (!is_open)
+ if (!this.is_open)
return;
-
+
int token;
try {
token = yield sessions_mutex.claim_async();
@@ -502,55 +497,17 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
adjust_session_pool.begin();
}
}
-
- 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();
- }
-
+
+ private void on_connectivity_change() {
+ this.is_endpoint_reachable = this.connectivity.is_reachable;
+ debug("Host %s became %s",
+ this.endpoint.to_string(),
+ this.is_endpoint_reachable ? "reachable" : "unreachable");
+ if (this.is_endpoint_reachable) {
+ this.adjust_session_pool.begin();
+ }
+ }
+
/**
* Use only for debugging and logging.
*/
@@ -559,4 +516,3 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
sessions.size, reserved_sessions.size);
}
}
-
diff --git a/src/engine/util/util-connectivity-manager.vala b/src/engine/util/util-connectivity-manager.vala
new file mode 100644
index 0000000..22e8e50
--- /dev/null
+++ b/src/engine/util/util-connectivity-manager.vala
@@ -0,0 +1,132 @@
+/*
+ * Copyright 2017 Michael Gratton <mike vee net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+/**
+ * Keeps track of network connectivity changes for an endpoint.
+ *
+ * This class is a convenience API for the GIO NetworkMonitor. Since
+ * when connecting and disconnecting from a network, multiple
+ * network-changed signals may be sent, this class coalesces these as
+ * best as possible so the rest of the engine is only notified once
+ * when an endpoint becomes reachable, and once when it becomes
+ * unreachable.
+ *
+ * Note this class is not thread safe and should only be invoked from
+ * the main loop.
+ */
+public class Geary.ConnectivityManager : BaseObject {
+
+ /** Determines if the managed endpoint is currently reachable. */
+ public bool is_reachable { get; private set; default = false; }
+
+ private Endpoint endpoint;
+
+ private NetworkMonitor monitor;
+
+ private Cancellable? existing_check = null;
+
+
+ /**
+ * Constructs a new manager for a specific endpoint.
+ */
+ public ConnectivityManager(Endpoint endpoint) {
+ this.endpoint = endpoint;
+
+ this.monitor = NetworkMonitor.get_default();
+ this.monitor.network_changed.connect(on_network_changed);
+ }
+
+ ~ConnectivityManager() {
+ this.monitor.network_changed.disconnect(on_network_changed);
+ }
+
+ /**
+ * Starts checking if the manager's endpoint is reachable.
+ *
+ * This will cancel any existing check, and start a new one
+ * running, updating the `is_reachable` property on completion.
+ */
+ public async void check_reachable() {
+ // We use a cancellable here as a guard instead of a boolean
+ // "is_checking" var since when a series of checks are
+ // requested in quick succession (as is the case when
+ // e.g. connecting or disconnecting from a network), the
+ // result of the *last* check is authoritative, not the first
+ // one.
+ cancel_check();
+
+ Cancellable cancellable = new Cancellable();
+ this.existing_check = cancellable;
+
+ string endpoint = this.endpoint.to_string();
+ bool is_reachable = this.is_reachable;
+ try {
+ debug("Checking if %s reachable...", endpoint);
+ is_reachable = yield this.monitor.can_reach_async(
+ this.endpoint.remote_address,
+ cancellable
+ );
+ } catch (Error err) {
+ if (err is IOError.NETWORK_UNREACHABLE &&
+ this.monitor.network_available) {
+ // If we get a network unreachable error, but the monitor
+ // says there actually is a network available, we may be
+ // running in a Flatpak and hitting Bug 777706. If so,
+ // just assume the service is reachable is for now. :(
+ is_reachable = true;
+ debug("Assuming %s is reachable, despite network unavailability",
+ endpoint);
+ } else if (!(err is IOError.CANCELLED)) {
+ // Service is unreachable
+ debug("Error checking %s reachable, treating as unreachable: %s",
+ endpoint, err.message);
+ is_reachable = false;
+ }
+ } finally {
+ if (!cancellable.is_cancelled()) {
+ set_reachable(is_reachable);
+ }
+ this.existing_check = null;
+ }
+ }
+
+ /**
+ * Cancels any running reachability check, if any.
+ */
+ public void cancel_check() {
+ if (this.existing_check != null) {
+ this.existing_check.cancel();
+ this.existing_check = null;
+ }
+ }
+
+ private void on_network_changed(bool some_available) {
+ // Always check if reachable because IMAP server could be on
+ // localhost. (This is a Linux program, after all...)
+ debug("Network changed: %s",
+ some_available ? "some available" : "none available");
+ if (some_available) {
+ // Some hosts may have dropped out despite network being
+ // still xavailable, so need to check again
+ this.check_reachable.begin();
+ } else {
+ // None available, so definitely not reachable
+ set_reachable(false);
+ }
+ }
+
+ private inline void set_reachable(bool reachable) {
+ // Coalesce changes to is_reachable, since Vala <= 0.34 always
+ // fires notify signals on set, even if the value doesn't
+ // change. 0.36 fixes that, so pull this out when we can
+ // depend on that as a minimum.
+ if (this.is_reachable != reachable) {
+ this.is_reachable = reachable;
+ }
+ }
+
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
