[geary/geary-0.12] Fix some serious run-time memory leaks.



commit 70b12d6799bdbf49affad2a3ee63ce0246e12164
Author: Michael James Gratton <mike vee net>
Date:   Wed Feb 21 12:36:59 2018 +1100

    Fix some serious run-time memory leaks.
    
    * src/client/components/client-web-view.vala (ClientWebView): Fix
      instances being leaked due to circular ref when registering JS message
      handlers.
    
    * src/engine/util/util-idle-manager.vala,
      src/engine/util/util-timeout-manager.vala: Fix instances and classes
      using these being leaked due to reference loop when used with closures
      as callbacks.

 src/client/components/client-web-view.vala |   22 +++++++++++++++++++---
 src/engine/util/util-idle-manager.vala     |    4 +++-
 src/engine/util/util-timeout-manager.vala  |    4 +++-
 3 files changed, 25 insertions(+), 5 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index 90743d3..01185fe 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -234,6 +234,9 @@ public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
     private Gee.Map<string,Geary.Memory.Buffer> internal_resources =
         new Gee.HashMap<string,Geary.Memory.Buffer>();
 
+    private Gee.List<ulong> registered_message_handlers =
+        new Gee.LinkedList<ulong>();
+
 
     /**
      * Emitted when the view's content has finished loaded.
@@ -321,6 +324,14 @@ public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
         base_unref();
     }
 
+    public override void destroy() {
+        foreach (ulong id in this.registered_message_handlers) {
+            this.user_content_manager.disconnect(id);
+        }
+        this.registered_message_handlers.clear();
+        base.destroy();
+    }
+
     /**
      * Loads a message HTML body into the view.
      */
@@ -413,11 +424,16 @@ public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
      */
     protected inline void register_message_handler(string name,
                                                    JavaScriptMessageHandler handler) {
-        // XXX cant use the delegate directly: b.g.o Bug 604781
-        this.user_content_manager.script_message_received[name].connect(
+        // XXX cant use the delegate directly, see b.g.o Bug
+        // 604781. However the workaround below creates a circular
+        // reference, causing ClientWebView instances to leak. So to
+        // work around that we need to record handler ids and
+        // disconnect them when being destroyed.
+        ulong id = this.user_content_manager.script_message_received[name].connect(
             (result) => { handler(result); }
         );
-        if (!get_user_content_manager().register_script_message_handler(name)) {
+        this.registered_message_handlers.add(id);
+        if (!this.user_content_manager.register_script_message_handler(name)) {
             debug("Failed to register script message handler: %s", name);
         }
     }
diff --git a/src/engine/util/util-idle-manager.vala b/src/engine/util/util-idle-manager.vala
index fac46dc..4193ed4 100644
--- a/src/engine/util/util-idle-manager.vala
+++ b/src/engine/util/util-idle-manager.vala
@@ -43,7 +43,9 @@ public class Geary.IdleManager : BaseObject {
         get { return this.source_id >= 0; }
     }
 
-    private IdleFunc callback;
+    // Callback must be unowned to avoid reference loop with owner's
+    // class when a closure is used as the callback.
+    private unowned IdleFunc callback;
     private int source_id = -1;
 
 
diff --git a/src/engine/util/util-timeout-manager.vala b/src/engine/util/util-timeout-manager.vala
index 8f493ad..fef1617 100644
--- a/src/engine/util/util-timeout-manager.vala
+++ b/src/engine/util/util-timeout-manager.vala
@@ -50,7 +50,9 @@ public class Geary.TimeoutManager : BaseObject {
         get { return this.source_id >= 0; }
     }
 
-    private TimeoutFunc callback;
+    // Callback must be unowned to avoid reference loop with owner's
+    // class when a closure is used as the callback.
+    private unowned TimeoutFunc callback;
     private int source_id = -1;
 
 


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