Merge lp:~rainct/libzeitgeist/986230 into lp:libzeitgeist

Proposed by Siegfried Gevatter on 2012-04-21
Status: Merged
Merged at revision: 237
Proposed branch: lp:~rainct/libzeitgeist/986230
Merge into: lp:libzeitgeist
Diff against target: 201 lines (+69/-10)
2 files modified
configure.ac (+1/-1)
src/zeitgeist-log.c (+68/-9)
To merge this branch: bzr merge lp:~rainct/libzeitgeist/986230
Reviewer Review Type Date Requested Status
Michal Hruby (community) 2012-04-21 Needs Fixing on 2012-04-23
Review via email: mp+102971@code.launchpad.net
To post a comment you must log in.
lp:~rainct/libzeitgeist/986230 updated on 2012-04-21
238. By Siegfried Gevatter on 2012-04-21

Call callbacks with g_idle and properly free stuff.

Michal Hruby (mhr3) wrote :

There's still one issue left - if the cancellable is cancelled, _finish needs to throw IO_ERROR_CANCELLED even if it wanted to throw the log_error. Ideally we'd just use g_simple_async_result_set_check_cancellable, but that's available only from 2.32, so we need to implement it manually.

review: Needs Fixing
lp:~rainct/libzeitgeist/986230 updated on 2012-04-23
239. By Siegfried Gevatter on 2012-04-23

Return G_IO_ERROR_CANCELLED when the cancellable is activated

We do this by abusing GSimpleAsyncResult to get a copy of the
cancellable.

Michal Hruby (mhr3) wrote :

Actually, looking at the non-trivial changes this requires, let's just bump the gio dependency and use the check_cancellable method.

lp:~rainct/libzeitgeist/986230 updated on 2012-04-24
240. By Siegfried Gevatter on 2012-04-24

So now we're gonna use g_simple_async_result_set_check_cancellable :)

Michal Hruby (mhr3) wrote :

To make sure this doesn't get lost: (from IRC)

- shouldn't you still check for log_error != NULL in the _finish methods?
- also log_error is not freed in finalize
- and if you use simple..._set_from_error, you don't need to copy it yourself

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-24 15:44:23 +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-24 15:44:23 +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@@ -251,6 +256,8 @@
37 dispatch_method (MethodDispatchContext *ctx)
38 {
39 ZeitgeistLogPrivate *priv;
40+ GSimpleAsyncResult *async_result;
41+ GError *error_copy;
42
43 priv = ZEITGEIST_LOG_GET_PRIVATE (ctx->self);
44
45@@ -265,9 +272,33 @@
46 dispatch_async_callback,
47 ctx);
48 }
49+ else if (priv->log_error)
50+ {
51+ // Zeitgeist couldn't be auto-started. We'll run the callback
52+ // anyway and give it an error.
53+ if (ctx->cb != NULL)
54+ {
55+ async_result = g_simple_async_result_new (G_OBJECT(ctx->self),
56+ ctx->cb,
57+ ctx,
58+ NULL);
59+ g_simple_async_result_set_check_cancellable (async_result,
60+ ctx->cancellable);
61+ error_copy = g_error_copy (priv->log_error);
62+ g_simple_async_result_take_error (async_result, error_copy);
63+ g_simple_async_result_complete_in_idle (async_result);
64+ g_object_unref (async_result);
65+ }
66+
67+ g_object_unref (ctx->self);
68+ g_free (ctx);
69+ }
70 else
71- priv->method_dispatch_queue = g_slist_prepend (priv->method_dispatch_queue,
72- ctx);
73+ {
74+ // Queue the request while we wait for Zeitgeist to show up
75+ priv->method_dispatch_queue = g_slist_prepend (priv->method_dispatch_queue,
76+ ctx);
77+ }
78 }
79
80 /* Used to marshal the async callbacks from GDBus into ones
81@@ -495,6 +526,9 @@
82 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
83 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
84
85+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
86+ return NULL;
87+
88 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
89 val = g_dbus_proxy_call_finish (priv->log, res, error);
90
91@@ -592,6 +626,9 @@
92 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
93 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
94
95+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
96+ return NULL;
97+
98 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
99 val = g_dbus_proxy_call_finish (priv->log, res, error);
100
101@@ -672,6 +709,9 @@
102 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
103 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
104
105+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
106+ return NULL;
107+
108 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
109 val = g_dbus_proxy_call_finish (priv->log, res, error);
110
111@@ -740,6 +780,9 @@
112 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
113 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
114
115+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
116+ return NULL;
117+
118 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
119 val = g_dbus_proxy_call_finish (priv->log, res, error);
120
121@@ -850,6 +893,9 @@
122 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), FALSE);
123 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
124
125+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
126+ return FALSE;
127+
128 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
129 val = g_dbus_proxy_call_finish (priv->log, res, error);
130
131@@ -927,6 +973,9 @@
132 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), NULL);
133 g_return_val_if_fail (error == NULL || *error == NULL, NULL);
134
135+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
136+ return NULL;
137+
138 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
139 val = g_dbus_proxy_call_finish (priv->log, res, error);
140
141@@ -981,6 +1030,9 @@
142 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), FALSE);
143 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
144
145+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
146+ return FALSE;
147+
148 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
149 val = g_dbus_proxy_call_finish (priv->log, res, error);
150
151@@ -1027,6 +1079,9 @@
152 g_return_val_if_fail (G_IS_ASYNC_RESULT (res), FALSE);
153 g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
154
155+ if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
156+ return FALSE;
157+
158 priv = ZEITGEIST_LOG_GET_PRIVATE (self);
159 val = g_dbus_proxy_call_finish (priv->log, res, error);
160
161@@ -1061,6 +1116,7 @@
162 "but we already have one. Discarding the old and using the "
163 "new one");
164 g_object_unref (priv->log);
165+ g_clear_error (&priv->log_error);
166 priv->log = NULL;
167 }
168
169@@ -1069,9 +1125,10 @@
170
171 if (error != NULL)
172 {
173+ priv->log_error = g_error_copy (error);
174 g_critical ("Failed to create proxy for Zeitgeist daemon: %s",
175 error->message);
176- goto cleanup;
177+ goto process_pending_calls;
178 }
179
180 priv->connection = G_DBUS_CONNECTION (g_object_ref (
181@@ -1097,12 +1154,14 @@
182 priv->is_connected = TRUE;
183 g_object_notify (G_OBJECT (self), "connected");
184
185- /* Dispatch all queued method calls we got while we didn't have a proxy.
186- * Note that dispatch_method() also frees all the queue members */
187- priv->method_dispatch_queue = g_slist_reverse (priv->method_dispatch_queue);
188- g_slist_foreach (priv->method_dispatch_queue, (GFunc) dispatch_method, NULL);
189- g_slist_free (priv->method_dispatch_queue);
190- priv->method_dispatch_queue = NULL;
191+ process_pending_calls:
192+
193+ /* Dispatch all queued method calls we got while we didn't have a proxy.
194+ * Note that dispatch_method() also frees all the queue members */
195+ priv->method_dispatch_queue = g_slist_reverse (priv->method_dispatch_queue);
196+ g_slist_foreach (priv->method_dispatch_queue, (GFunc) dispatch_method, NULL);
197+ g_slist_free (priv->method_dispatch_queue);
198+ priv->method_dispatch_queue = NULL;
199
200 cleanup:
201 g_object_unref (self);

Subscribers

People subscribed via source and target branches