[geary] Limit the number of pipelined STATUS commands sent to outloook.com.



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]