[Pkg-telepathy-commits] [telepathy-glib-1] 33/212: Make sure to register object paths *before* taking bus names

Simon McVittie smcv at debian.org
Wed May 14 12:08:48 UTC 2014


This is an automated email from the git hooks/post-receive script.

smcv pushed a commit to branch debian
in repository telepathy-glib-1.

commit 20ebaaa3abce98661c90cf025aa899988176f843
Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Fri Mar 14 18:39:35 2014 +0000

    Make sure to register object paths *before* taking bus names
    
    GDBus requires us to be much more careful about this than dbus-glib did.
---
 telepathy-glib/base-client.c             | 10 ++--
 telepathy-glib/base-connection-manager.c | 27 +++++++----
 telepathy-glib/base-connection.c         |  5 +-
 telepathy-glib/dbus-daemon.c             | 83 +++++++++++++++++++++++++++-----
 telepathy-glib/dbus-daemon.h             |  4 ++
 5 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/telepathy-glib/base-client.c b/telepathy-glib/base-client.c
index 4f57e14..26c6a14 100644
--- a/telepathy-glib/base-client.c
+++ b/telepathy-glib/base-client.c
@@ -937,6 +937,13 @@ tp_base_client_register (TpBaseClient *self,
 
   DEBUG ("request name %s", self->priv->bus_name);
 
+  if (!tp_dbus_daemon_try_register_object (self->priv->dbus,
+        self->priv->object_path, G_OBJECT (self), error))
+    {
+      DEBUG ("Failed to register object path %s", self->priv->object_path);
+      return FALSE;
+    }
+
   if (!tp_dbus_daemon_request_name (self->priv->dbus, self->priv->bus_name,
         TRUE, error))
     {
@@ -944,9 +951,6 @@ tp_base_client_register (TpBaseClient *self,
       return FALSE;
     }
 
-  tp_dbus_daemon_register_object (self->priv->dbus, self->priv->object_path,
-      G_OBJECT (self));
-
   self->priv->registered = TRUE;
 
   if (!(self->priv->flags & CLIENT_IS_HANDLER))
diff --git a/telepathy-glib/base-connection-manager.c b/telepathy-glib/base-connection-manager.c
index 6e5a7b1..7856f3d 100644
--- a/telepathy-glib/base-connection-manager.c
+++ b/telepathy-glib/base-connection-manager.c
@@ -833,15 +833,13 @@ tp_base_connection_manager_register (TpBaseConnectionManager *self)
 
   g_assert (self->priv->dbus_daemon != NULL);
 
-  string = g_string_new (TP_CM_BUS_NAME_BASE);
-  g_string_append (string, cls->cm_dbus_name);
-
-  if (!tp_dbus_daemon_request_name (self->priv->dbus_daemon, string->str,
-        TRUE, &error))
-    goto except;
+  string = g_string_new (TP_CM_OBJECT_PATH_BASE);
 
   g_string_assign (string, TP_CM_OBJECT_PATH_BASE);
   g_string_append (string, cls->cm_dbus_name);
+
+  /* don't bother handling failure gracefully: CMs should know what
+   * objects they export */
   tp_dbus_daemon_register_object (self->priv->dbus_daemon, string->str, self);
 
   g_hash_table_iter_init (&iter, self->priv->protocols);
@@ -853,7 +851,7 @@ tp_base_connection_manager_register (TpBaseConnectionManager *self)
 
       if (!tp_connection_manager_check_valid_protocol_name (name, &error))
         {
-          g_critical ("%s", error->message);
+          CRITICAL ("%s", error->message);
           goto except;
         }
 
@@ -872,6 +870,18 @@ tp_base_connection_manager_register (TpBaseConnectionManager *self)
           protocol);
     }
 
+  g_string_assign (string, TP_CM_BUS_NAME_BASE);
+  g_string_append (string, cls->cm_dbus_name);
+
+  if (!tp_dbus_daemon_request_name (self->priv->dbus_daemon, string->str,
+        TRUE, &error))
+    {
+      WARNING ("Couldn't claim bus name. If you are trying to debug this "
+          "connection manager, disable all accounts and kill any running "
+          "copies of this CM, then try again. %s", error->message);
+      goto except;
+    }
+
   g_string_free (string, TRUE);
 
   self->priv->registered = TRUE;
