GNOME.org
 

[geary: 4/9] Fix a number of objects being leaked at shutdown.

commit a6945b12b059af7b3b6abbb2a55d3c384855966d
Author: Michael James Gratton <mike vee net>
Date:   Wed Feb 21 16:04:19 2018 +1100

    Fix a number of objects being leaked at shutdown.

 src/client/application/geary-controller.vala       |   61 ++++++++++++++++----
 src/client/components/main-window.vala             |    5 --
 .../conversation-list-cell-renderer.vala           |    6 ++-
 .../conversation-list/conversation-list-store.vala |    1 +
 .../conversation-list/conversation-list-view.vala  |    3 +-
 src/client/notification/libnotify.vala             |    8 +--
 src/client/notification/new-messages-monitor.vala  |   12 +++-
 src/engine/api/geary-account-information.vala      |   35 ++++++------
 src/engine/api/geary-engine.vala                   |    3 +-
 src/engine/imap-db/imap-db-account.vala            |   14 +++--
 .../imap-engine/imap-engine-generic-account.vala   |   15 +----
 11 files changed, 97 insertions(+), 66 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 3fe7fdc..e38e674 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -59,8 +59,8 @@ public class GearyController : Geary.BaseObject {
 
     public weak GearyApplication application { get; private set; } // circular ref
 
-    public MainWindow main_window { get; private set; }
-    
+    public MainWindow? main_window { get; private set; default = null; }
+
     public Geary.App.ConversationMonitor? current_conversations { get; private set; default = null; }
     
     public AutostartManager? autostart_manager { get; private set; default = null; }
@@ -303,6 +303,10 @@ public class GearyController : Geary.BaseObject {
         Geary.Engine.instance.account_unavailable.disconnect(on_account_unavailable);
         Geary.Engine.instance.untrusted_host.disconnect(on_untrusted_host);
 
+        // Release folder and conversations in the main window
+        on_conversations_selected(new Gee.HashSet<Geary.App.Conversation>());
+        on_folder_selected(null);
+
         // Disconnect from various UI signals.
         main_window.conversation_list_view.conversations_selected.disconnect(on_conversations_selected);
         main_window.conversation_list_view.conversation_activated.disconnect(on_conversation_activated);
@@ -320,11 +324,18 @@ public class GearyController : Geary.BaseObject {
 
         // hide window while shutting down, as this can take a few seconds under certain conditions
         main_window.hide();
-        
+
+        // Release monitoring early so held resources can be freed up
+        this.libnotify = null;
+        this.new_messages_indicator = null;
+        this.unity_launcher = null;
+        this.new_messages_monitor.clear_folders();
+        this.new_messages_monitor = null;
+
         // drop the Revokable, which will commit it if necessary
         save_revokable(null, null);
 
-        this.autostart_manager = null;
+        this.cancellable_open_account.cancel();
 
         // Close the ConversationMonitor
         if (current_conversations != null) {
@@ -338,9 +349,9 @@ public class GearyController : Geary.BaseObject {
                     this.current_conversations.base_folder.to_string(),
                     err.message
                 );
-            } finally {
-                this.current_conversations = null;
             }
+
+            this.current_conversations = null;
         }
 
         // Close all inboxes. Launch these in parallel so we're not
@@ -388,12 +399,6 @@ public class GearyController : Geary.BaseObject {
             debug("Error waiting at shutdown barrier: %s", err.message);
         }
 
-        main_window.destroy();
-
-        debug("Flushing avatar cache...");
-        avatar_cache.flush();
-        avatar_cache.dump();
-
         // Turn off the lights and lock the door behind you
         try {
             debug("Closing Engine...");
@@ -402,6 +407,38 @@ public class GearyController : Geary.BaseObject {
         } catch (Error err) {
             message("Error closing Geary Engine instance: %s", err.message);
         }
+
+        this.application.remove_window(this.main_window);
+        this.main_window.destroy();
+        this.main_window = null;
+
+        this.upgrade_dialog = null;
+
+        if (login_dialog != null) {
+            this.login_dialog.destroy();
+            this.login_dialog = null;
+        }
+
+        this.current_account = null;
+        this.current_folder = null;
+
+        this.account_to_select = null;
+        this.previous_non_search_folder = null;
+        this.validating_endpoints.clear();
+
+        this.selected_conversations = new Gee.HashSet<Geary.App.Conversation>();
+        this.last_deleted_conversation = null;
+
+        this.pending_mailtos.clear();
+        this.composer_widgets.clear();
+        this.waiting_to_close.clear();
+
+        this.autostart_manager = null;
+
+        this.avatar_cache.flush();
+        this.avatar_cache.dump();
+
+        debug("Closed GearyController");
     }
 
     /**
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index 5b23f03..add9721 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -293,11 +293,6 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
             this.progress_monitor.add(conversations.progress_monitor);
             this.conversation_list_view.set_model(new_model);
         }
-
-        if (old_model != null) {
-            // Must be destroyed, but only after it has been replaced.
-            old_model.destroy();
-        }
     }
 
     private void on_folder_selected(Geary.Folder? folder) {
diff --git a/src/client/conversation-list/conversation-list-cell-renderer.vala 
b/src/client/conversation-list/conversation-list-cell-renderer.vala
index 36e1d66..94ab01a 100644
--- a/src/client/conversation-list/conversation-list-cell-renderer.vala
+++ b/src/client/conversation-list/conversation-list-cell-renderer.vala
@@ -10,10 +10,14 @@ public class ConversationListCellRenderer : Gtk.CellRenderer {
     
     // Mail message data.
     public FormattedConversationData data { get; set; }
-    
+
     public ConversationListCellRenderer() {
     }
 
+    ~ConversationListCellRenderer() {
+        example_data = null;
+    }
+
     public override void get_preferred_height(Gtk.Widget widget,
                                               out int minimum_size,
                                               out int natural_size) {
diff --git a/src/client/conversation-list/conversation-list-store.vala 
b/src/client/conversation-list/conversation-list-store.vala
index 695e1ed..8d1c438 100644
--- a/src/client/conversation-list/conversation-list-store.vala
+++ b/src/client/conversation-list/conversation-list-store.vala
@@ -124,6 +124,7 @@ public class ConversationListStore : Gtk.ListStore {
 
     public void destroy() {
         this.cancellable.cancel();
+        this.email_store = null;
         clear();
 
         // Release circular refs.
diff --git a/src/client/conversation-list/conversation-list-view.vala 
b/src/client/conversation-list/conversation-list-view.vala
index 8e6e796..d0ecb5d 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -8,7 +8,7 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
     const int LOAD_MORE_HEIGHT = 100;
 
     // Used to be able to refer to the action names of the MainWindow
-    private MainWindow main_window;
+    private weak MainWindow main_window;
 
     private bool enable_load_more = true;
 
@@ -102,6 +102,7 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
             old_store.row_changed.disconnect(on_rows_changed);
             old_store.row_deleted.disconnect(on_rows_changed);
             old_store.row_deleted.disconnect(on_row_deleted);
+            old_store.destroy();
         }
 
         if (new_store != null) {
diff --git a/src/client/notification/libnotify.vala b/src/client/notification/libnotify.vala
index f6717ab..4207bc8 100644
--- a/src/client/notification/libnotify.vala
+++ b/src/client/notification/libnotify.vala
@@ -11,7 +11,7 @@ public class Libnotify : Geary.BaseObject {
     
     private static Canberra.Context? sound_context = null;
     
-    private NewMessagesMonitor monitor;
+    private weak NewMessagesMonitor monitor;
     private Notify.Notification? current_notification = null;
     private Notify.Notification? error_notification = null;
     private Geary.Folder? folder = null;
@@ -37,11 +37,7 @@ public class Libnotify : Geary.BaseObject {
 
         monitor.new_messages_arrived.connect(on_new_messages_arrived);
     }
-    
-    ~Libnotify() {
-        monitor.new_messages_arrived.disconnect(on_new_messages_arrived);
-    }
-    
+
     private static void init_sound() {
         if (sound_context == null)
             Canberra.Context.create(out sound_context);
diff --git a/src/client/notification/new-messages-monitor.vala 
b/src/client/notification/new-messages-monitor.vala
index b60de2b..7e78da6 100644
--- a/src/client/notification/new-messages-monitor.vala
+++ b/src/client/notification/new-messages-monitor.vala
@@ -54,7 +54,7 @@ public class NewMessagesMonitor : Geary.BaseObject {
     public NewMessagesMonitor(ShouldNotifyNewMessages? should_notify_new_messages) {
         _should_notify_new_messages = should_notify_new_messages;
     }
-    
+
     public bool should_notify_new_messages(Geary.Folder folder) {
         return (_should_notify_new_messages == null ? true : _should_notify_new_messages(folder));
     }
@@ -85,7 +85,15 @@ public class NewMessagesMonitor : Geary.BaseObject {
         
         folder_removed(folder);
     }
-    
+
+    /** Releases all monitored folders. */
+    public void clear_folders() {
+        // Get an array so the loop does not blow up when removing values.
+        foreach (Geary.Folder monitored in this.folder_information.keys.to_array()) {
+            remove_folder(monitored);
+        }
+    }
+
     public Gee.Collection<Geary.Folder> get_folders() {
         return folder_information.keys;
     }
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 0b0ce5e..38c1c33 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -52,9 +52,13 @@ public class Geary.AccountInformation : BaseObject {
     public const int DEFAULT_PREFETCH_PERIOD_DAYS = 14;
     
     public static int default_ordinal = 0;
-    
-    private static Gee.HashMap<string, Geary.Endpoint>? known_endpoints = null;
-    
+
+    private static Gee.Map<string,weak Endpoint> known_endpoints;
+
+    static construct {
+        AccountInformation.known_endpoints = new Gee.HashMap<string,weak Endpoint>();
+    }
+
     /**
      * Location account information is stored (as well as other data, including database and
      * attachment files.
@@ -325,25 +329,20 @@ public class Geary.AccountInformation : BaseObject {
         if (smtp_endpoint != null)
             smtp_endpoint.untrusted_host.disconnect(on_smtp_untrusted_host);
     }
-    
-    internal static void init() {
-        known_endpoints = new Gee.HashMap<string, Geary.Endpoint>();
-    }
-    
+
     private static Geary.Endpoint get_shared_endpoint(Service service, Endpoint endpoint) {
         string key = "%s/%s:%u".printf(service.user_label(), endpoint.remote_address.hostname,
             endpoint.remote_address.port);
-        
-        // if already known, prefer it over this one
-        if (known_endpoints.has_key(key))
-            return known_endpoints.get(key);
-        
-        // save for future use and return this one
-        known_endpoints.set(key, endpoint);
-        
-        return endpoint;
+
+        Endpoint? cached = AccountInformation.known_endpoints.get(key);
+        if (cached == null) {
+            cached = endpoint;
+            AccountInformation.known_endpoints.set(key, cached);
+        }
+
+        return cached;
     }
-    
+
     // Copies all data from the "from" object into this one.
     public void copy_from(AccountInformation from) {
         this.id = from.id;
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index b1f4c02..9c47615 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -126,8 +126,7 @@ public class Geary.Engine : BaseObject {
             return;
         
         is_initialized = true;
-        
-        AccountInformation.init();
+
         Logging.init();
         RFC822.init();
         Imap.init();
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index a5faad1..cfa9f8e 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -351,12 +351,14 @@ private class Geary.ImapDB.Account : BaseObject {
         } finally {
             db = null;
         }
-        
-        background_cancellable.cancel();
-        background_cancellable = null;
-        
-        outbox.email_sent.disconnect(on_outbox_email_sent);
-        outbox = null;
+
+        this.background_cancellable.cancel();
+        this.background_cancellable = null;
+
+        this.folder_refs.clear();
+
+        this.outbox.email_sent.disconnect(on_outbox_email_sent);
+        this.outbox = null;
     }
 
     private void on_outbox_email_sent(Geary.RFC822.Message rfc822) {
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index aefed4b..9d1dde8 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -25,9 +25,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         Geary.SpecialFolderType.ARCHIVE,
     };
 
-    private static Geary.FolderPath? outbox_path = null;
-    private static Geary.FolderPath? search_path = null;
-
     /** This account's IMAP session pool. */
     public Imap.ClientSessionManager session_pool { get; private set; }
 
@@ -86,14 +83,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         db_vacuum_monitor = local.vacuum_monitor;
         opening_monitor = new Geary.ReentrantProgressMonitor(Geary.ProgressType.ACTIVITY);
         sending_monitor = local.sending_monitor;
-        
-        if (outbox_path == null) {
-            outbox_path = new SmtpOutboxFolderRoot();
-        }
-        
-        if (search_path == null) {
-            search_path = new ImapDB.SearchFolderRoot();
-        }
 
         this.sync = new AccountSynchronizer(this);
 
@@ -155,10 +144,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         // Local folders
 
         local.outbox.report_problem.connect(notify_report_problem);
-        local_only.set(outbox_path, local.outbox);
+        local_only.set(new SmtpOutboxFolderRoot(), local.outbox);
 
         this.search_folder = new_search_folder();
-        local_only.set(search_path, this.search_folder);
+        local_only.set(new ImapDB.SearchFolderRoot(), this.search_folder);
 
         this.open = true;
         notify_opened();
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]