[geary] Prevent hang when closing ClientSessionManager: Bug #745561

[Jim Nelson <jnelson src gnome org>]

commit f9e850292f671063a9a56198ac4709367f241787
Author: Jim Nelson <jim yorba org>
Date:   Thu Mar 19 18:36:37 2015 -0700

    Prevent hang when closing ClientSessionManager: Bug #745561
    
    This change prevents locking the ClientSessionManager sessions table
    during a close_async().  Looking at debug logs where Geary hangs at
    exit, that looks to be where the hang happens.
    
    However, I've also added some additional logging to verify the hang
    is not occurring due to an unrelease mutex in Imap.Account or the
    ClientSessionManager.

 src/engine/imap/api/imap-account.vala              |   20 ++++++++++++++++----
 .../transport/imap-client-session-manager.vala     |   20 ++++++--------------
 2 files changed, 22 insertions(+), 18 deletions(-)
---
diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala
index 8d6aefb..25b7e45 100644
--- a/src/engine/imap/api/imap-account.vala
+++ b/src/engine/imap/api/imap-account.vala
@@ -155,6 +155,8 @@ private class Geary.Imap.Account : BaseObject {
     }
     
     private async void drop_session_async(Cancellable? cancellable) {
+        debug("[%s] Dropping account session...", to_string());
+        
         int token;
         try {
             token = yield account_session_mutex.claim_async(cancellable);
@@ -164,17 +166,25 @@ private class Geary.Imap.Account : BaseObject {
             return;
         }
         
+        string desc = account_session != null ? account_session.to_string() : "(none)";
+        
         if (account_session != null) {
+            // disconnect signals before releasing (in particular, "disconnected" will in turn
+            // reenter this method, so avoid that)
+            account_session.list.disconnect(on_list_data);
+            account_session.status.disconnect(on_status_data);
+            account_session.server_data_received.disconnect(on_server_data_received);
+            account_session.disconnected.disconnect(on_disconnected);
+            
+            debug("[%s] Releasing account session %s", to_string(), desc);
+            
             try {
                 yield session_mgr.release_session_async(account_session, cancellable);
             } catch (Error err) {
                 // ignored
             }
             
-            account_session.list.disconnect(on_list_data);
-            account_session.status.disconnect(on_status_data);
-            account_session.server_data_received.disconnect(on_server_data_received);
-            account_session.disconnected.disconnect(on_disconnected);
+            debug("[%s] Released account session %s", to_string(), desc);
             
             account_session = null;
         }
@@ -184,6 +194,8 @@ private class Geary.Imap.Account : BaseObject {
         } catch (Error err) {
             // ignored
         }
+        
+        debug("[%s] Dropped account session (%s)", to_string(), desc);
     }
     
     private void on_list_data(MailboxInformation mailbox_info) {
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala 
b/src/engine/imap/transport/imap-client-session-manager.vala
index 39f87f5..c4f577d 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -115,25 +115,17 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         
         is_open = false;
         
-        int token;
-        try {
-            token = yield sessions_mutex.claim_async();
-        } catch (Error claim_err) {
-            debug("Unable to claim session table mutex for closing pool: %s", claim_err.message);
-            
-            return;
-        }
+        // to avoid locking down the sessions table while scheduling disconnects, make a copy
+        // and work off of that
+        ClientSession[]? sessions_copy = sessions.to_array();
         
         // disconnect all existing sessions at once; don't wait for each, since as they disconnect
         // they'll remove themselves from the sessions list and cause this foreach to explode
-        foreach (ClientSession session in sessions)
+        foreach (ClientSession session in sessions_copy)
             session.disconnect_async.begin();
         
-        try {
-            sessions_mutex.release(ref token);
-        } catch (Error release_err) {
-            debug("Unable to release session table mutex after closing pool: %s", release_err.message);
-        }
+        // free copy
+        sessions_copy = null;
         
         // TODO: This isn't the best (deterministic) way to deal with this, but it's easy and works
         // for now