From af36a127d3a0e69cf941e6a78ac0bb662d2c0401 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 23 Feb 2011 14:34:36 +0000 Subject: Various memory error fixes: - Don't call g_object_add_weak_pointer multiple times on GtkMenuShell's - Ensure we always clean up weak pointers to avoid invalid writes when objects are destroyed - If a GtkWidget is destroyed before it's DbusmenuMenuitem, don't try to access it with g_object_steal_data --- libdbusmenu-gtk/parser.c | 64 +++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 25 deletions(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index f516dde..f2fcb58 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -109,45 +109,58 @@ dbusmenu_gtk_parse_menu_structure (GtkWidget * widget) return recurse.parent; } -/* Called when the dbusmenu item that we're keeping around - is finalized */ static void -dbusmenu_cache_freed (gpointer data, GObject * obj) +parse_data_free (gpointer data) { - /* If the dbusmenu item is killed we don't need to remove - the weak ref as well. */ - g_object_steal_data(G_OBJECT(data), CACHED_MENUITEM); - - ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA); + ParserData *pdata = (ParserData *)data; if (pdata != NULL && pdata->label != NULL) { - g_signal_handlers_disconnect_by_func(pdata->label, G_CALLBACK(label_notify_cb), obj); + g_signal_handlers_disconnect_matched(pdata->label, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(label_notify_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->label), (gpointer*)&pdata->label); } if (pdata != NULL && pdata->action != NULL) { - g_signal_handlers_disconnect_by_func(pdata->action, G_CALLBACK(action_notify_cb), obj); + g_signal_handlers_disconnect_matched(pdata->action, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(action_notify_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->action), (gpointer*)&pdata->action); } if (pdata != NULL && pdata->widget != NULL) { - g_signal_handlers_disconnect_by_func(pdata->widget, G_CALLBACK(widget_notify_cb), obj); + g_signal_handlers_disconnect_matched(pdata->widget, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(widget_notify_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->widget), (gpointer*)&pdata->widget); } if (pdata != NULL && pdata->shell != NULL) { - g_signal_handlers_disconnect_by_func(pdata->shell, G_CALLBACK(child_added_cb), obj); + g_signal_handlers_disconnect_matched(pdata->shell, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(child_added_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->shell), (gpointer*)&pdata->shell); } if (pdata != NULL && pdata->image != NULL) { - g_signal_handlers_disconnect_by_func(pdata->image, G_CALLBACK(image_notify_cb), obj); + g_signal_handlers_disconnect_matched(pdata->image, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(image_notify_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->image), (gpointer*)&pdata->image); } + g_free(pdata); + return; } +/* Called when the dbusmenu item that we're keeping around + is finalized */ +static void +dbusmenu_item_freed (gpointer data, GObject * obj) +{ + ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA); + + if (pdata != NULL && pdata->widget != NULL) { + g_object_steal_data(G_OBJECT(pdata->widget), CACHED_MENUITEM); + } +} + /* Called if we replace the cache on the object with a new dbusmenu menuitem */ static void @@ -198,10 +211,13 @@ new_menuitem (GtkWidget * widget) DbusmenuMenuitem * item = dbusmenu_menuitem_new(); ParserData *pdata = g_new0 (ParserData, 1); - g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, g_free); + g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free); g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed); - g_object_weak_ref(G_OBJECT(item), dbusmenu_cache_freed, widget); + g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL); + + pdata->widget = widget; + g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget); return item; } @@ -237,16 +253,16 @@ parse_menu_structure_helper (GtkWidget * widget, RecurseContext * recurse) if (recurse->parent == NULL) { recurse->parent = new_menuitem(widget); - } - ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(recurse->parent), PARSER_DATA); + ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(recurse->parent), PARSER_DATA); - pdata->shell = widget; - g_signal_connect (G_OBJECT (widget), - "child-added", - G_CALLBACK (child_added_cb), - recurse->parent); - g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->shell); + pdata->shell = widget; + g_signal_connect (G_OBJECT (widget), + "child-added", + G_CALLBACK (child_added_cb), + recurse->parent); + g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->shell); + } gtk_container_foreach (GTK_CONTAINER (widget), (GtkCallback)parse_menu_structure_helper, @@ -472,12 +488,10 @@ construct_dbusmenu_for_widget (GtkWidget * widget) DBUSMENU_MENUITEM_PROP_ENABLED, sensitive); - pdata->widget = widget; g_signal_connect (widget, "notify", G_CALLBACK (widget_notify_cb), mi); - g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget); return mi; } -- cgit v1.2.3 From c90132b2e18252796eabf770ac2334c381b5d1f5 Mon Sep 17 00:00:00 2001 From: Mathieu Trudel-Lapierre Date: Wed, 23 Feb 2011 13:56:01 -0500 Subject: Rework widget_notify_cb() to get the properties directly. --- libdbusmenu-gtk/parser.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index f516dde..b910ee7 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -750,30 +750,34 @@ widget_notify_cb (GtkWidget *widget, gpointer data) { DbusmenuMenuitem *child = (DbusmenuMenuitem *)data; + GValue prop_value = {0}; + + g_value_init (&prop_value, pspec->value_type); + g_object_get_property (G_OBJECT (widget), pspec->name, &prop_value); if (pspec->name == g_intern_static_string ("sensitive")) { dbusmenu_menuitem_property_set_bool (child, DBUSMENU_MENUITEM_PROP_ENABLED, - gtk_widget_get_sensitive (widget)); + g_value_get_boolean (&prop_value)); } else if (pspec->name == g_intern_static_string ("label")) { dbusmenu_menuitem_property_set (child, DBUSMENU_MENUITEM_PROP_LABEL, - gtk_menu_item_get_label (GTK_MENU_ITEM (widget))); + g_value_get_string (&prop_value)); } else if (pspec->name == g_intern_static_string ("visible")) { dbusmenu_menuitem_property_set_bool (child, DBUSMENU_MENUITEM_PROP_VISIBLE, - gtk_widget_get_visible (widget)); + g_value_get_boolean (&prop_value)); } else if (pspec->name == g_intern_static_string ("image") || pspec->name == g_intern_static_string ("always-show-image")) { GtkWidget *image; - image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (widget)); + image = GTK_WIDGET (g_value_get_object (&prop_value)); update_icon (child, GTK_IMAGE (image)); } else if (pspec->name == g_intern_static_string ("parent")) @@ -782,7 +786,7 @@ widget_notify_cb (GtkWidget *widget, * We probably should have added a 'remove' method to the * UbuntuMenuProxy early on, but it's late in the cycle now. */ - if (gtk_widget_get_parent (widget) == NULL) + if (GTK_WIDGET (g_value_get_object (&prop_value)) == NULL) { g_signal_handlers_disconnect_by_func (widget, G_CALLBACK (widget_notify_cb), @@ -818,7 +822,7 @@ widget_notify_cb (GtkWidget *widget, recurse.parent = item; if (item != NULL) { - GtkWidget * menu = gtk_menu_item_get_submenu (GTK_MENU_ITEM (widget)); + GtkWidget * menu = GTK_WIDGET (g_value_get_object (&prop_value)); parse_menu_structure_helper(menu, &recurse); } else { /* Note: it would be really odd that we wouldn't have a cached @@ -827,6 +831,7 @@ widget_notify_cb (GtkWidget *widget, g_object_unref(G_OBJECT(recurse.parent)); } } + g_value_unset (&prop_value); } /* A child item was added to a menu we're watching. Let's try to integrate it. */ -- cgit v1.2.3 From 5653c7e5642ae25fb54445f749ecfb0c947ead0c Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Thu, 24 Feb 2011 15:41:16 +0000 Subject: Disconnect the theme_changed_cb handler when widgets are destroyed --- libdbusmenu-gtk/parser.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index f2fcb58..09b1e6e 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -161,6 +161,15 @@ dbusmenu_item_freed (gpointer data, GObject * obj) } } +static void +widget_freed (gpointer data, GObject * obj) +{ + g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), obj); + + return; +} + +#if 0 /* Called if we replace the cache on the object with a new dbusmenu menuitem */ static void @@ -175,6 +184,7 @@ object_cache_freed (gpointer data) return; } +#endif /* Gets the positon of the child with its' parent if it has one. Returns -1 if the position is unable to be calculated. */ @@ -213,8 +223,9 @@ new_menuitem (GtkWidget * widget) ParserData *pdata = g_new0 (ParserData, 1); g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free); - g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed); + /* g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed); */ g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL); + g_object_weak_ref(G_OBJECT(widget), widget_freed, NULL); pdata->widget = widget; g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget); -- cgit v1.2.3 From 37fb090b4f9fb1e1a894c5c57e7c3d28eb71f92d Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Thu, 24 Feb 2011 15:43:25 +0000 Subject: Hold a weak pointer to each DbusmenuMenuitems parent. This avoids using g_object_{get/set}_data for storing a nodes parent in the parser code, and the associated problems with that (ie, a child out-living its parent, leading to invalid reads --- libdbusmenu-gtk/parser.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 09b1e6e..44ea301 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -321,10 +321,6 @@ parse_menu_structure_helper (GtkWidget * widget, RecurseContext * recurse) /* Oops, let's tell our parents about us */ if (peek == NULL) { - /* TODO: Should we set a weak ref on the parent? */ - g_object_set_data (G_OBJECT (thisitem), - "dbusmenu-parent", - recurse->parent); gint pos = get_child_position (widget); if (pos >= 0) dbusmenu_menuitem_child_add_position (recurse->parent, @@ -813,7 +809,7 @@ widget_notify_cb (GtkWidget *widget, G_CALLBACK (widget_notify_cb), child); - DbusmenuMenuitem *parent = g_object_get_data (G_OBJECT (child), "dbusmenu-parent"); + DbusmenuMenuitem *parent = dbusmenu_menuitem_get_parent (child); if (DBUSMENU_IS_MENUITEM (parent) && DBUSMENU_IS_MENUITEM (child)) { -- cgit v1.2.3 From 017f243186a993adda6b55eac436bb7b6ce784c2 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Thu, 24 Feb 2011 10:22:46 -0600 Subject: Not commenting out code, just deleteing --- libdbusmenu-gtk/parser.c | 18 ------------------ 1 file changed, 18 deletions(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 09b1e6e..a68508b 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -169,23 +169,6 @@ widget_freed (gpointer data, GObject * obj) return; } -#if 0 -/* Called if we replace the cache on the object with a new - dbusmenu menuitem */ -static void -object_cache_freed (gpointer data) -{ - // TODO: make this have access to both data and obj so we can call these - //if (!G_IS_OBJECT(obj)) return; - //g_object_weak_unref(G_OBJECT(obj), dbusmenu_cache_freed, data); - //dbusmenu_cache_freed(data, obj); - - g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), data); - - return; -} -#endif - /* Gets the positon of the child with its' parent if it has one. Returns -1 if the position is unable to be calculated. */ static gint @@ -223,7 +206,6 @@ new_menuitem (GtkWidget * widget) ParserData *pdata = g_new0 (ParserData, 1); g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free); - /* g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed); */ g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL); g_object_weak_ref(G_OBJECT(widget), widget_freed, NULL); -- cgit v1.2.3 From ac984fac079ac48c7687aea3dce5808ace890b3f Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Mon, 28 Feb 2011 09:56:59 -0500 Subject: disconnect some more signals when menuitem dies --- libdbusmenu-gtk/parser.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 97f7979..8045b7f 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -129,6 +129,12 @@ parse_data_free (gpointer data) if (pdata != NULL && pdata->widget != NULL) { g_signal_handlers_disconnect_matched(pdata->widget, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, 0, 0, NULL, G_CALLBACK(widget_notify_cb), NULL); + g_signal_handlers_disconnect_matched(pdata->widget, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(accel_changed), NULL); + g_signal_handlers_disconnect_matched(pdata->widget, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(checkbox_toggled), NULL); + g_signal_handlers_disconnect_matched(pdata->widget, (GSignalMatchType)G_SIGNAL_MATCH_FUNC, + 0, 0, NULL, G_CALLBACK(menuitem_notify_cb), NULL); g_object_remove_weak_pointer(G_OBJECT(pdata->widget), (gpointer*)&pdata->widget); } -- cgit v1.2.3 From b8c5def4dd43b7061a65ba87f65b4b578a9d5c35 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Mon, 28 Feb 2011 18:29:17 +0000 Subject: Ensure we disconnect theme_changed_cb if the DbusmenuMenuitem disappears, but the corresponding GtkMenuItem stays alive. This shouldn't really be a problem, as dbusmenu_item_freed *should* do enough cleaning up to ensure that theme_changed_cb is inert if it fires later on. Just do this to be safe though --- libdbusmenu-gtk/parser.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 97f7979..30c802f 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -149,6 +149,14 @@ parse_data_free (gpointer data) return; } +static void +widget_freed (gpointer data, GObject * obj) +{ + g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), obj); + + return; +} + /* Called when the dbusmenu item that we're keeping around is finalized */ static void @@ -157,18 +165,12 @@ dbusmenu_item_freed (gpointer data, GObject * obj) ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA); if (pdata != NULL && pdata->widget != NULL) { + g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), pdata->widget); g_object_steal_data(G_OBJECT(pdata->widget), CACHED_MENUITEM); + g_object_weak_unref(G_OBJECT(pdata->widget), widget_freed, NULL); } } -static void -widget_freed (gpointer data, GObject * obj) -{ - g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), obj); - - return; -} - /* Gets the positon of the child with its' parent if it has one. Returns -1 if the position is unable to be calculated. */ static gint -- cgit v1.2.3 From d53aa22ffd52ca5d78de74faf7b52832a2ac0663 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 1 Mar 2011 14:45:06 +0000 Subject: Ensure we hook on to child-added signals from sub GtkMenuShell's. This broke in r220 in order to stop adding weak pointers on the root shell more than once. Now, we connect the signals when we create the DbusmenuMenuitem --- libdbusmenu-gtk/parser.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 97f7979..74da3cd 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -450,6 +450,17 @@ construct_dbusmenu_for_widget (GtkWidget * widget) } } + GtkWidget *submenu = gtk_menu_item_get_submenu(GTK_MENU_ITEM(widget)); + if (submenu) + { + pdata->shell = submenu; + g_signal_connect (G_OBJECT (submenu), + "child-added", + G_CALLBACK (child_added_cb), + thisitem); + g_object_add_weak_pointer(G_OBJECT(submenu), (gpointer*)&pdata->shell); + } + if (!g_object_get_data (G_OBJECT (widget), "gtk-empty-menu-item") && !GTK_IS_TEAROFF_MENU_ITEM (widget)) { visible = gtk_widget_get_visible (widget); -- cgit v1.2.3 From e319dcd8d8b2436b1f3bd5f8749b93c08f176251 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 1 Mar 2011 15:16:30 +0000 Subject: Oops, fix a silly typo --- libdbusmenu-gtk/parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libdbusmenu-gtk/parser.c') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 74da3cd..e705dc7 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -457,7 +457,7 @@ construct_dbusmenu_for_widget (GtkWidget * widget) g_signal_connect (G_OBJECT (submenu), "child-added", G_CALLBACK (child_added_cb), - thisitem); + mi); g_object_add_weak_pointer(G_OBJECT(submenu), (gpointer*)&pdata->shell); } -- cgit v1.2.3