@@ -879,9 +889,6 @@ tp_base_connection_manager_register (TpBaseConnectionManager *self)
   return TRUE;
 
 except:
-  WARNING ("Couldn't claim bus name. If you are trying to debug this "
-      "connection manager, disable all accounts and kill any running "
-      "copies of this CM, then try again. %s", error->message);
   g_error_free (error);
 
   if (string != NULL)
diff --git a/telepathy-glib/base-connection.c b/telepathy-glib/base-connection.c
index e654826..5dc1e87 100644
--- a/telepathy-glib/base-connection.c
+++ b/telepathy-glib/base-connection.c
@@ -1411,7 +1411,9 @@ tp_base_connection_register (TpBaseConnection *self,
   g_free (safe_proto);
   g_free (unique_name);
 
-  if (!tp_dbus_daemon_request_name (priv->bus_proxy, priv->bus_name, FALSE,
+  if (!tp_dbus_daemon_try_register_object (priv->bus_proxy, priv->object_path,
+        self, error) ||
+      !tp_dbus_daemon_request_name (priv->bus_proxy, priv->bus_name, FALSE,
         error))
     {
       g_free (priv->bus_name);
@@ -1421,7 +1423,6 @@ tp_base_connection_register (TpBaseConnection *self,
 
   DEBUG ("%p: bus name %s; object path %s", self, priv->bus_name,
       priv->object_path);
-  tp_dbus_daemon_register_object (priv->bus_proxy, priv->object_path, self);
   self->priv->been_registered = TRUE;
 
   if (bus_name != NULL)
diff --git a/telepathy-glib/dbus-daemon.c b/telepathy-glib/dbus-daemon.c
index 32614eb..184d00a 100644
--- a/telepathy-glib/dbus-daemon.c
+++ b/telepathy-glib/dbus-daemon.c
@@ -725,25 +725,71 @@ tp_dbus_daemon_registration_free (gpointer p)
  * Export @object at @object_path. Its `TpSvc` interfaces will all
  * be exported.
  *
+ * It is considered to be a programming error to register an object
+ * at a path where another object already exists.
+ *
  * Since 0.UNRELEASED, as a simplification, exporting an object in this
- * way at more than one location or on more than one bus is not allowed.
+ * way at more than one location or on more than one bus is not allowed,
+ * and is also considered to be a programming error.
+ * However, redundantly re-exporting the same object at the same path
+ * on the same bus is allowed.
  *
- * Since: 0.11.3
+ * Also since 0.UNRELEASED, this function must be called *before* taking any
+ * bus name whose presence is meant to correspond to the existence of this
+ * object. It is *not* sufficient to take the bus name within the same
+ * main-loop iteration as registering the object (even though that
+ * was sufficient under dbus-glib), because GDBus dispatches
+ * method calls in a separate thread.
  */
 void
 tp_dbus_daemon_register_object (TpDBusDaemon *self,
     const gchar *object_path,
     gpointer object)
 {
+  GError *error = NULL;
+
+  if (!tp_dbus_daemon_try_register_object (self, object_path, object, &error))
+    {
+      CRITICAL ("Unable to register %s %p at %s:%s: %s #%d: %s",
+          G_OBJECT_TYPE_NAME (object), object,
+          g_dbus_connection_get_unique_name (
+            tp_proxy_get_dbus_connection (self)),
+          object_path,
+          g_quark_to_string (error->domain), error->code,
+          error->message);
+    }
+}
+
+/**
+ * tp_dbus_daemon_try_register_object:
+ * @self: object representing a connection to a bus
+ * @object_path: an object path
+ * @object: (type GObject.Object) (transfer none): an object to export
+ * @error: used to raise %G_IO_ERROR_EXISTS if an object exists at that path
+ *
+ * The same as tp_dbus_daemon_register_object(), except that it is not
+ * considered to be a programming error to register an object at a path
+ * where another object exists.
+ *
+ * Returns: %TRUE if the object is successfully registered
+ */
+gboolean
+tp_dbus_daemon_try_register_object (TpDBusDaemon *self,
+    const gchar *object_path,
+    gpointer object,
+    GError **error)
+{
   GDBusConnection *conn;
   GType *interfaces;
   guint n = 0;
   guint i;
   Registration *r;
+  gboolean ret = FALSE;
 
-  g_return_if_fail (TP_IS_DBUS_DAEMON (self));
-  g_return_if_fail (tp_dbus_check_valid_object_path (object_path, NULL));
-  g_return_if_fail (G_IS_OBJECT (object));
+  g_return_val_if_fail (TP_IS_DBUS_DAEMON (self), FALSE);
+  g_return_val_if_fail (tp_dbus_check_valid_object_path (object_path, error),
+      FALSE);
+  g_return_val_if_fail (G_IS_OBJECT (object), FALSE);
 
   conn = tp_proxy_get_dbus_connection (self);
   r = g_slice_new0 (Registration);
@@ -774,14 +820,17 @@ tp_dbus_daemon_register_object (TpDBusDaemon *self,
           r->conn == conn)
         {
           DEBUG ("already exported at identical (connection, path), ignoring");
-          return;
+          return TRUE;
         }
 
-      CRITICAL ("object has already been exported on %s (%p) at %s, cannot "
+      CRITICAL ("%s %p has already been exported on %s (%p) at %s, cannot "
           "export on %s (%p) at %s",
+          G_OBJECT_TYPE_NAME (object), object,
           g_dbus_connection_get_unique_name (r->conn), r->conn, r->object_path,
           g_dbus_connection_get_unique_name (conn), conn, object_path);
-      return;
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_BUSY,
+          "Already exported with different connection or object-path");
+      return FALSE;
     }
 
   /* FIXME: if @object is a GDBusObject or GDBusObjectManagerServer,
@@ -794,7 +843,7 @@ tp_dbus_daemon_register_object (TpDBusDaemon *self,
       GType iface = interfaces[i];
       const TpSvcInterfaceInfo *iinfo;
       TpSvcInterfaceSkeleton *skeleton;
-      GError *error = NULL;
+      GError *inner_error = NULL;
 
       iinfo = tp_svc_interface_peek_dbus_interface_info (iface);
 
@@ -808,14 +857,19 @@ tp_dbus_daemon_register_object (TpDBusDaemon *self,
 
       if (!g_dbus_interface_skeleton_export (
             G_DBUS_INTERFACE_SKELETON (skeleton), conn, object_path,
-            &error))
+            &inner_error))
         {
-          CRITICAL ("cannot export %s %p skeleton %p as '%s': %s #%d: %s",
+          DEBUG ("cannot export %s %p skeleton %p as '%s': %s #%d: %s",
               g_type_name (iface), object, skeleton,
               iinfo->interface_info->name,
-              g_quark_to_string (error->domain), error->code, error->message);
+              g_quark_to_string (inner_error->domain), inner_error->code,
+              inner_error->message);
           g_object_unref (skeleton);
-          continue;
+          g_propagate_error (error, inner_error);
+
+          /* roll back */
+          tp_dbus_daemon_unregister_object (self, object);
+          goto finally;
         }
 
       r->skeletons = g_slist_prepend (r->skeletons, skeleton);
@@ -824,7 +878,10 @@ tp_dbus_daemon_register_object (TpDBusDaemon *self,
           iinfo->interface_info->name, skeleton, g_type_name (iface), object);
     }
 
+  ret = TRUE;
+finally:
   g_free (interfaces);
+  return ret;
 }
 
 /**
diff --git a/telepathy-glib/dbus-daemon.h b/telepathy-glib/dbus-daemon.h
index ed5c719..e576ca4 100644
--- a/telepathy-glib/dbus-daemon.h
+++ b/telepathy-glib/dbus-daemon.h
@@ -87,6 +87,10 @@ void tp_dbus_daemon_list_activatable_names (TpDBusDaemon *self,
 
 void tp_dbus_daemon_register_object (TpDBusDaemon *self,
     const gchar *object_path, gpointer object);
+gboolean tp_dbus_daemon_try_register_object (TpDBusDaemon *self,
+    const gchar *object_path,
+    gpointer object,
+    GError **error);
 void tp_dbus_daemon_unregister_object (TpDBusDaemon *self, gpointer object);
 
 G_END_DECLS

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-telepathy/telepathy-glib-1.git



More information about the Pkg-telepathy-commits mailing list