Attachment 59798 Details for Bug 45554 – [2/2] channel-contacts: reverse ownership of ContactsQueueItem and result

[patch] [2/2] channel-contacts: reverse ownership of ContactsQueueItem and result

0002-channel-contacts-reverse-ownership-of-ContactsQueueI.patch (text/plain), 5.57 KB, created by Simon McVittie on 2012-04-11 04:16:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2012-04-11 04:16:41 UTC

Size: 5.57 KB

patch

obsolete

>From 4781afdf451fba9b2446e8d4d386fb735e5c4949 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Wed, 11 Apr 2012 12:10:13 +0100
>Subject: [PATCH 2/3] channel-contacts: reverse ownership of ContactsQueueItem
> and result
>
>Previously, the ContactsQueueItem assumed that it owned the only
>reference to the GSimpleAsyncResult. This could result in the
>GSAR still existing after the CQI had been freed, if someone else took
>a reference to the GSAR.
>
>Refcounting the CQI would not solve this problem, because then the CQI
>and the GSAR would have a cyclic reference.
>
>Instead, change the queue of CQIs into a queue of GSARs, and have the
>CQI owned by the GSAR. This means the GSAR's refcounting protects the
>CQI from being freed prematurely.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=45554
>---
> telepathy-glib/channel-contacts.c |   41 ++++++++++++++++++++-----------------
> telepathy-glib/channel-internal.h |    3 ++-
> 2 files changed, 24 insertions(+), 20 deletions(-)
>
>diff --git a/telepathy-glib/channel-contacts.c b/telepathy-glib/channel-contacts.c
>index efaa9a0..aaf394a 100644
>--- a/telepathy-glib/channel-contacts.c
>+++ b/telepathy-glib/channel-contacts.c
>@@ -236,7 +236,6 @@ _tp_channel_contacts_group_init (TpChannel *self,
> 
> struct _ContactsQueueItem
> {
>-  GSimpleAsyncResult *result;
>   GPtrArray *contacts;
>   GPtrArray *ids;
>   GArray *handles;
>@@ -248,8 +247,6 @@ contacts_queue_item_free (ContactsQueueItem *item)
>   tp_clear_pointer (&item->contacts, g_ptr_array_unref);
>   tp_clear_pointer (&item->ids, g_ptr_array_unref);
>   tp_clear_pointer (&item->handles, g_array_unref);
>-  g_clear_object (&item->result);
>-
>   g_slice_free (ContactsQueueItem, item);
> }
> 
>@@ -259,19 +256,19 @@ static void
> contacts_queue_head_ready (TpChannel *self,
>     const GError *error)
> {
>-  ContactsQueueItem *item = self->priv->current_item;
>+  GSimpleAsyncResult *result = self->priv->current_contacts_queue_result;
> 
>   if (error != NULL)
>     {
>       DEBUG ("Error preparing channel contacts queue item: %s", error->message);
>-      g_simple_async_result_set_from_error (item->result, error);
>+      g_simple_async_result_set_from_error (result, error);
>     }
>-  g_simple_async_result_complete (item->result);
>+  g_simple_async_result_complete (result);
> 
>-  self->priv->current_item = NULL;
>+  self->priv->current_contacts_queue_result = NULL;
>   process_contacts_queue (self);
> 
>-  contacts_queue_item_free (item);
>+  g_object_unref (result);
> }
> 
> static void
>@@ -347,11 +344,12 @@ contacts_queue_item_idle_cb (gpointer user_data)
> static void
> process_contacts_queue (TpChannel *self)
> {
>+  GSimpleAsyncResult *result;
>   ContactsQueueItem *item;
>   GArray *features;
>   const GError *error = NULL;
> 
>-  if (self->priv->current_item != NULL)
>+  if (self->priv->current_contacts_queue_result != NULL)
>     return;
> 
>   /* self can't die while there are queued items because item->result keeps a
>@@ -360,21 +358,24 @@ process_contacts_queue (TpChannel *self)
>   if (error != NULL)
>     {
>       g_object_ref (self);
>-      while ((item = g_queue_pop_head (self->priv->contacts_queue)) != NULL)
>+      while ((result = g_queue_pop_head (self->priv->contacts_queue)) != NULL)
>         {
>-          g_simple_async_result_set_from_error (item->result, error);
>-          g_simple_async_result_complete (item->result);
>-          contacts_queue_item_free (item);
>+          g_simple_async_result_set_from_error (result, error);
>+          g_simple_async_result_complete (result);
>+          g_object_unref (result);
>         }
>       g_object_unref (self);
> 
>       return;
>     }
> 
>-  item = g_queue_pop_head (self->priv->contacts_queue);
>-  if (item == NULL)
>+  result = g_queue_pop_head (self->priv->contacts_queue);
>+
>+  if (result == NULL)
>     return;
>-  self->priv->current_item = item;
>+
>+  self->priv->current_contacts_queue_result = result;
>+  item = g_simple_async_result_get_op_res_gpointer (result);
> 
>   features = tp_simple_client_factory_dup_contact_features (
>       tp_proxy_get_factory (self->priv->connection), self->priv->connection);
>@@ -436,16 +437,18 @@ contacts_queue_item (TpChannel *self,
>     gpointer user_data)
> {
>   ContactsQueueItem *item = g_slice_new (ContactsQueueItem);
>+  GSimpleAsyncResult *result;
> 
>   item->contacts = contacts != NULL ? g_ptr_array_ref (contacts) : NULL;
>   item->ids = ids != NULL ? g_ptr_array_ref (ids) : NULL;
>   item->handles = handles != NULL ? g_array_ref (handles) : NULL;
>-  item->result = g_simple_async_result_new ((GObject *) self,
>+  result = g_simple_async_result_new ((GObject *) self,
>       callback, user_data, contacts_queue_item);
> 
>-  g_simple_async_result_set_op_res_gpointer (item->result, item, NULL);
>+  g_simple_async_result_set_op_res_gpointer (result, item,
>+      (GDestroyNotify) contacts_queue_item_free);
> 
>-  g_queue_push_tail (self->priv->contacts_queue, item);
>+  g_queue_push_tail (self->priv->contacts_queue, result);
>   process_contacts_queue (self);
> }
> 
>diff --git a/telepathy-glib/channel-internal.h b/telepathy-glib/channel-internal.h
>index 0f13666..f8700f2 100644
>--- a/telepathy-glib/channel-internal.h
>+++ b/telepathy-glib/channel-internal.h
>@@ -88,9 +88,10 @@ struct _TpChannelPrivate {
>     GHashTable *group_contact_owners;
>     gboolean cm_too_old_for_contacts;
> 
>+    /* Queue of GSimpleAsyncResult with ContactsQueueItem payload */
>     GQueue *contacts_queue;
>     /* Item currently being prepared, not part of contacts_queue anymore */
>-    ContactsQueueItem *current_item;
>+    GSimpleAsyncResult *current_contacts_queue_result;
> 
>     /* NULL, or TpHandle => TpChannelChatState;
>      * if non-NULL, we're watching for ChatStateChanged */
>-- 
>1.7.9.5
>

Actions: View | Splinter Review

Attachments on bug 45554: 56541 | 56542 | 56543 | 56544 | 56545 | 56546 | 56547 | 59796 | 59798

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.