Merge lp:~mterry/libdbusmenu/unref-gtkmenuitems into lp:libdbusmenu/0.5

Proposed by Michael Terry on 2011-10-04
Status: Merged
Approved by: Ted Gould on 2011-10-12
Approved revision: 341
Merged at revision: 342
Proposed branch: lp:~mterry/libdbusmenu/unref-gtkmenuitems
Merge into: lp:libdbusmenu/0.5
Diff against target: 40 lines (+14/-6)
1 file modified
libdbusmenu-gtk/client.c (+14/-6)
To merge this branch: bzr merge lp:~mterry/libdbusmenu/unref-gtkmenuitems
Reviewer Review Type Date Requested Status
Ted Gould (community) 2011-10-04 Approve on 2011-10-12
Review via email: mp+78156@code.launchpad.net

Description of the change

Another result of my investigations into bug 835646. I don't think this is the last leak, but it should help a bit.

GtkMenuItems weren't necessarily being finalized, because we would still hold a reference to them.

dbusmenu_gtkclient_newitem_base would always call g_object_ref_sink, owning the initial floating ref. Then the menu item would often be added to a menu, which would call ref_sink again, adding a normal ref (total of 2 now).

When the corresponding DbusmenuMenuitem would die, we used to just be calling gtk_widget_destroy, which removes the item from any containers and does some other cleanups. Being unparented would reduce the ref count to 1, where it would remain.

So I've added another g_object_unref call to match our initial ref_sink. And some comments to explain why.

To post a comment you must log in.
Ted Gould (ted) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-gtk/client.c'
2--- libdbusmenu-gtk/client.c 2011-08-22 19:54:42 +0000
3+++ libdbusmenu-gtk/client.c 2011-10-04 20:21:15 +0000
4@@ -823,14 +823,22 @@
5 return;
6 }
7
8+static void
9+destroy_gmi (GtkMenuItem * gmi)
10+{
11 #ifdef MASSIVEDEBUGGING
12-static void
13-destroy_gmi (GtkMenuItem * gmi, DbusmenuMenuitem * mi)
14-{
15- g_debug("Destorying GTK Menuitem for %d", dbusmenu_menuitem_get_id(mi));
16+ g_debug("Destroying GTK Menuitem %d", gmi);
17+#endif
18+
19+ /* Call gtk_widget_destroy to remove from any containers and cleanup */
20+ gtk_widget_destroy(GTK_WIDGET(gmi));
21+
22+ /* Now remove last ref that we are holding (due to g_object_ref_sink in
23+ dbusmenu_gtkclient_newitem_base). This should finalize the object */
24+ g_object_unref(G_OBJECT(gmi));
25+
26 return;
27 }
28-#endif
29
30 /**
31 * dbusmenu_gtkclient_newitem_base:
32@@ -857,7 +865,7 @@
33
34 /* Attach these two */
35 g_object_ref_sink(G_OBJECT(gmi));
36- g_object_set_data_full(G_OBJECT(item), data_menuitem, gmi, (GDestroyNotify)gtk_widget_destroy);
37+ g_object_set_data_full(G_OBJECT(item), data_menuitem, gmi, (GDestroyNotify)destroy_gmi);
38
39 /* DbusmenuMenuitem signals */
40 g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK(menu_prop_change_cb), client);

Subscribers

People subscribed via source and target branches