[geary: 2/9] Enable app reference tracking for objects such as GTK+ widgets.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary: 2/9] Enable app reference tracking for objects such as GTK+ widgets.
- Date: Tue, 6 Mar 2018 05:30:02 +0000 (UTC)
commit be9a02eaf7893a52d85c8c7bc0067216c728853d
Author: Michael James Gratton <mike vee net>
Date: Wed Feb 21 12:14:45 2018 +1100
Enable app reference tracking for objects such as GTK+ widgets.
This splits Geary.BaseObject up into a class an interface so that classes
derived from a pre-existing, non-GLib.Object class can still use the
reference tracking infrastructure, and adds the mixin interface to a
number of key client classes such as MainWindow and ClientWebView.
* src/engine/api/geary-base-object.vala: Split Geary.BaseObject up into a
class an interface so that classed derived from a pre-existing,
non-GLib.Object class can still use the reference tracking
infrastructure.
src/client/components/client-web-view.vala | 7 +-
src/client/components/main-window.vala | 7 +-
src/client/composer/contact-list-store.vala | 6 +-
.../conversation-list/conversation-list-view.vala | 7 +-
.../conversation-viewer/conversation-email.vala | 7 +-
.../conversation-viewer/conversation-list-box.vala | 14 +-
.../conversation-viewer/conversation-message.vala | 7 +-
.../conversation-viewer/conversation-viewer.vala | 8 +-
src/client/folder-list/folder-list-tree.vala | 10 +-
src/client/sidebar/sidebar-branch.vala | 2 +-
src/client/sidebar/sidebar-common.vala | 2 +-
src/engine/api/geary-base-object.vala | 217 +++++++++++++++----
.../imap-engine/imap-engine-email-prefetcher.vala | 2 +-
13 files changed, 233 insertions(+), 63 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index 1d34516..c891bd7 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -14,7 +14,7 @@
* integration, Inspector support, and remote and inline image
* handling.
*/
-public class ClientWebView : WebKit.WebView {
+public class ClientWebView : WebKit.WebView, Geary.BaseInterface {
/** URI Scheme and delimiter for internal resource loads. */
@@ -286,6 +286,7 @@ public class ClientWebView : WebKit.WebView {
user_content_manager: content_manager,
settings: setts
);
+ base_ref();
// XXX get the allow prefix from the extension somehow
@@ -320,6 +321,10 @@ public class ClientWebView : WebKit.WebView {
"monospace-font", SettingsBindFlags.DEFAULT);
}
+ ~ClientWebView() {
+ base_unref();
+ }
+
/**
* Loads a message HTML body into the view.
*/
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index e5cd7db..5b23f03 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -7,7 +7,7 @@
*/
[GtkTemplate (ui = "/org/gnome/Geary/main-window.ui")]
-public class MainWindow : Gtk.ApplicationWindow {
+public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
private const int STATUS_BAR_HEIGHT = 18;
public new GearyApplication application {
@@ -63,6 +63,7 @@ public class MainWindow : Gtk.ApplicationWindow {
public MainWindow(GearyApplication application) {
Object(application: application);
+ base_ref();
load_config(application.config);
restore_saved_window_state();
@@ -80,6 +81,10 @@ public class MainWindow : Gtk.ApplicationWindow {
this.main_layout.show_all();
}
+ ~MainWindow() {
+ base_unref();
+ }
+
public void show_infobar(MainWindowInfoBar info_bar) {
this.info_bar_container.add(info_bar);
this.info_bar_frame.show();
diff --git a/src/client/composer/contact-list-store.vala b/src/client/composer/contact-list-store.vala
index 821dffd..5959eb8 100644
--- a/src/client/composer/contact-list-store.vala
+++ b/src/client/composer/contact-list-store.vala
@@ -4,7 +4,7 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
-public class ContactListStore : Gtk.ListStore {
+public class ContactListStore : Gtk.ListStore, Geary.BaseInterface {
// Minimum visibility for the contact to appear in autocompletion.
private const Geary.ContactImportance CONTACT_VISIBILITY_THRESHOLD = Geary.ContactImportance.TO_TO;
@@ -65,8 +65,9 @@ public class ContactListStore : Gtk.ListStore {
}
public Geary.ContactStore contact_store { get; private set; }
-
+
public ContactListStore(Geary.ContactStore contact_store) {
+ base_ref();
set_column_types(Column.get_types());
this.contact_store = contact_store;
contact_store.contacts_added.connect(on_contacts_added);
@@ -74,6 +75,7 @@ public class ContactListStore : Gtk.ListStore {
}
~ContactListStore() {
+ base_unref();
this.contact_store.contacts_added.disconnect(on_contacts_added);
this.contact_store.contacts_updated.disconnect(on_contacts_updated);
}
diff --git a/src/client/conversation-list/conversation-list-view.vala
b/src/client/conversation-list/conversation-list-view.vala
index 3c7d6ec..8e6e796 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -4,7 +4,7 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
-public class ConversationListView : Gtk.TreeView {
+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
@@ -39,6 +39,7 @@ public class ConversationListView : Gtk.TreeView {
public ConversationListView(MainWindow parent) {
+ base_ref();
set_show_expanders(false);
set_headers_visible(false);
this.main_window = parent;
@@ -78,6 +79,10 @@ public class ConversationListView : Gtk.TreeView {
this.selection_update.priority = Geary.IdleManager.Priority.LOW;
}
+ ~ConversationListView() {
+ base_unref();
+ }
+
public override void destroy() {
this.selection_update.reset();
base.destroy();
diff --git a/src/client/conversation-viewer/conversation-email.vala
b/src/client/conversation-viewer/conversation-email.vala
index 45f1a6c..84c1b98 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -16,7 +16,7 @@
* ConversationMessage}.
*/
[GtkTemplate (ui = "/org/gnome/Geary/conversation-email.ui")]
-public class ConversationEmail : Gtk.Box {
+public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
// This isn't a Gtk.Grid since when added to a Gtk.ListBoxRow the
// hover style isn't applied to it.
@@ -363,6 +363,7 @@ public class ConversationEmail : Gtk.Box {
Configuration config,
bool is_sent,
bool is_draft) {
+ base_ref();
this.email = email;
this.contact_store = contact_store;
this.config = config;
@@ -496,6 +497,10 @@ public class ConversationEmail : Gtk.Box {
}
}
+ ~ConversationEmail() {
+ base_unref();
+ }
+
/**
* Starts loading the complete email.
*
diff --git a/src/client/conversation-viewer/conversation-list-box.vala
b/src/client/conversation-viewer/conversation-list-box.vala
index 8bd9c60..5cbe01d 100644
--- a/src/client/conversation-viewer/conversation-list-box.vala
+++ b/src/client/conversation-viewer/conversation-list-box.vala
@@ -18,7 +18,7 @@
* ConversationListBox sorts by the {@link Geary.Email.date} field
* (the Date: header), as that's the date displayed to the user.
*/
-public class ConversationListBox : Gtk.ListBox {
+public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
/** Fields that must be available for display as a conversation. */
private const Geary.Email.Field REQUIRED_FIELDS =
@@ -44,7 +44,7 @@ public class ConversationListBox : Gtk.ListBox {
// Base class for list rows it the list box
- private abstract class ConversationRow : Gtk.ListBoxRow {
+ private abstract class ConversationRow : Gtk.ListBoxRow, Geary.BaseInterface {
protected const string EXPANDED_CLASS = "geary-expanded";
@@ -91,10 +91,15 @@ public class ConversationListBox : Gtk.ListBox {
public ConversationRow(Geary.Email? email) {
+ base_ref();
this.email = email;
show();
}
+ ~ConversationRow() {
+ base_unref();
+ }
+
// Request the row be expanded, if supported.
public virtual new void expand() {
// Not supported by default
@@ -494,6 +499,7 @@ public class ConversationListBox : Gtk.ListBox {
Configuration config,
Soup.Session avatar_session,
Gtk.Adjustment adjustment) {
+ base_ref();
this.conversation = conversation;
this.location = location;
this.email_store = email_store;
@@ -532,6 +538,10 @@ public class ConversationListBox : Gtk.ListBox {
});
}
+ ~ConversationListBox() {
+ base_unref();
+ }
+
public override void destroy() {
if (this.loading_timeout_id != 0) {
Source.remove(this.loading_timeout_id);
diff --git a/src/client/conversation-viewer/conversation-message.vala
b/src/client/conversation-viewer/conversation-message.vala
index be6fddb..75a28ad 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -15,7 +15,7 @@
* embeds at least one instance of this class.
*/
[GtkTemplate (ui = "/org/gnome/Geary/conversation-message.ui")]
-public class ConversationMessage : Gtk.Grid {
+public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
private const string FROM_CLASS = "geary-from";
@@ -276,6 +276,7 @@ public class ConversationMessage : Gtk.Grid {
public ConversationMessage(Geary.RFC822.Message message,
Configuration config,
bool load_remote_images) {
+ base_ref();
this.message = message;
this.is_loading_images = load_remote_images;
@@ -402,6 +403,10 @@ public class ConversationMessage : Gtk.Grid {
);
}
+ ~ConversationMessage() {
+ base_unref();
+ }
+
public override void destroy() {
this.show_progress_timeout.reset();
this.hide_progress_timeout.reset();
diff --git a/src/client/conversation-viewer/conversation-viewer.vala
b/src/client/conversation-viewer/conversation-viewer.vala
index 387197b..a941929 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -10,7 +10,7 @@
* Displays the messages in a conversation and in-window composers.
*/
[GtkTemplate (ui = "/org/gnome/Geary/conversation-viewer.ui")]
-public class ConversationViewer : Gtk.Stack {
+public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
/**
* The current conversation listbox, if any.
@@ -68,6 +68,8 @@ public class ConversationViewer : Gtk.Stack {
* Constructs a new conversation view instance.
*/
public ConversationViewer() {
+ base_ref();
+
EmptyPlaceholder no_conversations = new EmptyPlaceholder();
no_conversations.title = _("No conversations selected");
no_conversations.subtitle = _(
@@ -103,6 +105,10 @@ public class ConversationViewer : Gtk.Stack {
this.conversation_find_bar.connect_entry(this.conversation_find_entry);
}
+ ~ConversationViewer() {
+ base_unref();
+ }
+
/**
* Puts the view into composer mode, showing a full-height composer.
*/
diff --git a/src/client/folder-list/folder-list-tree.vala b/src/client/folder-list/folder-list-tree.vala
index d4a3ebe..0ad1969 100644
--- a/src/client/folder-list/folder-list-tree.vala
+++ b/src/client/folder-list/folder-list-tree.vala
@@ -4,7 +4,7 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
-public class FolderList.Tree : Sidebar.Tree {
+public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
public const Gtk.TargetEntry[] TARGET_ENTRY_LIST = {
{ "application/x-geary-mail", Gtk.TargetFlags.SAME_APP, 0 }
};
@@ -21,9 +21,10 @@ public class FolderList.Tree : Sidebar.Tree {
private InboxesBranch inboxes_branch = new InboxesBranch();
private SearchBranch? search_branch = null;
private NewMessagesMonitor? monitor = null;
-
+
public Tree() {
base(new Gtk.TargetEntry[0], Gdk.DragAction.ASK, drop_handler);
+ base_ref();
entry_selected.connect(on_entry_selected);
// Set self as a drag destination.
@@ -52,11 +53,12 @@ public class FolderList.Tree : Sidebar.Tree {
"""
);
}
-
+
~Tree() {
set_new_messages_monitor(null);
+ base_unref();
}
-
+
private void drop_handler(Gdk.DragContext context, Sidebar.Entry? entry,
Gtk.SelectionData data, uint info, uint time) {
}
diff --git a/src/client/sidebar/sidebar-branch.vala b/src/client/sidebar/sidebar-branch.vala
index bbb3ca8..4caf0de 100644
--- a/src/client/sidebar/sidebar-branch.vala
+++ b/src/client/sidebar/sidebar-branch.vala
@@ -6,7 +6,7 @@
public delegate bool Locator<G>(G item);
-public class Sidebar.Branch : Object {
+public class Sidebar.Branch : Geary.BaseObject {
[Flags]
public enum Options {
NONE = 0,
diff --git a/src/client/sidebar/sidebar-common.vala b/src/client/sidebar/sidebar-common.vala
index 7d157ac..921a639 100644
--- a/src/client/sidebar/sidebar-common.vala
+++ b/src/client/sidebar/sidebar-common.vala
@@ -5,7 +5,7 @@
*/
// A simple grouping Entry that is only expandable
-public class Sidebar.Grouping : Object, Sidebar.Entry, Sidebar.ExpandableEntry,
+public class Sidebar.Grouping : Geary.BaseObject, Sidebar.Entry, Sidebar.ExpandableEntry,
Sidebar.RenameableEntry {
private string name;
diff --git a/src/engine/api/geary-base-object.vala b/src/engine/api/geary-base-object.vala
index da92237..bd3f575 100644
--- a/src/engine/api/geary-base-object.vala
+++ b/src/engine/api/geary-base-object.vala
@@ -1,64 +1,189 @@
-/* Copyright 2016 Software Freedom Conservancy Inc.
+/*
+ * Copyright 2016 Software Freedom Conservancy Inc.
+ * Copyright 2018 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.
*/
-public abstract class Geary.BaseObject : Object {
#if REF_TRACKING
- private static Gee.HashMap<unowned string, int>? refmap = null;
-
+private class Counts {
+ public int created = 0;
+ public int destroyed = 0;
+}
+
+private static GLib.Mutex reflock;
+private static Gee.HashMap<unowned string,Counts?> refmap;
+#endif
+
+/**
+ * Common parent class for Geary Engine objects.
+ *
+ * Most Engine classes should be derived from this class to be able to
+ * access common functionality.
+ *
+ * Currently, this class enables automatic application-based reference
+ * tracking of objects, which can assist in tracking down memory
+ * leaks. To enable this, set the //ref_track// compile time option to
+ * true and compile. An application can then call {@link dump_refs} to
+ * output a list of objects with both instantiation and destruction
+ * counts.
+ *
+ * This class can only be used for classes that would otherwise derive
+ * directly from GLib.Object. If a class must be derived from some
+ * other pre-existing class, it is possible to use {@link
+ * BaseInterface} as a mixin with a little extra work instead.
+ */
+public abstract class Geary.BaseObject : Geary.BaseInterface, Object {
+
+#if REF_TRACKING
+ static construct {
+ reflock = GLib.Mutex();
+ refmap = new Gee.HashMap<unowned string,Counts?>(
+ // because strings are unowned and guaranteed to be
+ // unique by GType, use direct comparison functions,
+ // more efficient then string hash/equal
+ Gee.Functions.get_hash_func_for(typeof(void*)),
+ Gee.Functions.get_equal_func_for(typeof(void*))
+ );
+ }
+#endif
+
+ /**
+ * Constructs a new base object.
+ *
+ * This constructor automatically increments the reference count
+ * by calling {@link BaseInterface.base_ref} if reference tracking
+ * is enabled at compile-time, otherwise this is a no-op.
+ */
protected BaseObject() {
- lock (refmap) {
- if (refmap == null) {
- // because strings are unowned and guaranteed to be
- // unique by GType, use direct comparison functions,
- // more efficient then string hash/equal
- refmap = new Gee.HashMap<unowned string, int>(
- Gee.Functions.get_hash_func_for(typeof(void*)),
- Gee.Functions.get_equal_func_for(typeof(void*)));
- }
-
- unowned string classname = get_classname();
- refmap.set(classname, refmap.get(classname) + 1);
- }
+#if REF_TRACKING
+ base_ref();
+#endif
}
-
+
+ /**
+ * Destructs an existing base object.
+ *
+ * This constructor automatically decrements the reference count
+ * by calling {@link BaseInterface.base_unref} if reference
+ * tracking is enabled at compile-time, otherwise this is a no-op.
+ */
~BaseObject() {
- lock (refmap) {
- unowned string classname = get_classname();
- int count = refmap.get(classname) - 1;
- if (count == 0)
- refmap.unset(classname);
- else
- refmap.set(classname, count);
- }
- }
-
- private unowned string get_classname() {
- return get_class().get_type().name();
+#if REF_TRACKING
+ base_unref();
+#endif
}
-
+
+ /**
+ * Dumps reference counting logs to the given stream.
+ *
+ * This method prints a list of reference-counted classes, and the
+ * number of times each was constructed and destructed along with
+ * a flag indicating those that are not equal. If reference
+ * tracking was not enabled at compile-time, this is no-op.
+ */
public static void dump_refs(FileStream outs) {
- if (refmap == null || refmap.size == 0) {
+#if REF_TRACKING
+ if (!refmap.is_empty) {
+ Gee.ArrayList<unowned string> list = new Gee.ArrayList<unowned string>();
+ list.add_all(refmap.keys);
+ list.sort();
+ outs.printf("? created/destroyed class\n");
+ foreach (unowned string classname in list) {
+ Counts? counts = refmap.get(classname);
+ string alert = " ";
+ if (counts.created != counts.destroyed) {
+ double leak_rate = (counts.created - counts.destroyed) / counts.created;
+ alert = (leak_rate > 0.1) ? "!" : "*";
+ }
+ outs.printf(
+ "%s %9d/%9d %s\n",
+ alert,
+ counts.created,
+ counts.destroyed,
+ classname
+ );
+ }
+ } else {
outs.printf("No references to report.\n");
-
- return;
}
-
- Gee.ArrayList<unowned string> list = new Gee.ArrayList<unowned string>();
- list.add_all(refmap.keys);
- list.sort();
- foreach (unowned string classname in list)
- outs.printf("%9d %s\n", refmap.get(classname), classname);
+#endif
}
-#else
- protected BaseObject() {
+
+}
+
+/**
+ * Base mixin interface for Engine objects derived from another.
+ *
+ * This interface provides an analogue to {@link BaseObject} for
+ * objects that must be derived from an existing class, such a GTK
+ * widget. Since this is simply a mixin, some additional work is
+ * required to use this interface compared to BaseObject.
+ *
+ * To use this interface, declare it as a parent of your class and
+ * call {@link base_ref} immediately after the base constructor is
+ * called, and call {@link base_unref} at the end of the destructor:
+ *
+ * public class CustomWidget : Gtk.Widget, Geary.BaseInterface {
+ *
+ * public CustomWidget() {
+ * base_ref();
+ * ...
+ * }
+ *
+ * ~CustomWidget() {
+ * ...
+ * base_unref();
+ * }
+ *
+ * }
+ *
+ * Care must be taken to ensure that if {@link base_unref} is not
+ * called for an instance if {@link base_ref} was not called.
+ */
+public interface Geary.BaseInterface {
+
+ /**
+ * Increments the reference count for the implementing class.
+ *
+ * Implementing classes should call this as soon as possible after
+ * the base class's constructor is called. This method is a no-op
+ * if reference tracking is not enabled at compile-time.
+ */
+ protected void base_ref() {
+#if REF_TRACKING
+ reflock.lock();
+ unowned string classname = get_classname();
+ Counts? counts = refmap.get(classname);
+ if (counts == null) {
+ counts = new Counts();
+ refmap.set(classname, counts);
+ }
+ counts.created++;
+ reflock.unlock();
+#endif
}
-
- public static void dump(FileStream outs) {
- outs.printf("Reference tracking disabled.\n");
+
+ /**
+ * Decrements the reference count for the implementing class.
+ *
+ * Implementing classes should call this at the end of the class's
+ * destructor. This method is a no-op if reference tracking is not
+ * enabled at compile-time.
+ */
+ protected void base_unref() {
+#if REF_TRACKING
+ reflock.lock();
+ refmap.get(get_classname()).destroyed++;
+ reflock.unlock();
+#endif
}
+
+#if REF_TRACKING
+ private unowned string get_classname() {
+ return ((Object) this).get_type().name();
+ }
+
#endif
}
-
diff --git a/src/engine/imap-engine/imap-engine-email-prefetcher.vala
b/src/engine/imap-engine/imap-engine-email-prefetcher.vala
index c46628e..37c3a40 100644
--- a/src/engine/imap-engine/imap-engine-email-prefetcher.vala
+++ b/src/engine/imap-engine/imap-engine-email-prefetcher.vala
@@ -11,7 +11,7 @@
*
* The EmailPrefetcher does not maintain a reference to the folder.
*/
-private class Geary.ImapEngine.EmailPrefetcher : Object {
+private class Geary.ImapEngine.EmailPrefetcher : Geary.BaseObject {
public const int PREFETCH_DELAY_SEC = 1;
private const Geary.Email.Field PREFETCH_FIELDS = Geary.Email.Field.ALL;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
