Merge lp:~ted/libdbusmenu/reference-with-update into lp:libdbusmenu/0.5

Proposed by Ted Gould
Status: Merged
Merged at revision: 200
Proposed branch: lp:~ted/libdbusmenu/reference-with-update
Merge into: lp:libdbusmenu/0.5
Diff against target: 217 lines (+68/-11)
2 files modified
libdbusmenu-glib/client.c (+29/-11)
libdbusmenu-gtk/menu.c (+39/-0)
To merge this branch: bzr merge lp:~ted/libdbusmenu/reference-with-update
Reviewer Review Type Date Requested Status
Conor Curran (community) Approve
Review via email: mp+48022@code.launchpad.net

Description of the change

A set of bugs that were caused by objects getting unreferenced without entirely cleaning up. These were a result of working with lool on #ubuntu-desktop who had a stressful situation on his laptop and could recreate easily. Thanks lool!

To post a comment you must log in.
Revision history for this message
Conor Curran (cjcurran) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-glib/client.c'
2--- libdbusmenu-glib/client.c 2011-01-27 19:54:18 +0000
3+++ libdbusmenu-glib/client.c 2011-01-31 17:04:14 +0000
4@@ -129,6 +129,12 @@
5 gchar * type;
6 };
7
8+typedef struct _properties_callback_t properties_callback_t;
9+struct _properties_callback_t {
10+ DbusmenuClient * client;
11+ GArray * listeners;
12+};
13+
14
15 #define DBUSMENU_CLIENT_GET_PRIVATE(o) (DBUSMENU_CLIENT(o)->priv)
16 #define DBUSMENU_INTERFACE "com.canonical.dbusmenu"
17@@ -512,7 +518,8 @@
18 static void
19 get_properties_callback (GObject *obj, GAsyncResult * res, gpointer user_data)
20 {
21- GArray * listeners = (GArray *)user_data;
22+ properties_callback_t * cbdata = (properties_callback_t *)user_data;
23+ GArray * listeners = cbdata->listeners;
24 int i;
25 GError * error = NULL;
26 GVariant * params = NULL;
27@@ -526,9 +533,8 @@
28 properties_listener_t * listener = &g_array_index(listeners, properties_listener_t, i);
29 listener->callback(NULL, error, listener->user_data);
30 }
31- g_array_free(listeners, TRUE);
32 g_error_free(error);
33- return;
34+ goto out;
35 }
36
37 /* Callback all the folks we can find */
38@@ -575,8 +581,11 @@
39 g_error_free(localerror);
40 }
41
42+out:
43 /* Clean up */
44 g_array_free(listeners, TRUE);
45+ g_object_unref(cbdata->client);
46+ g_free(user_data);
47
48 return;
49 }
50@@ -586,6 +595,7 @@
51 static gboolean
52 get_properties_idle (gpointer user_data)
53 {
54+ properties_callback_t * cbdata = NULL;
55 DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(user_data);
56 g_return_val_if_fail(priv->menuproxy != NULL, TRUE);
57
58@@ -616,6 +626,11 @@
59 g_variant_builder_add_value(&builder, variant_props);
60 GVariant * variant_params = g_variant_builder_end(&builder);
61
62+ cbdata = g_new(properties_callback_t, 1);
63+ cbdata->listeners = priv->delayed_property_listeners;
64+ cbdata->client = DBUSMENU_CLIENT(user_data);
65+ g_object_ref(G_OBJECT(user_data));
66+
67 g_dbus_proxy_call(priv->menuproxy,
68 "GetGroupProperties",
69 variant_params,
70@@ -623,7 +638,7 @@
71 -1, /* timeout */
72 NULL, /* cancellable */
73 get_properties_callback,
74- priv->delayed_property_listeners);
75+ cbdata);
76
77 /* Free properties */
78 gchar ** dataregion = (gchar **)g_array_free(priv->delayed_property_list, FALSE);
79@@ -1553,11 +1568,6 @@
80 DbusmenuClient * client = DBUSMENU_CLIENT(data);
81 DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(client);
82
83- if (priv->layoutcall != NULL) {
84- g_object_unref(priv->layoutcall);
85- priv->layoutcall = NULL;
86- }
87-
88 GError * error = NULL;
89 GVariant * params = NULL;
90
91@@ -1566,7 +1576,7 @@
92 if (error != NULL) {
93 g_warning("Getting layout failed: %s", error->message);
94 g_error_free(error);
95- return;
96+ goto out;
97 }
98
99 guint rev;
100@@ -1580,7 +1590,7 @@
101
102 if (parseable == 0) {
103 g_warning("Unable to parse layout!");
104- return;
105+ goto out;
106 }
107
108 priv->my_revision = rev;
109@@ -1596,6 +1606,13 @@
110 update_layout(client);
111 }
112
113+out:
114+ if (priv->layoutcall != NULL) {
115+ g_object_unref(priv->layoutcall);
116+ priv->layoutcall = NULL;
117+ }
118+
119+ g_object_unref(G_OBJECT(client));
120 return;
121 }
122
123@@ -1622,6 +1639,7 @@
124
125 priv->layoutcall = g_cancellable_new();
126
127+ g_object_ref(G_OBJECT(client));
128 g_dbus_proxy_call(priv->menuproxy,
129 "GetLayout",
130 g_variant_new("(i)", 0), /* root */
131
132=== modified file 'libdbusmenu-gtk/menu.c'
133--- libdbusmenu-gtk/menu.c 2010-11-11 14:15:20 +0000
134+++ libdbusmenu-gtk/menu.c 2011-01-31 17:04:14 +0000
135@@ -46,6 +46,7 @@
136 /* Private */
137 struct _DbusmenuGtkMenuPrivate {
138 DbusmenuGtkClient * client;
139+ DbusmenuMenuitem * root;
140
141 gchar * dbus_object;
142 gchar * dbus_name;
143@@ -63,6 +64,8 @@
144 /* Internal */
145 static void build_client (DbusmenuGtkMenu * self);
146 static void child_realized (DbusmenuMenuitem * child, gpointer userdata);
147+static void remove_child_signals (gpointer data, gpointer user_data);
148+static void root_changed (DbusmenuGtkClient * client, DbusmenuMenuitem * newroot, DbusmenuGtkMenu * menu);
149
150 /* GObject Stuff */
151 G_DEFINE_TYPE (DbusmenuGtkMenu, dbusmenu_gtkmenu, GTK_TYPE_MENU);
152@@ -127,6 +130,12 @@
153 {
154 DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(object);
155
156+ /* Remove signals from the root */
157+ if (priv->root != NULL) {
158+ /* This will clear the root */
159+ root_changed(priv->client, NULL, DBUSMENU_GTKMENU(object));
160+ }
161+
162 if (priv->client != NULL) {
163 g_object_unref(G_OBJECT(priv->client));
164 priv->client = NULL;
165@@ -271,6 +280,10 @@
166 #ifdef MASSIVEDEBUGGING
167 g_debug("Root child deleted");
168 #endif
169+
170+ /* Remove signal for realized */
171+ remove_child_signals(child, menu);
172+
173 DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(menu);
174 GtkWidget * item = GTK_WIDGET(dbusmenu_gtkclient_menuitem_get(priv->client, child));
175 if (item != NULL) {
176@@ -308,15 +321,41 @@
177 return;
178 }
179
180+/* Remove any signals we attached to children -- just realized right now */
181+static void
182+remove_child_signals (gpointer data, gpointer user_data)
183+{
184+ g_signal_handlers_disconnect_by_func(G_OBJECT(data), child_realized, user_data);
185+ return;
186+}
187+
188 /* When the root menuitem changes we need to resetup things so that
189 we're back in the game. */
190 static void
191 root_changed (DbusmenuGtkClient * client, DbusmenuMenuitem * newroot, DbusmenuGtkMenu * menu) {
192+ DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(menu);
193+
194+ /* Clear out our interest in the old root */
195+ if (priv->root != NULL) {
196+ GList * children = dbusmenu_menuitem_get_children(priv->root);
197+ g_list_foreach(children, remove_child_signals, menu);
198+
199+ g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_added, menu);
200+ g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_moved, menu);
201+ g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_delete, menu);
202+
203+ g_object_unref(priv->root);
204+ priv->root = NULL;
205+ }
206+
207 if (newroot == NULL) {
208 gtk_widget_hide(GTK_WIDGET(menu));
209 return;
210 }
211
212+ priv->root = newroot;
213+ g_object_ref(priv->root);
214+
215 g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_ADDED, G_CALLBACK(root_child_added), menu);
216 g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_MOVED, G_CALLBACK(root_child_moved), menu);
217 g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_REMOVED, G_CALLBACK(root_child_delete), menu);

Subscribers

People subscribed via source and target branches