From 9ad6aacbbccf55bdb48b6d7453883d0ffe75f313 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 5 Mar 2012 17:55:52 +0000 Subject: [PATCH 2/2] Always call TpProxy async method callbacks in an idle Previously, we made a re-entrant call to the callback if the TpProxy didn't have the interface. Having a method that is "sometimes async" is bad, though: we should follow the GAsyncResult pattern, and have the method return be always-async. In particular, this means we can safely complete a GAsyncResult from the callback, without doing an additional idle. --- telepathy-glib/proxy-methods.c | 18 ++++++++++++++---- tests/dbus/unsupported-interface.c | 9 ++++----- tools/glib-client-gen.py | 25 ++++++++++++------------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/telepathy-glib/proxy-methods.c b/telepathy-glib/proxy-methods.c index a7472bd..8c56ebf 100644 --- a/telepathy-glib/proxy-methods.c +++ b/telepathy-glib/proxy-methods.c @@ -202,7 +202,8 @@ _tp_proxy_pending_call_dgproxy_destroy (DBusGProxy *iface_proxy, * @self: a proxy * @iface: a quark whose string value is the D-Bus interface * @member: the name of the method being called - * @iface_proxy: the interface-specific #DBusGProxy for @iface + * @iface_proxy: the interface-specific #DBusGProxy for @iface, + * or %NULL if the call will immediately fail * @invoke_callback: an implementation of #TpProxyInvokeFunc which will * invoke @callback with appropriate arguments * @callback: a callback to be called when the call completes @@ -270,7 +271,7 @@ tp_proxy_pending_call_v0_new (TpProxy *self, pc->user_data = user_data; pc->destroy = destroy; pc->weak_object = weak_object; - pc->iface_proxy = g_object_ref (iface_proxy); + pc->iface_proxy = NULL; pc->pending_call = NULL; pc->priv = pending_call_magic; pc->cancel_must_raise = cancel_must_raise; @@ -278,8 +279,13 @@ tp_proxy_pending_call_v0_new (TpProxy *self, if (weak_object != NULL) g_object_weak_ref (weak_object, tp_proxy_pending_call_lost_weak_ref, pc); - g_signal_connect (iface_proxy, "destroy", - G_CALLBACK (_tp_proxy_pending_call_dgproxy_destroy), pc); + if (iface_proxy != NULL) + { + pc->iface_proxy = g_object_ref (iface_proxy); + + g_signal_connect (iface_proxy, "destroy", + G_CALLBACK (_tp_proxy_pending_call_dgproxy_destroy), pc); + } return pc; } @@ -444,6 +450,9 @@ tp_proxy_pending_call_v0_completed (gpointer p) * should usually only be called from code generated by * tools/glib-client-gen.py. * + * This function may not be called if @pc was constructed with a %NULL + * #DBusGProxy. + * * Since: 0.7.1 */ void @@ -453,6 +462,7 @@ tp_proxy_pending_call_v0_take_pending_call (TpProxyPendingCall *pc, g_return_if_fail (pc->priv == pending_call_magic); g_return_if_fail (pc->pending_call == NULL); g_return_if_fail (pc->proxy != NULL); + g_return_if_fail (pc->iface_proxy != NULL); pc->pending_call = pending_call; } diff --git a/tests/dbus/unsupported-interface.c b/tests/dbus/unsupported-interface.c index 0f6698a..0b8febb 100644 --- a/tests/dbus/unsupported-interface.c +++ b/tests/dbus/unsupported-interface.c @@ -144,8 +144,7 @@ inbox_url_cb (TpConnection *conn, Fixture *f = user_data; g_assert_no_error (f->error); - /* Unsupported interfaces are signalled by a re-entrant callback in 0.x */ - g_assert (f->reentrant); + g_assert (!f->reentrant); g_assert (!f->freed); if (error != NULL) @@ -166,14 +165,14 @@ test_unsupported_async (Fixture *f, f->conn, -1, inbox_url_cb, f, pretend_to_free, NULL); f->reentrant = FALSE; - /* Unsupported interfaces are signalled by a re-entrant callback in 0.x */ - g_assert (call == NULL); - g_assert (f->freed); + g_assert (call != NULL); + g_assert (!f->freed); while (f->wait) g_main_context_iteration (NULL, TRUE); g_assert_error (f->error, TP_DBUS_ERRORS, TP_DBUS_ERROR_NO_INTERFACE); + g_assert (f->freed); } static void diff --git a/tools/glib-client-gen.py b/tools/glib-client-gen.py index d2b9c12..6919b6b 100644 --- a/tools/glib-client-gen.py +++ b/tools/glib-client-gen.py @@ -798,19 +798,6 @@ class Generator(object): self.b(' (TpProxy *) proxy,') self.b(' interface, (callback == NULL ? NULL : &error));') self.b('') - self.b(' if (iface == NULL && callback != NULL)') - self.b(' {') - self.b(' /* consumes error */') - self.b(' %s ((TpProxy *) proxy,' % invoke_callback) - self.b(' error, NULL, (GCallback) callback, user_data,') - self.b(' weak_object);') - self.b('') - self.b(' if (destroy != NULL)') - self.b(' destroy (user_data);') - self.b('') - self.b(' return NULL;') - self.b(' }') - self.b('') self.b(' if (callback == NULL)') self.b(' {') self.b(' if (iface == NULL)') @@ -838,6 +825,18 @@ class Generator(object): self.b(' %s,' % invoke_callback) self.b(' G_CALLBACK (callback), user_data, destroy,') self.b(' weak_object, FALSE);') + self.b('') + # If iface is NULL then the only valid thing we can do is to + # terminate the call with an error. Go through the machinery + # we'd use for dbus-glib anyway, to stop it being re-entrant. + self.b(' if (iface == NULL)') + self.b(' {') + self.b(' tp_proxy_pending_call_v0_take_results (data,') + self.b(' error, NULL);') + self.b(' tp_proxy_pending_call_v0_completed (data);') + self.b(' return data;') + self.b(' }') + self.b('') self.b(' tp_proxy_pending_call_v0_take_pending_call (data,') self.b(' dbus_g_proxy_begin_call_with_timeout (iface,') self.b(' "%s",' % member) -- 1.7.9.1