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

Proposed by Ted Gould on 2012-09-28
Status: Merged
Approved by: Ted Gould on 2012-10-01
Approved revision: 2767
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) 2012-09-28 Approve on 2012-10-01
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.
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?

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
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.

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
1=== modified file 'services/panel-indicator-entry-accessible.c'
2--- services/panel-indicator-entry-accessible.c 2012-03-20 16:59:23 +0000
3+++ services/panel-indicator-entry-accessible.c 2012-09-28 19:47:23 +0000
4@@ -30,6 +30,7 @@
5 struct _PanelIndicatorEntryAccessiblePrivate
6 {
7 IndicatorObjectEntry *entry;
8+ GtkMenu * menu;
9 PanelService *service;
10 gint x;
11 gint y;
12@@ -111,6 +112,25 @@
13 }
14
15 static void
16+panel_indicator_entry_accessible_dispose (GObject *object)
17+{
18+ PanelIndicatorEntryAccessible *piea;
19+
20+ g_return_if_fail (PANEL_IS_INDICATOR_ENTRY_ACCESSIBLE (object));
21+
22+ piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (object);
23+
24+ if (piea->priv != NULL)
25+ {
26+ piea->priv->entry = NULL;
27+ g_clear_object(&piea->priv->menu);
28+ }
29+
30+ G_OBJECT_CLASS (panel_indicator_entry_accessible_parent_class)->dispose (object);
31+ return;
32+}
33+
34+static void
35 panel_indicator_entry_accessible_finalize (GObject *object)
36 {
37 PanelIndicatorEntryAccessible *piea;
38@@ -136,6 +156,7 @@
39
40 /* GObject */
41 object_class = G_OBJECT_CLASS (klass);
42+ object_class->dispose = panel_indicator_entry_accessible_dispose;
43 object_class->finalize = panel_indicator_entry_accessible_finalize;
44
45 /* AtkObject */
46@@ -225,6 +246,10 @@
47
48 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);
49 piea->priv->entry = (IndicatorObjectEntry *) data;
50+ if (piea->priv->entry->menu != NULL)
51+ {
52+ piea->priv->menu = g_object_ref(piea->priv->entry->menu);
53+ }
54
55 if (GTK_IS_LABEL (piea->priv->entry->label))
56 {
57@@ -253,7 +278,7 @@
58
59 piea = PANEL_INDICATOR_ENTRY_ACCESSIBLE (accessible);
60
61- if (piea->priv->entry->parent_object && GTK_IS_MENU (piea->priv->entry->menu))
62+ if (piea->priv->entry != NULL && piea->priv->entry->parent_object && piea->priv->menu != NULL && GTK_IS_MENU (piea->priv->menu))
63 n_children = 1;
64
65 return n_children;