Attachment 24854 Details for Bug 5688 – Yet another iteration of the implementation

[patch] Yet another iteration of the implementation

0001-fd.o-5688-don-t-assert-when-exported-object-is-des.patch (text/plain), 6.47 KB, created by Simon McVittie on 2009-04-16 05:00:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2009-04-16 05:00:33 UTC

Size: 6.47 KB

patch

obsolete

>From 6de1441865da2816c6bcd8cae842be93a8a96304 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Thu, 16 Apr 2009 12:06:26 +0100
>Subject: [PATCH 1/5] fd.o #5688: don't assert when exported object is destroyed *after* D-Bus connection closes
>
>My solution was to introduce an ObjectRegistration struct which
>encapsulates the D-Bus <-> GObject glue.
>
>Also, warn and keep the first object path if the library user registers
>an object at two object paths (previously, this would fail silently, use
>the second object path, and leak memory).
>---
> dbus/dbus-gobject.c |  111 ++++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 88 insertions(+), 23 deletions(-)
>
>diff --git a/dbus/dbus-gobject.c b/dbus/dbus-gobject.c
>index fb92a27..f300c35 100644
>--- a/dbus/dbus-gobject.c
>+++ b/dbus/dbus-gobject.c
>@@ -378,16 +378,51 @@ lookup_object_info_by_iface (GObject     *object,
>   return data.info;
> }
> 
>+typedef struct {
>+    DBusGConnection *connection;
>+    gchar *object_path;
>+    GObject *object;
>+} ObjectRegistration;
>+
>+static void object_registration_object_died (gpointer user_data, GObject *dead);
>+
>+static ObjectRegistration *
>+object_registration_new (DBusGConnection *connection,
>+                         const gchar *object_path,
>+                         GObject *object)
>+{
>+  ObjectRegistration *o = g_slice_new0 (ObjectRegistration);
>+
>+  o->connection = connection;
>+  o->object_path = g_strdup (object_path);
>+  o->object = object;
>+
>+  g_object_weak_ref (o->object, object_registration_object_died, o);
>+
>+  return o;
>+}
>+
> static void
>-gobject_unregister_function (DBusConnection  *connection,
>-                             void            *user_data)
>+object_registration_free (ObjectRegistration *o)
> {
>-  GObject *object;
>+  if (o->object != NULL)
>+    {
>+      g_object_weak_unref (o->object, object_registration_object_died, o);
>+      o->object = NULL;
>+    }
> 
>-  object = G_OBJECT (user_data);
>+  g_free (o->object_path);
> 
>-  /* FIXME */
>+  g_slice_free (ObjectRegistration, o);
>+}
> 
>+/* Called when the object falls off the bus (e.g. because connection just
>+ * closed) */
>+static void
>+object_registration_unregistered (DBusConnection *connection,
>+                                  void *user_data)
>+{
>+  object_registration_free (user_data);
> }
> 
> typedef struct
>@@ -1467,9 +1502,9 @@ invoke_object_method (GObject         *object,
> }
> 
> static DBusHandlerResult
>-gobject_message_function (DBusConnection  *connection,
>-                          DBusMessage     *message,
>-                          void            *user_data)
>+object_registration_message (DBusConnection  *connection,
>+                             DBusMessage     *message,
>+                             void            *user_data)
> {
>   GParamSpec *pspec;
>   GObject *object;
>@@ -1483,8 +1518,10 @@ gobject_message_function (DBusConnection  *connection,
>   const DBusGMethodInfo *method;
>   const DBusGObjectInfo *object_info;
>   DBusMessage *ret;
>+  ObjectRegistration *o;
> 
>-  object = G_OBJECT (user_data);
>+  o = user_data;
>+  object = G_OBJECT (o->object);
> 
>   if (dbus_message_is_method_call (message,
>                                    DBUS_INTERFACE_INTROSPECTABLE,
>@@ -1600,8 +1637,8 @@ gobject_message_function (DBusConnection  *connection,
> }
> 
> static const DBusObjectPathVTable gobject_dbus_vtable = {
>-  gobject_unregister_function,
>-  gobject_message_function,
>+  object_registration_unregistered,
>+  object_registration_message,
>   NULL
> };
> 
>@@ -1963,13 +2000,20 @@ dbus_g_error_domain_register (GQuark                domain,
>   g_static_rw_lock_writer_unlock (&globals_lock);
> }
> 
>+/* Called when the object is destroyed */
> static void
>-unregister_gobject (DBusGConnection *connection, GObject *dead)
>+object_registration_object_died (gpointer user_data, GObject *dead)
> {
>-  char *path;
>-  path = g_object_steal_data (dead, "dbus_glib_object_path");
>-  dbus_connection_unregister_object_path (DBUS_CONNECTION_FROM_G_CONNECTION (connection), path);
>-  g_free (path);
>+  ObjectRegistration *o = user_data;
>+
>+  g_assert (dead == o->object);
>+
>+  /* this prevents the weak unref from taking place, which would cause an
>+   * assertion failure since the object has already gone... */
>+  o->object = NULL;
>+
>+  /* ... while this results in a call to object_registration_unregistered */
>+  dbus_connection_unregister_object_path (DBUS_CONNECTION_FROM_G_CONNECTION (o->connection), o->object_path);
> }
> 
> /**
>@@ -1992,6 +2036,9 @@ dbus_g_connection_register_g_object (DBusGConnection       *connection,
>                                      GObject               *object)
> {
>   GList *info_list;
>+  const gchar *already;
>+  ObjectRegistration *o;
>+
>   g_return_if_fail (connection != NULL);
>   g_return_if_fail (at_path != NULL);
>   g_return_if_fail (G_IS_OBJECT (object));
>@@ -2004,20 +2051,31 @@ dbus_g_connection_register_g_object (DBusGConnection       *connection,
>       return;
>     }
> 
>+  already = g_object_get_data (object, "dbus_glib_object_path");
>+
>+  if (already != NULL)
>+    {
>+      g_warning ("Object already registered at %s, can't re-register at %s",
>+          already, at_path);
>+      return;
>+    }
>+
>+  o = object_registration_new (connection, at_path, object);
>+
>   if (!dbus_connection_register_object_path (DBUS_CONNECTION_FROM_G_CONNECTION (connection),
>                                              at_path,
>                                              &gobject_dbus_vtable,
>-                                             object))
>+                                             o))
>     {
>       g_error ("Failed to register GObject with DBusConnection");
>+      object_registration_free (o);
>       return;
>     }
> 
>   export_signals (connection, info_list, object);
>   g_list_free (info_list);
>-
>-  g_object_set_data (object, "dbus_glib_object_path", g_strdup (at_path));
>-  g_object_weak_ref (object, (GWeakNotify)unregister_gobject, connection);
>+  g_object_set_data_full (object, "dbus_glib_object_path", g_strdup (at_path),
>+      g_free);
> }
> 
> /**
>@@ -2033,10 +2091,17 @@ GObject *
> dbus_g_connection_lookup_g_object (DBusGConnection       *connection,
> 				   const char            *at_path)
> {
>-  gpointer ret;
>-  if (!dbus_connection_get_object_path_data (DBUS_CONNECTION_FROM_G_CONNECTION (connection), at_path, &ret))
>+  gpointer p;
>+  ObjectRegistration *o;
>+
>+  if (!dbus_connection_get_object_path_data (DBUS_CONNECTION_FROM_G_CONNECTION (connection), at_path, &p))
>     return NULL;
>-  return ret;
>+
>+  if (p == NULL)
>+    return NULL;
>+
>+  o = p;
>+  return G_OBJECT (o->object);
> }
> 
> typedef struct {
>-- 
>1.6.2.2
>

Actions: View | Splinter Review

Attachments on bug 5688: 24849 | 24850 | 24851 | 24852 | 24853 | 24854 | 24855

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.