AppMenu GTK+

Merge lp:~mterry/appmenu-gtk/cancellable-register-window into lp:appmenu-gtk/0.4

  1. cancellable-register-window
  2. Merge into trunk
Proposed by Michael Terry
Status: Merged
Approved by: Ted Gould
Approved revision: 122
Merge reported by: Sebastien Bacher
Merged at revision: not available
Proposed branch: lp:~mterry/appmenu-gtk/cancellable-register-window
Merge into: lp:appmenu-gtk/0.4
Diff against target: 180 lines (+66/-41)
1 file modified
src/bridge.c (+66/-41)
To merge this branch: bzr merge lp:~mterry/appmenu-gtk/cancellable-register-window
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+52248@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Michael Terry (mterry) wrote :

A quick comment about this branch: it fixed the bug for the reporter, though that user then experienced a different crash that he has not been able to reproduce. So this should get a thorough lookover.

Logic seems sound to me though. Basically, the callback for RegisterWindow needs to be cancelled on dispose.

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

Looks good to me.

  review approve
  merge approve

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/bridge.c'
2--- src/bridge.c 2011-03-03 17:19:12 +0000
3+++ src/bridge.c 2011-03-04 20:07:05 +0000
4@@ -73,6 +73,7 @@
5 gboolean registered;
6 AppMenuBridge *bridge;
7 GHashTable *lookup;
8+ GCancellable *cancel;
9 };
10
11 struct _AppMenuBridgePrivate
12@@ -154,24 +155,13 @@
13 }
14
15 static void
16-app_menu_bridge_dispose (GObject *object)
17-{
18- AppMenuBridge *bridge = APP_MENU_BRIDGE (object);
19-
20- if (bridge->priv->appmenuproxy)
21- {
22- g_object_unref (bridge->priv->appmenuproxy);
23- bridge->priv->appmenuproxy = NULL;
24- }
25-}
26-
27-static void
28-free_context_contents (AppWindowContext *context)
29-{
30- if (context->path != NULL)
31- {
32- g_free (context->path);
33- context->path = NULL;
34+context_dispose (AppWindowContext *context)
35+{
36+ if (context->cancel != NULL)
37+ {
38+ g_cancellable_cancel (context->cancel);
39+ g_object_unref (context->cancel);
40+ context->cancel = NULL;
41 }
42
43 if (context->server != NULL)
44@@ -184,11 +174,41 @@
45 {
46 g_signal_handlers_disconnect_by_func(context->window, G_CALLBACK(mnemonic_shown_cb), context);
47 g_object_remove_weak_pointer(G_OBJECT(context->window), (gpointer *)&(context->window));
48+ context->window = NULL;
49 }
50
51- if (context->lookup)
52+ if (context->lookup != NULL)
53 {
54 g_hash_table_unref (context->lookup);
55+ context->lookup = NULL;
56+ }
57+}
58+
59+static void
60+context_free (AppWindowContext *context)
61+{
62+ context_dispose (context);
63+
64+ if (context->path != NULL)
65+ {
66+ g_free (context->path);
67+ context->path = NULL;
68+ }
69+
70+ g_free (context);
71+}
72+
73+static void
74+app_menu_bridge_dispose (GObject *object)
75+{
76+ AppMenuBridge *bridge = APP_MENU_BRIDGE (object);
77+
78+ g_list_foreach (bridge->priv->windows, (GFunc)context_dispose, NULL);
79+
80+ if (bridge->priv->appmenuproxy)
81+ {
82+ g_object_unref (bridge->priv->appmenuproxy);
83+ bridge->priv->appmenuproxy = NULL;
84 }
85 }
86
87@@ -196,17 +216,8 @@
88 app_menu_bridge_finalize (GObject *object)
89 {
90 AppMenuBridge *bridge = APP_MENU_BRIDGE (object);
91- GList *tmp = NULL;
92-
93- for (tmp = bridge->priv->windows; tmp != NULL; tmp = tmp->next)
94- {
95- AppWindowContext *context = tmp->data;
96-
97- free_context_contents (context);
98-
99- g_free (context);
100- }
101-
102+
103+ g_list_foreach (bridge->priv->windows, (GFunc)context_free, NULL);
104 g_list_free (bridge->priv->windows);
105 bridge->priv->windows = NULL;
106
107@@ -225,11 +236,8 @@
108
109 if (context)
110 {
111- free_context_contents (context);
112-
113 context->bridge->priv->windows = g_list_remove (context->bridge->priv->windows, context);
114-
115- g_free (context);
116+ context_free (context);
117 }
118 }
119
120@@ -308,19 +316,33 @@
121 g_variant_unref(variants);
122 }
123
124+ if (error != NULL &&
125+ error->domain == G_IO_ERROR && error->code == G_IO_ERROR_CANCELLED)
126+ {
127+ /* If we were cancelled, we've been disposed (and possibly finalized) and
128+ shouldn't trust anything in context. This is because cancel callbacks
129+ are done in the idle loop for GIO functions. */
130+ g_error_free(error);
131+ return;
132+ }
133+
134+ if (context->cancel != NULL)
135+ {
136+ g_object_unref (context->cancel);
137+ context->cancel = NULL;
138+ }
139+
140 if (error != NULL)
141 {
142 g_warning("Unable to register window with path '%s': %s", context->path, error->message);
143 g_error_free(error);
144
145+ context->registered = FALSE;
146+
147 if (context->bridge != NULL)
148- {
149- context->registered = FALSE;
150-
151 app_menu_bridge_set_show_local (context->bridge, TRUE);
152
153- return;
154- }
155+ return;
156 }
157
158 context->registered = TRUE;
159@@ -347,8 +369,11 @@
160 }
161 }
162
163- if (!context->registered && context->server != NULL && GTK_IS_WINDOW (widget) && bridge->priv->appmenuproxy != NULL)
164+ if (!context->registered && context->server != NULL &&
165+ context->cancel == NULL && GTK_IS_WINDOW (widget) &&
166+ bridge->priv->appmenuproxy != NULL)
167 {
168+ context->cancel = g_cancellable_new ();
169 g_dbus_proxy_call(bridge->priv->appmenuproxy,
170 "RegisterWindow",
171 g_variant_new("(uo)",
172@@ -356,7 +381,7 @@
173 context->path),
174 G_DBUS_CALL_FLAGS_NONE,
175 -1, /* timeout */
176- NULL, /* cancelable */
177+ context->cancel,
178 register_application_window_cb,
179 context);
180 }

Subscribers

People subscribed via source and target branches