From 4781afdf451fba9b2446e8d4d386fb735e5c4949 Mon Sep 17 00:00:00 2001 From: Simon McVittie 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