[next] telepathy-glib: TpContact: stop keeping strong ref on its connection

[Xavier Claessens]

Module: telepathy-glib
Branch: next
Commit: 599eb54867b81df2b4f0229087945b99dcf88bf6
URL:    http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=599eb54867b81df2b4f0229087945b99dcf88bf6

Author: Xavier Claessens <xavier.claessens at collabora.co.uk>
Date:   Fri Jun  1 17:28:54 2012 +0200

TpContact: stop keeping strong ref on its connection

Reset TpContact::connection to NULL when connection is disposed

https://bugs.freedesktop.org/show_bug.cgi?id=49373

---

 telepathy-glib/connection.c |   18 +-----------------
 telepathy-glib/contact.c    |   24 +++++++++++-------------
 2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/telepathy-glib/connection.c b/telepathy-glib/connection.c
index 56bbbed..b5040ed 100644
--- a/telepathy-glib/connection.c
+++ b/telepathy-glib/connection.c
@@ -1027,19 +1027,6 @@ tp_connection_invalidated (TpConnection *self)
       tp_proxy_pending_call_cancel (self->priv->introspection_call);
       self->priv->introspection_call = NULL;
     }
-
-  /* Drop the ref we have on all roster contacts, this is to break the refcycle
-   * we have between TpConnection and TpContact, otherwise self would never
-   * run dispose.
-   * Note that invalidated is also called from dispose, so self->priv->roster
-   * could already be NULL.
-   *
-   * FIXME: When we decide to break tp-glib API/guarantees, we should stop
-   * TpContact taking a strong ref on its TpConnection and force user to keep
-   * a ref on the TpConnection to use its TpContact, this would avoid the
-   * refcycle completely. */
-  if (self->priv->roster != NULL)
-    g_hash_table_remove_all (self->priv->roster);
 }
 
 static void
@@ -1156,10 +1143,7 @@ tp_connection_constructed (GObject *object)
   tp_cli_dbus_properties_call_get_all (self, -1,
       TP_IFACE_CONNECTION, _tp_connection_got_properties, NULL, NULL, NULL);
 
-  /* Give a chance to TpAccount to know about invalidated connection before we
-   * unref all roster contacts. This is to let applications properly remove all
-   * contacts at once instead of getting weak notify on each. */
-  g_signal_connect_after (self, "invalidated",
+  g_signal_connect (self, "invalidated",
       G_CALLBACK (tp_connection_invalidated), NULL);
 }
 
diff --git a/telepathy-glib/contact.c b/telepathy-glib/contact.c
index 8ea339a..cb20b8b 100644
--- a/telepathy-glib/contact.c
+++ b/telepathy-glib/contact.c
@@ -356,7 +356,7 @@ typedef enum {
 
 struct _TpContactPrivate {
     /* basics */
-    TpConnection *connection;
+    TpConnection *connection; /* Weakref */
     TpHandle handle;
     gchar *identifier;
     ContactFeatureFlags has_features;
@@ -992,11 +992,13 @@ tp_contact_set_contact_groups_finish (TpContact *self,
 void
 _tp_contact_connection_disposed (TpContact *contact)
 {
-  /* The connection has gone away, so we no longer have a meaningful handle,
-   * and will never have one again. */
-  g_assert (contact->priv->handle != 0);
-  contact->priv->handle = 0;
-  g_object_notify ((GObject *) contact, "handle");
+  /* This is called from TpConnection::dispose. It is necessary to set
+   * priv->connection to NULL sooner than a weak-notify would do, to prevent
+   * TpContact:dispose from calling _tp_connection_remove_contact() when
+   * TpConnection will unref its roster contacts. */
+  g_assert (contact->priv->connection != NULL);
+  contact->priv->connection = NULL;
+  g_object_notify ((GObject *) contact, "connection");
 }
 
 static void
@@ -1004,17 +1006,13 @@ tp_contact_dispose (GObject *object)
 {
   TpContact *self = TP_CONTACT (object);
 
-  if (self->priv->handle != 0)
+  if (self->priv->connection != NULL)
     {
-      g_assert (self->priv->connection != NULL);
-
       _tp_connection_remove_contact (self->priv->connection,
           self->priv->handle, self);
-
-      self->priv->handle = 0;
+      self->priv->connection = NULL;
     }
 
-  tp_clear_object (&self->priv->connection);
   tp_clear_pointer (&self->priv->location, g_hash_table_unref);
   tp_clear_object (&self->priv->capabilities);
   tp_clear_object (&self->priv->avatar_file);
@@ -1150,7 +1148,7 @@ tp_contact_set_property (GObject *object,
     {
     case PROP_CONNECTION:
       g_assert (self->priv->connection == NULL); /* construct only */
-      self->priv->connection = g_value_dup_object (value);
+      self->priv->connection = g_value_get_object (value);
       break;
     case PROP_HANDLE:
       g_assert (self->priv->handle == 0); /* construct only */