Merge lp:~ted/unity/menu-ref into lp:unity

Proposed by Ted Gould
Status: Merged
Approved by: Ted Gould
Approved revision: no longer in the source branch.
Merged at revision: 2775
Proposed branch: lp:~ted/unity/menu-ref
Merge into: lp:unity
Diff against target: 65 lines (+26/-1)
1 file modified
services/panel-indicator-entry-accessible.c (+26/-1)
To merge this branch: bzr merge lp:~ted/unity/menu-ref
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Review via email: mp+127055@code.launchpad.net

Commit message

Reference the menu in the accessible object

Description of the change

The quick summary is that IndicatorObjectEntry should really be an object. But it's not. So that means we can't have a proper lifecycle for it, and thus we can end up with distruction race conditions. The bug attached finds one. We only really need the menu in that case, so we can keep a reference to that and it'll be cleaned up a little bit later. Not ideal, but fixing all of libindicator is out-of-scope and it is causing a crash for real users.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Mh, this g_clear_object(&piea->priv->menu); seems to be really aggressive if someone else needs that menu... Just unreffing it is not enough?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Mh, this g_clear_object(&piea->priv->menu); seems to be really aggressive if
> someone else needs that menu... Just unreffing it is not enough?

Oh, sorry, I misread the pointer, I tought you were acting on the entry itself...
So, that's fine.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1422/console reported an error when processing this lp:~ted/unity/menu-ref branch.
Not merging it.

Revision history for this message
Ted Gould (ted) wrote :

Looks like a packaging depends blip. Remarking approved to try again.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'services/panel-indicator-entry-accessible.c'
--- services/panel-indicator-entry-accessible.c 2012-03-20 16:59:23 +0000
+++ services/panel-indicator-entry-accessible.c 2012-09-28 19:47:23 +0000
@@ -30,6 +30,7 @@
30struct _PanelIndicatorEntryAccessiblePrivate30struct _PanelIndicatorEntryAccessiblePrivate
31{31{
32 IndicatorObjectEntry *entry;32 IndicatorObjectEntry *entry;
33 GtkMenu * menu;
33 PanelService *service;34 PanelService *service;
34 gint x;35 gint x;
35 gint y;36 gint y;
@@ -111,6 +112,25 @@
111}112}
112113
113static void114static void
115panel_indicator_entry_accessible_dispose (GObject *object)
116{
117 PanelIndicatorEntryAccessible *piea;
118
119 g_return_if_fail (PANEL_IS_INDICATOR_ENTRY_ACCESSIBLE (object));
120
121 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (object);
122
123 if (piea->priv != NULL)
124 {
125 piea->priv->entry = NULL;
126 g_clear_object(&piea->priv->menu);
127 }
128
129 G_OBJECT_CLASS (panel_indicator_entry_accessible_parent_class)->dispose (object);
130 return;
131}
132
133static void
114panel_indicator_entry_accessible_finalize (GObject *object)134panel_indicator_entry_accessible_finalize (GObject *object)
115{135{
116 PanelIndicatorEntryAccessible *piea;136 PanelIndicatorEntryAccessible *piea;
@@ -136,6 +156,7 @@
136156
137 /* GObject */157 /* GObject */
138 object_class = G_OBJECT_CLASS (klass);158 object_class = G_OBJECT_CLASS (klass);
159 object_class->dispose = panel_indicator_entry_accessible_dispose;
139 object_class->finalize = panel_indicator_entry_accessible_finalize;160 object_class->finalize = panel_indicator_entry_accessible_finalize;
140161
141 /* AtkObject */162 /* AtkObject */
@@ -225,6 +246,10 @@
225246
226 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);247 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);
227 piea->priv->entry = (IndicatorObjectEntry *) data;248 piea->priv->entry = (IndicatorObjectEntry *) data;
249 if (piea->priv->entry->menu != NULL)
250 {
251 piea->priv->menu = g_object_ref(piea->priv->entry->menu);
252 }
228253
229 if (GTK_IS_LABEL (piea->priv->entry->label))254 if (GTK_IS_LABEL (piea->priv->entry->label))
230 {255 {
@@ -253,7 +278,7 @@
253278
254 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);279 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);
255280
256 if (piea->priv->entry->parent_object && GTK_IS_MENU (piea->priv->entry->menu))281 if (piea->priv->entry != NULL && piea->priv->entry->parent_object && piea->priv->menu != NULL && GTK_IS_MENU (piea->priv->menu))
257 n_children = 1;282 n_children = 1;
258283
259 return n_children;284 return n_children;