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

Proposed by Lars Karlitski on 2012-09-24
Status: Merged
Approved by: Charles Kerr on 2012-09-24
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) 2012-09-24 Approve on 2012-09-24
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.
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
1=== modified file 'libdbusmenu-gtk/parser.c'
2--- libdbusmenu-gtk/parser.c 2012-09-10 15:30:56 +0000
3+++ libdbusmenu-gtk/parser.c 2012-09-24 16:16:27 +0000
4@@ -62,6 +62,7 @@
5 gulong widget_visible_handler_id;
6 gulong widget_screen_changed_handler_id;
7
8+ GtkSettings *settings;
9 gulong settings_notify_handler_id;
10
11 } ParserData;
12@@ -303,14 +304,18 @@
13 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id);
14 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id);
15 dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_screen_changed_handler_id);
16- dbusmenu_gtk_clear_signal_handler (gtk_widget_get_settings (GTK_WIDGET (o)),
17- &pdata->settings_notify_handler_id);
18 g_object_remove_weak_pointer(o, (gpointer*)&pdata->widget);
19
20 /* since the DbusmenuMenuitem is being destroyed, uncache it from the GtkWidget */
21 g_object_steal_data(o, CACHED_MENUITEM);
22 }
23
24+ if (pdata->settings != NULL) {
25+ dbusmenu_gtk_clear_signal_handler (pdata->settings,
26+ &pdata->settings_notify_handler_id);
27+ g_object_unref (pdata->settings);
28+ }
29+
30 if (pdata->shell != NULL) {
31 GObject * o = G_OBJECT(pdata->shell);
32 dbusmenu_gtk_clear_signal_handler (o, &pdata->item_inserted_handler_id);
33@@ -1309,10 +1314,15 @@
34
35 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(mi), PARSER_DATA);
36
37- if (old_screen != NULL)
38- dbusmenu_gtk_clear_signal_handler (gtk_settings_get_for_screen (old_screen),
39- &pdata->settings_notify_handler_id);
40- pdata->settings_notify_handler_id = g_signal_connect (gtk_widget_get_settings (widget), "notify",
41+ if (pdata->settings != NULL)
42+ {
43+ dbusmenu_gtk_clear_signal_handler (pdata->settings,
44+ &pdata->settings_notify_handler_id);
45+ g_object_unref (pdata->settings);
46+ }
47+
48+ pdata->settings = g_object_ref (gtk_widget_get_settings (widget));
49+ pdata->settings_notify_handler_id = g_signal_connect (pdata->settings, "notify",
50 G_CALLBACK (settings_notify_cb), mi);
51
52 /* And update widget now that we have a new GtkSettings */

Subscribers

People subscribed via source and target branches

to all changes: