[geary/wip/replay-queue-wedging: 4/5] Rework how Imap.Command handles being cancelled



commit e2077984ecb0299f7e6a61e38ab03a0e0aa67c68
Author: Michael Gratton <mike vee net>
Date:   Sat Aug 10 08:42:34 2019 +1000

    Rework how Imap.Command handles being cancelled
    
    When a command is cancelled due to a disconnect, throwing a
    GLib.IOError.CANCELLED is ambigouous, since it could have also been
    because the API client cancelled sending the command. Instead, ensure
    an ImapError.DISCONNECTED is thrown instead.
    
    This fixes ImapEngine.ReplayQueue not retrying ops on disconnect.

 src/engine/imap/command/imap-command.vala          | 52 ++++++++++------------
 .../imap/transport/imap-client-connection.vala     |  4 +-
 2 files changed, 26 insertions(+), 30 deletions(-)
---
diff --git a/src/engine/imap/command/imap-command.vala b/src/engine/imap/command/imap-command.vala
index fd980831..d5be36b4 100644
--- a/src/engine/imap/command/imap-command.vala
+++ b/src/engine/imap/command/imap-command.vala
@@ -78,8 +78,7 @@ public class Geary.Imap.Command : BaseObject {
     private Geary.Nonblocking.Semaphore complete_lock =
         new Geary.Nonblocking.Semaphore();
 
-    private bool timed_out = false;
-    private bool cancelled = false;
+    private ImapError? cancelled_cause = null;
 
     private Geary.Nonblocking.Spinlock? literal_spinlock = null;
     private GLib.Cancellable? literal_cancellable = null;
@@ -206,20 +205,6 @@ public class Geary.Imap.Command : BaseObject {
         // Nothing to do by default
     }
 
-    /**
-     * Cancels this command's execution.
-     *
-     * When this method is called, all locks will be released,
-     * including {@link wait_until_complete}, which will then throw a
-     * `GLib.IOError.CANCELLED` error.
-     */
-    internal virtual void cancel_command() {
-        cancel_send();
-        this.cancelled = true;
-        this.response_timer.reset();
-        this.complete_lock.blind_notify();
-    }
-
     /**
      * Yields until the command has been completed or cancelled.
      *
@@ -231,16 +216,8 @@ public class Geary.Imap.Command : BaseObject {
         throws GLib.Error {
         yield this.complete_lock.wait_async(cancellable);
 
-        if (this.cancelled) {
-            throw new GLib.IOError.CANCELLED(
-                "%s: Command was cancelled", to_brief_string()
-            );
-        }
-
-        if (this.timed_out) {
-            throw new ImapError.TIMED_OUT(
-                "%s: Command timed out", to_brief_string()
-            );
+        if (this.cancelled_cause != null) {
+            throw this.cancelled_cause;
         }
 
         check_has_status();
@@ -288,6 +265,17 @@ public class Geary.Imap.Command : BaseObject {
         check_has_status();
     }
 
+    /**
+     * Cancels this command due to a network or server disconnect.
+     *
+     * When this method is called, all locks will be released,
+     * including {@link wait_until_complete}, which will then throw a
+     * `GLib.IOError.CANCELLED` error.
+     */
+    internal virtual void disconnected(string reason) {
+        cancel(new ImapError.NOT_CONNECTED("%s: %s", to_brief_string(), reason));
+    }
+
     /**
      * Called when tagged server data is received for this command.
      */
@@ -347,6 +335,13 @@ public class Geary.Imap.Command : BaseObject {
         }
     }
 
+    private void cancel(ImapError cause) {
+        cancel_send();
+        this.cancelled_cause = cause;
+        this.response_timer.reset();
+        this.complete_lock.blind_notify();
+    }
+
     private void check_has_status() throws ImapError {
         if (this.status == null) {
             throw new ImapError.SERVER_ERROR(
@@ -369,8 +364,9 @@ public class Geary.Imap.Command : BaseObject {
     }
 
     private void on_response_timeout() {
-        this.timed_out = true;
-        cancel_command();
+        cancel(
+            new ImapError.TIMED_OUT("%s: Command timed out", to_brief_string())
+        );
         response_timed_out();
     }
 
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index 4f7e9a26..88aeb168 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -262,7 +262,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
                 "[%s] Cancelling pending command: %s",
                 to_string(), pending.to_string()
             );
-            pending.cancel_command();
+            pending.disconnected("Disconnected");
         }
         this.pending_queue.clear();
 
@@ -385,7 +385,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
                 "[%s] Cancelling sent command: %s",
                 to_string(), sent.to_string()
             );
-            sent.cancel_command();
+            sent.disconnected("Connection channels closed");
         }
         this.sent_queue.clear();
 


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