Merge lp:~larsu/libdbusmenu/lp1053670 into lp:libdbusmenu/12.10

Proposed by Lars Karlitski
Status: Merged
Approved by: Charles Kerr
Approved revision: 427
Merge reported by: Lars Karlitski
Merged at revision: not available
Proposed branch: lp:~larsu/libdbusmenu/lp1053670
Merge into: lp:libdbusmenu/12.10
Diff against target: 52 lines (+16/-6)
1 file modified
libdbusmenu-gtk/parser.c (+16/-6)
To merge this branch: bzr merge lp:~larsu/libdbusmenu/lp1053670
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
Review via email: mp+126047@code.launchpad.net

Description of the change

This tries to fix bug #1053670. I'm not sure it actually fixes it, because I can't reproduce.

The problem seems to be that settings_notify_cb is called with an invalid DbusmenuMenuitem instance as user_data. This should only happen when the signal is not disconnected properly. I think it isn't, because `widget` might already be NULL in parser_data_free (the menu item has a g_object_add_weak_pointer set on it).

This patch gives ParserData a ref to the GtkSettings object to whose "gtk-menu-images" signal it connects and disconnects the signal from that instance.

To post a comment you must log in.
Revision history for this message
Charles Kerr (charlesk) wrote :

I can't reproduce this crash either, but the patch looks reasonable and I agree with its logic.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libdbusmenu-gtk/parser.c'
--- libdbusmenu-gtk/parser.c 2012-09-10 15:30:56 +0000
+++ libdbusmenu-gtk/parser.c 2012-09-24 16:16:27 +0000
@@ -62,6 +62,7 @@
62 gulong widget_visible_handler_id;62 gulong widget_visible_handler_id;
63 gulong widget_screen_changed_handler_id;63 gulong widget_screen_changed_handler_id;
6464
65 GtkSettings *settings;
65 gulong settings_notify_handler_id;66 gulong settings_notify_handler_id;
6667
67} ParserData;68} ParserData;
@@ -303,14 +304,18 @@
303 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id);304 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id);
304 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id);305 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id);
305 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_screen_changed_handler_id);306 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_screen_changed_handler_id);
306 dbusmenu_gtk_clear_signal_handler (gtk_widget_get_settings (GTK_WIDGET (o)),
307 &pdata->settings_notify_handler_id);
308 g_object_remove_weak_pointer(o, (gpointer*)&pdata->widget);307 g_object_remove_weak_pointer(o, (gpointer*)&pdata->widget);
309308
310 /* since the DbusmenuMenuitem is being destroyed, uncache it from the GtkWidget */309 /* since the DbusmenuMenuitem is being destroyed, uncache it from the GtkWidget */
311 g_object_steal_data(o, CACHED_MENUITEM);310 g_object_steal_data(o, CACHED_MENUITEM);
312 }311 }
313312
313 if (pdata->settings != NULL) {
314 dbusmenu_gtk_clear_signal_handler (pdata->settings,
315 &pdata->settings_notify_handler_id);
316 g_object_unref (pdata->settings);
317 }
318
314 if (pdata->shell != NULL) {319 if (pdata->shell != NULL) {
315 GObject * o = G_OBJECT(pdata->shell);320 GObject * o = G_OBJECT(pdata->shell);
316 dbusmenu_gtk_clear_signal_handler (o, &pdata->item_inserted_handler_id);321 dbusmenu_gtk_clear_signal_handler (o, &pdata->item_inserted_handler_id);
@@ -1309,10 +1314,15 @@
13091314
1310 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(mi), PARSER_DATA);1315 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(mi), PARSER_DATA);
13111316
1312 if (old_screen != NULL)1317 if (pdata->settings != NULL)
1313 dbusmenu_gtk_clear_signal_handler (gtk_settings_get_for_screen (old_screen),1318 {
1314 &pdata->settings_notify_handler_id);1319 dbusmenu_gtk_clear_signal_handler (pdata->settings,
1315 pdata->settings_notify_handler_id = g_signal_connect (gtk_widget_get_settings (widget), "notify",1320 &pdata->settings_notify_handler_id);
1321 g_object_unref (pdata->settings);
1322 }
1323
1324 pdata->settings = g_object_ref (gtk_widget_get_settings (widget));
1325 pdata->settings_notify_handler_id = g_signal_connect (pdata->settings, "notify",
1316 G_CALLBACK (settings_notify_cb), mi);1326 G_CALLBACK (settings_notify_cb), mi);
13171327
1318 /* And update widget now that we have a new GtkSettings */1328 /* And update widget now that we have a new GtkSettings */

Subscribers

People subscribed via source and target branches

to all changes: