[geary] Don't signal ClientConnection IDLE StatusResponses



commit 96aaf32e3b03b770d97b3efa0f0f1af09e569cfa
Author: Jim Nelson <jim yorba org>
Date:   Mon Sep 1 21:48:19 2014 -0700

    Don't signal ClientConnection IDLE StatusResponses
    
    This fixes a subtle bug in the IMAP stack.  The most prominent symptom
    was for the entire folder list to clear after running Geary for a day
    or so.  Smaller symptoms were less noticeable, but could include dropped
    incoming messages (which would be picked up later when Geary was restarted
    or the connection was dropped and reestablished).
    
    The essential problem is that ClientConnection generates internal
    IDLE commands to control data flow and to receive unsolicited
    server data.  It would always signal the corresponding IDLE responses
    from the server.  These were being stored in the ClientSession object
    as "finished commands" for commands it did not issue, leading to the
    internal IDLE tag being kept around in a hash map.  When the tag
    numbering rolled over (which could take 8 - 12 hours depending on the
    connection) the ClientSession would complete new commands with those
    leftover tags instantly, causing for results to be dropped and the
    session state machine to be out of sync with the server.
    
    The solution is simply not to report status responses for internal
    IDLE commands.

 .../imap/transport/imap-client-connection.vala     |   24 ++++++-------------
 1 files changed, 8 insertions(+), 16 deletions(-)
---
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index d4dd7b7..d627670 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -972,18 +972,10 @@ public class Geary.Imap.ClientConnection : BaseObject {
         return do_proceed(State.DEIDLING, user);
     }
     
-    private void idle_status_response_post_transition(void *user, Object? object, Error? err) {
-        received_status_response((StatusResponse) object);
-        
-        // non-null use means "leaving idle"
-        if (user != null)
-            in_idle(false);
-    }
-    
     private uint on_idle_status_response(uint state, uint event, void *user, Object? object) {
         StatusResponse status_response = (StatusResponse) object;
         
-        // if not a post IDLE tag, then treat as external status response
+        // if not a posted IDLE tag, then treat as external status response
         if (!posted_idle_tags.remove(status_response.tag)) {
             fsm.do_post_transition(signal_status_response, user, object);
             
@@ -1003,13 +995,13 @@ public class Geary.Imap.ClientConnection : BaseObject {
         // if leaving IDLE state for another)
         uint next = (posted_idle_tags.size == 0) ? State.CONNECTED : state;
         
-        // need to always signal the StatusResponse, don't always signal about changing IDLE (this
-        // may've been signalled already when the DONE was transmitted)
-        //
-        // user pointer indicates if leaving idle.  playing with fire here...
-        fsm.do_post_transition(idle_status_response_post_transition,
-            (state == State.IDLE && next != State.IDLE) ? (void *) 1 : null,
-            status_response, null);
+        // don't signal about the StatusResponse, it's in response to a Command generated
+        // internally (IDLE) and will confuse watchers who receive StatusResponse for a Command
+        // they didn't issue
+        
+        // However, need to signal about leaving idle
+        if (state == State.IDLE && next != State.IDLE)
+            fsm.do_post_transition(signal_left_idle);
         
         // If leaving IDLE for CONNECTED but user has asked to stay in IDLE whenever quiet, reschedule
         // flush (which will automatically send IDLE command)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]