From 619605ee1c01719130b54f56e7c7379db287d487 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Fri, 1 Jun 2012 17:28:54 +0200 Subject: [PATCH] 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 085969c..cfba807 100644 --- a/telepathy-glib/connection.c +++ b/telepathy-glib/connection.c @@ -1124,19 +1124,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 gboolean @@ -1284,10 +1271,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 ec9b92f..e5376c2 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 */ -- 1.7.9.5