libzeitgeist

Merge lp:~mhr3/libzeitgeist/fix-986230 into lp:libzeitgeist

  1. fix-986230
  2. Merge into trunk
Proposed by Michal Hruby
Status: Merged
Approved by: Mikkel Kamstrup Erlandsen
Approved revision: 238
Merged at revision: 238
Proposed branch: lp:~mhr3/libzeitgeist/fix-986230
Merge into: lp:libzeitgeist
Diff against target: 219 lines (+80/-10)
2 files modified
configure.ac (+1/-1)
src/zeitgeist-log.c (+79/-9)
To merge this branch: bzr merge lp:~mhr3/libzeitgeist/fix-986230
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen Approve
Review via email: mp+103845@code.launchpad.net

Description of the change

Async methods linger indefinitely if getting the dbus proxy right after construction returns an error.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

60 + else if (priv->log_error)
61 + {

Should there not be a g_clear_error() in this branch?

195 + goto process_pending_calls;

Why this? We supposedly don't have a proxy to process the pending calls on?

review: Needs Information
Revision history for this message
Michal Hruby (mhr3) wrote :

> 60 + else if (priv->log_error)
> 61 + {
>
> Should there not be a g_clear_error() in this branch?
>

Nope, we still keep the error for later calls.

> 195 + goto process_pending_calls;
>
> Why this? We supposedly don't have a proxy to process the pending calls on?

We don't have proxy, but we do have an error we'll throw.

Revision history for this message
Siegfried Gevatter (rainct) wrote :

2012/4/27 Mikkel Kamstrup Erlandsen <email address hidden>:
> 60      + else if (priv->log_error)
> 61      + {
>
> Should there not be a g_clear_error() in this branch?

No, the error is still needed.

> 195     + goto process_pending_calls;
>
> Why this? We supposedly don't have a proxy to process the pending calls on?

The code in dispatch_method will run the callback itself giving it the
error code we received when connecting.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

I grilled mhr3 a bit on IRC about this, and I am happy now. This should also be safe for SRU because it only changes the error paths (to the better!).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configure.ac'
2--- configure.ac 2012-04-05 10:11:53 +0000
3+++ configure.ac 2012-04-27 10:21:29 +0000
4@@ -69,7 +69,7 @@
5 ####################################################################
6 # Check library deps
7 ####################################################################
8-GLIB_REQUIRED=2.26
9+GLIB_REQUIRED=2.32
10 PKG_CHECK_MODULES(GLIB2, [glib-2.0 >= $GLIB_REQUIRED ])
11 PKG_CHECK_MODULES(GOBJECT2, [gobject-2.0 >= $GLIB_REQUIRED ])
12 PKG_CHECK_MODULES(GIO2, [gio-2.0 >= $GLIB_REQUIRED ])
13
14=== modified file 'src/zeitgeist-log.c'
15--- src/zeitgeist-log.c 2012-03-26 11:03:34 +0000
16+++ src/zeitgeist-log.c 2012-04-27 10:21:29 +0000
17@@ -57,6 +57,10 @@
18 * If log != NULL it means we have a connection */
19 GDBusProxy *log;
20
21+ /* In case auto-launching the ZG daemon failed, this
22+ * variable will hold the error. */
23+ GError *log_error;
24+
25 /* Hash set of ZeitgeistMonitors we've installed.
26 * We store a map of (monitor, registration_id) */
27 GHashTable *monitors;
28@@ -124,6 +128,7 @@
29
30 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
31 priv->log = NULL;
32+ priv->log_error = NULL;
33
34 /* Reset hash set of monitors */
35 priv->monitors = g_hash_table_new (g_direct_hash, g_direct_equal);
36@@ -173,6 +178,11 @@
37 {
38 g_variant_unref (priv->engine_version);
39 }
40+
41+ if (priv->log_error)
42+ {
43+ g_error_free (priv->log_error);
44+ }
45
46 G_OBJECT_CLASS (zeitgeist_log_parent_class)->finalize (object);
47 }
48@@ -251,6 +261,7 @@
49 dispatch_method (MethodDispatchContext *ctx)
50 {
51 ZeitgeistLogPrivate *priv;
52+ GSimpleAsyncResult *async_result;
53
54 priv = ZEITGEIST_LOG_GET_PRIVATE (ctx->self);
55
56@@ -265,9 +276,32 @@
57 dispatch_async_callback,
58 ctx);
59 }
60+ else if (priv->log_error)
61+ {
62+ // Zeitgeist couldn't be auto-started. We'll run the callback
63+ // anyway and give it an error.
64+ if (ctx->cb != NULL)
65+ {
66+ async_result = g_simple_async_result_new (G_OBJECT (ctx->self),
67+ ctx->cb,
68+ ctx->user_data,
69+ NULL);
70+ g_simple_async_result_set_check_cancellable (async_result,
71+ ctx->cancellable);
72+ g_simple_async_result_set_from_error (async_result, priv->log_error);
73+ g_simple_async_result_complete_in_idle (async_result);
74+ g_object_unref (async_result);
75+ }
76+
77+ g_object_unref (ctx->self);
78+ g_free (ctx);
79+ }
80 else
81- priv->method_dispatch_queue = g_slist_prepend (priv->method_dispatch_queue,
82- ctx);
83+ {
84+ // Queue the request while we wait for Zeitgeist to show up
85+ priv->method_dispatch_queue = g_slist_prepend (priv->method_dispatch_queue,
86+ ctx);
87+ }
88 }
89
90 /* Used to marshal the async callbacks from GDBus into ones
91@@ -496,6 +530,10 @@
92 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
93
94 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
95+
96+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
97+ return NULL;
98+
99 val = g_dbus_proxy_call_finish (priv->log, res, error);
100
101 if (val == NULL)
102@@ -593,6 +631,10 @@
103 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
104
105 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
106+
107+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
108+ return NULL;
109+
110 val = g_dbus_proxy_call_finish (priv->log, res, error);
111
112 if (val == NULL)
113@@ -673,6 +715,10 @@
114 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
115
116 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
117+
118+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
119+ return NULL;
120+
121 val = g_dbus_proxy_call_finish (priv->log, res, error);
122
123 if (val == NULL)
124@@ -741,6 +787,10 @@
125 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
126
127 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
128+
129+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
130+ return NULL;
131+
132 val = g_dbus_proxy_call_finish (priv->log, res, error);
133
134 if (val == NULL)
135@@ -851,6 +901,10 @@
136 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
137
138 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
139+
140+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
141+ return FALSE;
142+
143 val = g_dbus_proxy_call_finish (priv->log, res, error);
144
145 if (val == NULL)
146@@ -928,6 +982,10 @@
147 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
148
149 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
150+
151+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
152+ return NULL;
153+
154 val = g_dbus_proxy_call_finish (priv->log, res, error);
155
156 if (val == NULL)
157@@ -982,6 +1040,10 @@
158 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
159
160 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
161+
162+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
163+ return FALSE;
164+
165 val = g_dbus_proxy_call_finish (priv->log, res, error);
166
167 if (val == NULL)
168@@ -1028,6 +1090,10 @@
169 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
170
171 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
172+
173+ if (priv->log_error && g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
174+ return FALSE;
175+
176 val = g_dbus_proxy_call_finish (priv->log, res, error);
177
178 if (val == NULL)
179@@ -1061,6 +1127,7 @@
180 "but we already have one. Discarding the old and using the "
181 "new one");
182 g_object_unref (priv->log);
183+ g_clear_error (&priv->log_error);
184 priv->log = NULL;
185 }
186
187@@ -1069,9 +1136,10 @@
188
189 if (error != NULL)
190 {
191+ priv->log_error = g_error_copy (error);
192 g_critical ("Failed to create proxy for Zeitgeist daemon: %s",
193 error->message);
194- goto cleanup;
195+ goto process_pending_calls;
196 }
197
198 priv->connection = G_DBUS_CONNECTION (g_object_ref (
199@@ -1097,12 +1165,14 @@
200 priv->is_connected = TRUE;
201 g_object_notify (G_OBJECT (self), "connected");
202
203- /* Dispatch all queued method calls we got while we didn't have a proxy.
204- * Note that dispatch_method() also frees all the queue members */
205- priv->method_dispatch_queue = g_slist_reverse (priv->method_dispatch_queue);
206- g_slist_foreach (priv->method_dispatch_queue, (GFunc) dispatch_method, NULL);
207- g_slist_free (priv->method_dispatch_queue);
208- priv->method_dispatch_queue = NULL;
209+ process_pending_calls:
210+
211+ /* Dispatch all queued method calls we got while we didn't have a proxy.
212+ * Note that dispatch_method() also frees all the queue members */
213+ priv->method_dispatch_queue = g_slist_reverse (priv->method_dispatch_queue);
214+ g_slist_foreach (priv->method_dispatch_queue, (GFunc) dispatch_method, NULL);
215+ g_slist_free (priv->method_dispatch_queue);
216+ priv->method_dispatch_queue = NULL;
217
218 cleanup:
219 g_object_unref (self);

Subscribers

People subscribed via source and target branches