[geary: 3/9] Fix some serious run-time memory leaks.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary: 3/9] Fix some serious run-time memory leaks.
- Date: Tue, 6 Mar 2018 05:30:07 +0000 (UTC)
commit 635d3a947a7d6540d4e5378a92184756158cd203
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 | 8 +++++---
src/engine/util/util-timeout-manager.vala | 12 +++++++-----
3 files changed, 31 insertions(+), 11 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index c891bd7..5d38d4d 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -238,6 +238,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.
@@ -325,6 +328,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.
*/
@@ -417,11 +428,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 f6bf90e..d99431d 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;
@@ -53,8 +55,8 @@ public class Geary.IdleManager : BaseObject {
* The idle function will be by default not running, and hence
* needs to be started by a call to {@link schedule}.
*/
- public IdleManager(owned IdleFunc callback) {
- this.callback = (owned) callback;
+ public IdleManager(IdleFunc callback) {
+ this.callback = callback;
}
~IdleManager() {
diff --git a/src/engine/util/util-timeout-manager.vala b/src/engine/util/util-timeout-manager.vala
index b060097..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;
@@ -60,10 +62,10 @@ public class Geary.TimeoutManager : BaseObject {
* The timeout will be by default not running, and hence needs to be
* started by a call to {@link start}.
*/
- public TimeoutManager.seconds(uint interval, owned TimeoutFunc callback) {
+ public TimeoutManager.seconds(uint interval, TimeoutFunc callback) {
this.use_seconds = true;
this.interval = interval;
- this.callback = (owned) callback;
+ this.callback = callback;
}
/**
@@ -72,10 +74,10 @@ public class Geary.TimeoutManager : BaseObject {
* The timeout will be by default not running, and hence needs to be
* started by a call to {@link start}.
*/
- public TimeoutManager.milliseconds(uint interval, owned TimeoutFunc callback) {
+ public TimeoutManager.milliseconds(uint interval, TimeoutFunc callback) {
this.use_seconds = false;
this.interval = interval;
- this.callback = (owned) callback;
+ this.callback = callback;
}
~TimeoutManager() {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]