[geary] Limit the number of pipelined STATUS commands sent to outloook.com.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Limit the number of pipelined STATUS commands sent to outloook.com.
- Date: Wed, 8 Jun 2016 06:45:41 +0000 (UTC)
commit 1a87d0d16decd79c16322676c24ff326fd52b9f2
Author: Michael James Gratton <mike vee net>
Date: Wed Jun 8 00:25:01 2016 +1000
Limit the number of pipelined STATUS commands sent to outloook.com.
Outlook.com's IMAP servers are buggy and will return a bogus BAD if too
many STATUS commands are sent at once. Limit this to 25 since there is at
least one known case of it falling over when ~50 are sent.
Bug 766552.
* src/engine/api/geary-endpoint.vala (Endpoint): Add
max_pipeline_batch_size property and doc comment.
* src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
(OutlookAccount): Set max_pipeline_batch_size for IMAP connection to 25.
* src/engine/imap/transport/imap-client-session.vala
(ClientSession::send_multiple_commands_async): Check
Endpoint.max_pipeline_batch_size and if non-zero, use multiple batches
and limite them to that value in size.
src/engine/api/geary-endpoint.vala | 10 ++++-
.../outlook/imap-engine-outlook-account.vala | 13 ++++-
src/engine/imap/transport/imap-client-session.vala | 49 +++++++++++++-------
3 files changed, 52 insertions(+), 20 deletions(-)
---
diff --git a/src/engine/api/geary-endpoint.vala b/src/engine/api/geary-endpoint.vala
index d554bb3..cded061 100644
--- a/src/engine/api/geary-endpoint.vala
+++ b/src/engine/api/geary-endpoint.vala
@@ -44,7 +44,15 @@ public class Geary.Endpoint : BaseObject {
public uint timeout_sec { get; private set; }
public TlsCertificateFlags tls_validation_flags { get; set; default = TlsCertificateFlags.VALIDATE_ALL; }
public bool force_ssl3 { get; set; default = false; }
-
+
+ /**
+ * The maximum number of commands that will be pipelined at once.
+ *
+ * If 0 (the default), there is no limit on the number of
+ * pipelined commands sent to this endpoint.
+ */
+ public uint max_pipeline_batch_size = 0;
+
/**
* When set, TLS has reported certificate issues.
*
diff --git a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
index 3ca4acd..06eab91 100644
--- a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
+++ b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
@@ -6,13 +6,22 @@
private class Geary.ImapEngine.OutlookAccount : Geary.ImapEngine.GenericAccount {
public static Geary.Endpoint generate_imap_endpoint() {
- return new Geary.Endpoint(
+ Geary.Endpoint endpoint = new Geary.Endpoint(
"imap-mail.outlook.com",
Imap.ClientConnection.DEFAULT_PORT_SSL,
Geary.Endpoint.Flags.SSL,
Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
+ // As of June 2016, outlook.com's IMAP servers have a bug
+ // where a large number (~50) of pipelined STATUS commands on
+ // mailboxes with many messages will eventually cause it to
+ // break command parsing and return a BAD response, causing us
+ // to drop the connection. Limit the number of pipelined
+ // commands per batch to work around this. See b.g.o Bug
+ // 766552
+ endpoint.max_pipeline_batch_size = 25;
+ return endpoint;
}
-
+
public static Geary.Endpoint generate_smtp_endpoint() {
return new Geary.Endpoint(
"smtp-mail.outlook.com",
diff --git a/src/engine/imap/transport/imap-client-session.vala
b/src/engine/imap/transport/imap-client-session.vala
index e4896bc..a15e6fa 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -196,7 +196,7 @@ public class Geary.Imap.ClientSession : BaseObject {
private Command? state_change_cmd = null;
private Nonblocking.Semaphore? connect_waiter = null;
private Error? connect_err = null;
-
+
//
// Connection state changes
//
@@ -982,24 +982,39 @@ public class Geary.Imap.ClientSession : BaseObject {
throw params.err;
assert(params.proceed);
-
- // Issue all at once using a Nonblocking.Batch
- Nonblocking.Batch batch = new Nonblocking.Batch();
- foreach (Command cmd in cmds)
- batch.add(new SendCommandOperation(this, cmd));
-
- yield batch.execute_all_async(cancellable);
- batch.throw_first_exception();
-
- Gee.Map<Command, StatusResponse> map = new Gee.HashMap<Command, StatusResponse>();
- foreach (int id in batch.get_ids()) {
- SendCommandOperation op = (SendCommandOperation) batch.get_operation(id);
- map.set(op.cmd, op.response);
+
+ // Issue all at once using a single Nonblocking.Batch unless
+ // the endpoint's max pipeline size is positive, if so use
+ // multiple batches with a maximum size of that.
+
+ uint max_batch_size = this.imap_endpoint.max_pipeline_batch_size;
+ if (max_batch_size < 1) {
+ max_batch_size = cmds.size;
}
-
- return map;
+
+ Gee.Iterator<Command> cmd_remaining = cmds.iterator();
+ Nonblocking.Batch? batch = null;
+ Gee.Map<Command, StatusResponse> responses = new Gee.HashMap<Command, StatusResponse>();
+ while (cmd_remaining.has_next()) {
+ batch = new Nonblocking.Batch();
+ while (cmd_remaining.has_next() && batch.size < max_batch_size) {
+ cmd_remaining.next();
+ batch.add(new SendCommandOperation(this, cmd_remaining.get()));
+ }
+
+ debug("[%s] Sending batch of %u commands", to_string(), batch.size);
+ yield batch.execute_all_async(cancellable);
+ batch.throw_first_exception();
+
+ foreach (int id in batch.get_ids()) {
+ SendCommandOperation op = (SendCommandOperation) batch.get_operation(id);
+ responses.set(op.cmd, op.response);
+ }
+ }
+
+ return responses;
}
-
+
private void check_unsupported_send_command(Command cmd) throws Error {
// look for special commands that we wish to handle directly, as they affect the state
// machine
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]