[geary] Try harder to ensure Geary always shuts down cleanly.



commit 7ad941be8ea26a69ab587c2da58a31cce1eb929b
Author: Michael James Gratton <mike vee net>
Date:   Tue Nov 21 01:13:21 2017 +1100

    Try harder to ensure Geary always shuts down cleanly.
    
    * src/engine/imap-engine/imap-engine-generic-account.vala
      (GenericAccount): When closing, wait for each folder to close before
      shutting down the local and remote connections.
    
    * src/client/application/geary-controller.vala
      (GearyController::close_async): Close both inboxes and accounts in
      parallel. Don't wait for conversation monitor or inboxes to close
      before closing the account now that the accound does so. The account
      needs to start closing so that the background synchroniser stops, and
      that needs to stop before any open folder will close. Otherwise if the
      background synchroniser is currently synchronising an inbox or any open
      folder, then the account will not close until the synchroniser
      finishes, which could block for a long time.
    
    * src/engine/api/geary-abstract-local-folder.vala (Folder),
      src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
      Notify the close semaphore in the classes' ctors so that if they are
      never opened, calling wait_for_close_async() does not block.

 src/client/application/geary-controller.vala       |  105 +++++++++++---------
 src/engine/api/geary-abstract-local-folder.vala    |    7 +-
 .../imap-engine/imap-engine-generic-account.vala   |   35 +++++--
 .../imap-engine/imap-engine-minimal-folder.vala    |    4 +
 4 files changed, 92 insertions(+), 59 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index ea27f2d..08fd326 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1,6 +1,6 @@
 /*
  * Copyright 2016 Software Freedom Conservancy Inc.
- * Copyright 2016 Michael Gratton <mike vee net>
+ * Copyright 2016, 2017 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later). See the COPYING file in this distribution.
@@ -326,57 +326,68 @@ public class GearyController : Geary.BaseObject {
 
         this.autostart_manager = null;
 
-        // close the ConversationMonitor
-        try {
-            if (current_conversations != null) {
-                debug("Stopping conversation monitor for %s...", current_conversations.folder.to_string());
-                
-                bool closing = yield current_conversations.stop_monitoring_async(null);
-                
-                // If not an Inbox, wait for it to close so all pending operations are flushed
-                if (closing) {
-                    debug("Waiting for %s to close...", current_conversations.folder.to_string());
-                    yield current_conversations.folder.wait_for_close_async(null);
-                }
-                
-                debug("Stopped conversation monitor for %s", current_conversations.folder.to_string());
-            }
-        } catch (Error err) {
-            message("Error closing conversation monitor %s at shutdown: %s",
-                current_conversations.folder.to_string(), err.message);
-        } finally {
-            current_conversations = null;
-        }
-        
-        // close all Inboxes
-        foreach (Geary.Folder inbox in inboxes.values) {
+        // Close the ConversationMonitor
+        if (current_conversations != null) {
+            debug("Stopping conversation monitor for %s...",
+                  this.current_conversations.folder.to_string());
             try {
-                debug("Closing %s...", inbox.to_string());
-                
-                // close and wait for all pending operations to be flushed
-                yield inbox.close_async(null);
-                
-                debug("Waiting for %s to close completely...", inbox.to_string());
-                
-                yield inbox.wait_for_close_async(null);
-                
-                debug("Closed %s", inbox.to_string());
+                yield this.current_conversations.stop_monitoring_async(null);
             } catch (Error err) {
-                message("Error closing Inbox %s at shutdown: %s", inbox.to_string(), err.message);
+                debug(
+                    "Error closing conversation monitor %s at shutdown: %s",
+                    this.current_conversations.folder.to_string(),
+                    err.message
+                );
+            } finally {
+                this.current_conversations = null;
             }
         }
-        
-        // close all Accounts
-        foreach (Geary.Account account in email_stores.keys) {
-            try {
-                debug("Closing account %s", account.to_string());
-                yield account.close_async(null);
-                debug("Closed account %s", account.to_string());
-            } catch (Error err) {
-                message("Error closing account %s at shutdown: %s", account.to_string(), err.message);
-            }
+
+        // Close all inboxes. Launch these in parallel so we're not
+        // waiting time waiting for each one to close. The account
+        // will wait around for them to actually close.
+        foreach (Geary.Folder inbox in this.inboxes.values) {
+            debug("Closing %s...", inbox.to_string());
+            inbox.close_async.begin(null, (obj, ret) => {
+                    try {
+                        inbox.close_async.end(ret);
+                    } catch (Error err) {
+                        debug(
+                            "Error closing Inbox %s at shutdown: %s",
+                            inbox.to_string(), err.message
+                        );
+                    }
+                });
         }
-        
+
+        // Close all Accounts. Again, do this in parallel to minimise
+        // time taken to close, but here use a barrier to wait for all
+        // to actually finish closing.
+        Geary.Nonblocking.CountingSemaphore close_barrier =
+            new Geary.Nonblocking.CountingSemaphore(null);
+        foreach (Geary.Account account in this.email_stores.keys) {
+            debug("Closing account %s", account.to_string());
+            close_barrier.acquire();
+            account.close_async.begin(null, (obj, res) => {
+                    try {
+                        account.close_async.end(res);
+                        debug("Closed account %s", account.to_string());
+                        close_barrier.notify();
+                    } catch (Error err) {
+                        debug(
+                            "Error closing account %s at shutdown: %s",
+                            account.to_string(),
+                            err.message
+                        );
+                    }
+                });
+        }
+        try {
+            yield close_barrier.wait_async();
+        } catch (Error err) {
+            debug("Error waiting at shutdown barrier: %s", err.message);
+        }
+
         main_window.destroy();
 
         debug("Flushing avatar cache...");
diff --git a/src/engine/api/geary-abstract-local-folder.vala b/src/engine/api/geary-abstract-local-folder.vala
index 73923ed..7d72d0c 100644
--- a/src/engine/api/geary-abstract-local-folder.vala
+++ b/src/engine/api/geary-abstract-local-folder.vala
@@ -13,10 +13,13 @@ public abstract class Geary.AbstractLocalFolder : Geary.Folder {
     
     private int open_count = 0;
     private Nonblocking.Semaphore closed_semaphore = new Nonblocking.Semaphore();
-    
+
     protected AbstractLocalFolder() {
+        // Notify now to ensure that wait_for_close_async does not
+        // block if never opened.
+        this.closed_semaphore.blind_notify();
     }
-    
+
     public override Geary.Folder.OpenState get_open_state() {
         return open_count > 0 ? Geary.Folder.OpenState.LOCAL : Geary.Folder.OpenState.CLOSED;
     }
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index d8a54f4..7eeabfc 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -203,12 +203,29 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
         this.refresh_folder_timer.reset();
 
-        notify_folders_available_unavailable(null, sort_by_path(local_only.values));
-        notify_folders_available_unavailable(null, sort_by_path(folder_map.values));
-        
-        local.outbox.report_problem.disconnect(notify_report_problem);
-        
-        // attempt to close both regardless of errors
+        // Notify folders and ensure they are closed
+
+        Gee.List<Geary.Folder> locals = sort_by_path(this.local_only.values);
+        Gee.List<Geary.Folder> remotes = sort_by_path(this.folder_map.values);
+
+        this.local_only.clear();
+        this.folder_map.clear();
+
+        notify_folders_available_unavailable(null, locals);
+        notify_folders_available_unavailable(null, remotes);
+
+        foreach (Geary.Folder folder in locals) {
+            debug("%s: Waiting for local to close: %s", to_string(), folder.to_string());
+            yield folder.wait_for_close_async();
+        }
+        foreach (Geary.Folder folder in remotes) {
+            debug("%s: Waiting for remote to close: %s", to_string(), folder.to_string());
+            yield folder.wait_for_close_async();
+        }
+
+        this.local.outbox.report_problem.disconnect(notify_report_problem);
+
+        // Close accounts
         Error? local_err = null;
         try {
             yield local.close_async(cancellable);
@@ -222,10 +239,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         } catch (Error rclose_err) {
             remote_err = rclose_err;
         }
-        
-        folder_map.clear();
-        local_only.clear();
-        open = false;
+
+        this.open = false;
 
         notify_closed();
 
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 5fdc357..1287e95 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -112,6 +112,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         email_prefetcher = new EmailPrefetcher(this);
         
         local_folder.email_complete.connect(on_email_complete);
+
+        // Notify now to ensure that wait_for_close_async does not
+        // block if never opened.
+        this.closed_semaphore.blind_notify();
     }
 
     ~MinimalFolder() {


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