Use the ConnectivityManager to detect connection problems up front.

* src/engine/util/util-connectivity-manager.vala (ConnectivityManager):
  Work with a NetworkAddress rather than an Endpoint to simplify a
  bit. Use trillian for is-reachable property, since that's more
  realistic Add new is-valid property and address_error_reported signal,
  update and fire these when the reachability check reports an error
  rather than just not reachable. Don't treat transient DNS errors as an
  error, however. Update call sites.

* src/engine/imap-db/outbox/smtp-outbox-folder.vala (SmtpOutboxFolder),
  src/engine/imap/transport/imap-client-session-manager.vala
  (ClientSessionManager): Look up to new address_error_reported signal
  and clean up then report the problem when it is fired.
This commit is contained in:
Michael James Gratton 2017-11-14 00:46:52 +11:00
parent 36092d4a9c
commit 0d1efc2fe0
4 changed files with 97 additions and 33 deletions

View file

@ -127,7 +127,7 @@ public class Geary.Endpoint : BaseObject {
this.remote_address = new NetworkAddress(host_specifier, default_port);
this.flags = flags;
this.timeout_sec = timeout_sec;
this.connectivity = new ConnectivityManager(this);
this.connectivity = new ConnectivityManager(this.remote_address);
}
private SocketClient get_socket_client() {

View file

@ -884,7 +884,8 @@ private class Geary.SmtpOutboxFolder :
private void on_account_opened() {
this.fill_outbox_queue.begin();
this.smtp_endpoint.connectivity.notify["is-reachable"].connect(on_reachable_changed);
if (this.smtp_endpoint.connectivity.is_reachable) {
this.smtp_endpoint.connectivity.address_error_reported.connect(on_connectivity_error);
if (this.smtp_endpoint.connectivity.is_reachable.is_certain()) {
this.start_timer.start();
} else {
this.smtp_endpoint.connectivity.check_reachable.begin();
@ -894,18 +895,23 @@ private class Geary.SmtpOutboxFolder :
private void on_account_closed() {
this.stop_postman();
this.smtp_endpoint.connectivity.notify["is-reachable"].disconnect(on_reachable_changed);
this.smtp_endpoint.connectivity.address_error_reported.disconnect(on_connectivity_error);
}
private void on_reachable_changed() {
if (this.smtp_endpoint.connectivity.is_reachable) {
if (this.smtp_endpoint.connectivity.is_reachable.is_certain()) {
if (this.queue_cancellable == null) {
this.start_timer.start();
}
} else {
this.start_timer.reset();
if (this.queue_cancellable != null) {
stop_postman();
}
stop_postman();
}
}
private void on_connectivity_error(Error error) {
stop_postman();
notify_report_problem(ProblemType.CONNECTION_ERROR, error);
}
}

View file

@ -123,7 +123,8 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
this.authentication_failed = false;
this.endpoint.connectivity.notify["is-reachable"].connect(on_connectivity_change);
if (this.endpoint.connectivity.is_reachable) {
this.endpoint.connectivity.address_error_reported.connect(on_connectivity_error);
if (this.endpoint.connectivity.is_reachable.is_certain()) {
this.adjust_session_pool.begin();
} else {
this.endpoint.connectivity.check_reachable.begin();
@ -141,6 +142,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
this.pool_stop.reset();
this.endpoint.connectivity.notify["is-reachable"].disconnect(on_connectivity_change);
this.endpoint.connectivity.address_error_reported.disconnect(on_connectivity_error);
// to avoid locking down the sessions table while scheduling disconnects, make a copy
// and work off of that
@ -204,7 +206,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
&& this.is_open
&& !this.authentication_failed
&& !this.untrusted_host
&& this.endpoint.connectivity.is_reachable) {
&& this.endpoint.connectivity.is_reachable.is_certain()) {
this.pending_sessions++;
create_new_authorized_session.begin(
null,
@ -232,9 +234,9 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
if (untrusted_host)
throw new ImapError.UNAVAILABLE("Untrusted host %s", endpoint.to_string());
if (!this.endpoint.connectivity.is_reachable)
if (!this.endpoint.connectivity.is_reachable.is_certain())
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
@ -546,7 +548,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
}
private void on_connectivity_change() {
bool is_reachable = this.endpoint.connectivity.is_reachable;
bool is_reachable = this.endpoint.connectivity.is_reachable.is_certain();
if (is_reachable) {
this.pool_start.start();
this.pool_stop.reset();
@ -558,6 +560,13 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
}
}
private void on_connectivity_error(Error error) {
this.is_ready = false;
this.pool_start.reset();
this.pool_stop.start();
connection_failed(error);
}
/**
* Use only for debugging and logging.
*/

View file

@ -6,13 +6,13 @@
*/
/**
* Keeps track of network connectivity changes for an endpoint.
* Keeps track of network connectivity changes for a network address.
*
* 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
* when an address becomes reachable, and once when it becomes
* unreachable.
*
* Note this class is not thread safe and should only be invoked from
@ -20,11 +20,19 @@
*/
public class Geary.ConnectivityManager : BaseObject {
/** Determines if the managed endpoint is currently reachable. */
public bool is_reachable { get; private set; default = false; }
/** The address being monitored. */
public NetworkAddress address { get; private set; default = null; }
// Weak to avoid a circular ref with the endpoint
private weak Endpoint endpoint;
/** Determines if the managed address is currently reachable. */
public Trillian is_reachable { get; private set; default = Geary.Trillian.UNKNOWN; }
/**
* Determines if a the address's network address name is valid.
*
* This will become certain if the address becomes reachable, and
* will become impossible if a fatal address error is reported.
*/
public Trillian is_valid { get; private set; default = Geary.Trillian.UNKNOWN; }
private NetworkMonitor monitor;
@ -32,10 +40,21 @@ public class Geary.ConnectivityManager : BaseObject {
/**
* Constructs a new manager for a specific endpoint.
* Fired when a fatal error was reported checking the address.
*
* This is typically caused by an an authoritative DNS name not
* found error, but may be anything else that indicates that the
* address will be unusable as-is without some kind of user or
* server administrator intervention.
*/
public ConnectivityManager(Endpoint endpoint) {
this.endpoint = endpoint;
public signal void address_error_reported(Error error);
/**
* Constructs a new manager for a specific address.
*/
public ConnectivityManager(NetworkAddress address) {
this.address = address;
this.monitor = NetworkMonitor.get_default();
this.monitor.network_changed.connect(on_network_changed);
@ -46,7 +65,7 @@ public class Geary.ConnectivityManager : BaseObject {
}
/**
* Starts checking if the manager's endpoint is reachable.
* Starts checking if the manager's address is reachable.
*
* This will cancel any existing check, and start a new one
* running, updating the `is_reachable` property on completion.
@ -63,14 +82,11 @@ public class Geary.ConnectivityManager : BaseObject {
Cancellable cancellable = new Cancellable();
this.existing_check = cancellable;
string endpoint = this.endpoint.to_string();
bool is_reachable = this.is_reachable;
string endpoint = to_address_string();
bool is_reachable = false;
try {
debug("Checking if %s reachable...", endpoint);
is_reachable = yield this.monitor.can_reach_async(
this.endpoint.remote_address,
cancellable
);
is_reachable = yield this.monitor.can_reach_async(this.address, cancellable);
} catch (Error err) {
if (err is IOError.NETWORK_UNREACHABLE &&
this.monitor.network_available) {
@ -81,11 +97,19 @@ public class Geary.ConnectivityManager : BaseObject {
is_reachable = true;
debug("Assuming %s is reachable, despite network unavailability",
endpoint);
} else if (err is ResolverError.TEMPORARY_FAILURE) {
// This often happens when networking is coming back
// online, may because the interface is up but has not
// been assigned an address yet? Since we should get
// another network change when the interface is
// configured, just ignore it.
debug("Ignoring: %s", err.message);
} else if (!(err is IOError.CANCELLED)) {
// Service is unreachable
debug("Error checking %s reachable, treating as unreachable: %s",
endpoint, err.message);
is_reachable = false;
endpoint, err.message);
set_invalid();
address_error_reported(err);
}
} finally {
if (!cancellable.is_cancelled()) {
@ -123,13 +147,38 @@ public class Geary.ConnectivityManager : BaseObject {
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
// change. 0.36 fixes that, so pull this test out when we can
// depend on that as a minimum.
if (this.is_reachable != reachable) {
if ((reachable && !this.is_reachable.is_certain()) ||
(!reachable && !this.is_reachable.is_impossible())) {
debug("Host %s became %s",
this.endpoint.to_string(), reachable ? "reachable" : "unreachable");
this.is_reachable = reachable;
this.address.to_string(), reachable ? "reachable" : "unreachable");
this.is_reachable = reachable ? Trillian.TRUE : Trillian.FALSE;
}
// We only work out if the name is valid (or becomes valid
// again) if the address becomes reachable.
if (reachable && this.is_valid.is_uncertain()) {
this.is_valid = Trillian.TRUE;
}
}
private inline void set_invalid() {
// 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 method out when we can
// depend on that as a minimum.
if (this.is_valid != Trillian.FALSE) {
this.is_valid = Trillian.FALSE;
}
}
private inline string to_address_string() {
// Unlikely to be the case, but if IPv6 format it nicely
return (this.address.hostname.index_of(":") == -1)
? "%s:%u".printf(this.address.hostname, this.address.port)
: "[%s]:%u".printf(this.address.hostname, this.address.port);
}
}