Attachment 62388 Details for Bug 49373 – TpContact: stop keeping strong ref on its connection

[patch] TpContact: stop keeping strong ref on its connection

TpContact-stop-keeping-strong-ref-on-its-connectio.patch (text/plain), 4.27 KB, created by Xavier Claessens on 2012-06-01 09:40:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Xavier Claessens

Created: 2012-06-01 09:40:31 UTC

Size: 4.27 KB

patch

obsolete

>From 619605ee1c01719130b54f56e7c7379db287d487 Mon Sep 17 00:00:00 2001
>From: Xavier Claessens <xavier.claessens@collabora.co.uk>
>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

Actions: View | Splinter Review

Attachments on bug 49373: 62384 | 62385 | 62386 | 62387 | 62388 | 62389 | 62390

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.