[geary/wip/714802-chron: 1/3] Don't issue multiple DONEs when deidling connection: Bug #713417

[Jim Nelson <jnelson src gnome org>]

commit 77863e58b6657d753af1fc60d5b90195f3883799
Author: Jim Nelson <jim yorba org>
Date:   Fri Jan 23 16:51:00 2015 -0800

    Don't issue multiple DONEs when deidling connection: Bug #713417
    
    ClientConnection state machine had a timing hole where two DONEs could
    be issued in near-succession when closing a single IDLE command.
    The hole is issuing the DONE *before* the server acknowledged it
    was idling.  When the idling continuation response was received,
    the state machine would go to IDLE.  Then, when the next command
    was submitted, a second DONE would be issued.  The second DONE
    would be treated by the server as a tag with no command, causing
    a CLIENTBUG response.
    
    This tracks outstanding DONE commands when deidling and prevents
    switching to the IDLE state when an idline continuation response
    is received while waiting for the DONE to complete.

 .../imap/transport/imap-client-connection.vala     |   21 +++++++++++++++++--
 1 files changed, 18 insertions(+), 3 deletions(-)
---
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index 9a88834..9231dad 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -114,6 +114,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
     private uint flush_timeout_id = 0;
     private bool idle_when_quiet = false;
     private Gee.HashSet<Tag> posted_idle_tags = new Gee.HashSet<Tag>();
+    private int outstanding_idle_dones = 0;
     private Tag? posted_synchronization_tag = null;
     private StatusResponse? synchronization_status_response = null;
     private bool waiting_for_idle_to_synchronize = false;
@@ -215,7 +216,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
             new Geary.State.Mapping(State.IDLING, Event.SYNCHRONIZE, on_idle_synchronize),
             new Geary.State.Mapping(State.IDLING, Event.RECVD_STATUS_RESPONSE, on_idle_status_response),
             new Geary.State.Mapping(State.IDLING, Event.RECVD_SERVER_DATA, on_server_data),
-            new Geary.State.Mapping(State.IDLING, Event.RECVD_CONTINUATION_RESPONSE, on_idling_continuation),
+            new Geary.State.Mapping(State.IDLING, Event.RECVD_CONTINUATION_RESPONSE, 
on_idling_deidling_continuation),
             new Geary.State.Mapping(State.IDLING, Event.DISCONNECTED, on_disconnected),
             
             new Geary.State.Mapping(State.IDLE, Event.SEND, on_idle_send),
@@ -231,7 +232,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
             new Geary.State.Mapping(State.DEIDLING, Event.SYNCHRONIZE, on_deidling_synchronize),
             new Geary.State.Mapping(State.DEIDLING, Event.RECVD_STATUS_RESPONSE, on_idle_status_response),
             new Geary.State.Mapping(State.DEIDLING, Event.RECVD_SERVER_DATA, on_server_data),
-            new Geary.State.Mapping(State.DEIDLING, Event.RECVD_CONTINUATION_RESPONSE, 
on_idling_continuation),
+            new Geary.State.Mapping(State.DEIDLING, Event.RECVD_CONTINUATION_RESPONSE, 
on_idling_deidling_continuation),
             new Geary.State.Mapping(State.DEIDLING, Event.DISCONNECTED, on_disconnected),
             
             new Geary.State.Mapping(State.DEIDLING_SYNCHRONIZING, Event.SEND, on_proceed),
@@ -925,11 +926,16 @@ public class Geary.Imap.ClientConnection : BaseObject {
         return state;
     }
     
-    private uint on_idling_continuation(uint state, uint event, void *user, Object? object) {
+    private uint on_idling_deidling_continuation(uint state, uint event, void *user, Object? object) {
         ContinuationResponse continuation = (ContinuationResponse) object;
         
         Logging.debug(Logging.Flag.NETWORK, "[%s R] %s", to_string(), continuation.to_string());
         
+        // if deidling and a DONE is outstanding, keep waiting for it to complete -- don't go to
+        // IDLE, as that will cause another spurious DONE to be issued
+        if (state == State.DEIDLING && outstanding_idle_dones > 0)
+            return state;
+        
         // only signal entering IDLE state if that's the case
         if (state != State.IDLE)
             fsm.do_post_transition(signal_entered_idle);
@@ -952,6 +958,11 @@ public class Geary.Imap.ClientConnection : BaseObject {
             Logging.debug(Logging.Flag.NETWORK, "[%s S] %s", to_string(), IDLE_DONE);
             ser.push_unquoted_string(IDLE_DONE);
             ser.push_eol();
+            
+            // track the number of DONE's outstanding, as their responses are pipelined as well
+            // (this prevents issuing more than one DONE when the idle continuation response comes
+            // in *after* issuing the DONE)
+            outstanding_idle_dones++;
         } catch (Error err) {
             debug("[%s] Unable to close IDLE: %s", to_string(), err.message);
             
@@ -984,6 +995,10 @@ public class Geary.Imap.ClientConnection : BaseObject {
                 posted_idle_tags.size, status_response.to_string());
         }
         
+        // DONE has round-tripped (but watch for underflows, especially if server "forces" an IDLE
+        // to complete)
+        outstanding_idle_dones = Numeric.int_floor(outstanding_idle_dones - 1, 0);
+        
         // Only return to CONNECTED if no other IDLE commands are outstanding (and only signal
         // if leaving IDLE state for another)
         uint next = (posted_idle_tags.size == 0) ? State.CONNECTED : state;