From 3d2e5f00b338e86f041097c6ece6e499f27c6c93 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/2] 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 |   98 ++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 78 insertions(+), 20 deletions(-)

diff --git a/dbus/dbus-gobject.c b/dbus/dbus-gobject.c
index fb92a27..dd1694e 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);
 }
 
 /**
-- 
1.6.2.2