From 49b35489b487f82e988e0b41c826e73a53cc2d0b Mon Sep 17 00:00:00 2001
From: Philip Withnall <philip.withnall@collabora.co.uk>
Date: Mon, 5 Jul 2010 17:11:34 +0100
Subject: [PATCH] Rework cancellable handling in tp_contact_request_contact_info_async()

The call to
tp_cli_connection_interface_contact_info_call_request_contact_info() returns
synchronously in the error condition, meaning the cancellable connection is
redundant.

The cancellable doesn't have to be disconnected from in the cancelled
callback, so the idle function for achiving this without deadlock can be
removed.

Also make the async operation complete in an idle callback rather than
synchronously in this case.

Fixes: bfo#28920
---
 telepathy-glib/contact.c |   49 +++++++++++++--------------------------------
 1 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/telepathy-glib/contact.c b/telepathy-glib/contact.c
index 1622a81..c54e7c6 100644
--- a/telepathy-glib/contact.c
+++ b/telepathy-glib/contact.c
@@ -2501,7 +2501,6 @@ typedef struct
   TpProxyPendingCall *call;
   GCancellable *cancellable;
   gulong cancelled_id;
-  guint cancel_idle_id;
 } ContactInfoRequestData;
 
 static void
@@ -2510,10 +2509,6 @@ contact_info_request_data_disconnect_cancellable (ContactInfoRequestData *data)
   if (data->cancelled_id != 0)
     g_cancellable_disconnect (data->cancellable, data->cancelled_id);
   data->cancelled_id = 0;
-
-  if (data->cancel_idle_id != 0)
-    g_source_remove (data->cancel_idle_id);
-  data->cancel_idle_id = 0;
 }
 
 static void
@@ -2541,7 +2536,9 @@ contact_info_request_cb (TpConnection *connection,
   ContactInfoRequestData *data = user_data;
   TpContact *self = data->contact;
 
-  /* At this point it's too late to cancel the operation */
+  /* At this point it's too late to cancel the operation. This will block
+   * until the signal handler has finished if it's already running, so we're
+   * guaranteed to never be in a partially-cancelled state after this call. */
   contact_info_request_data_disconnect_cancellable (data);
 
   if (error != NULL)
@@ -2554,43 +2551,26 @@ contact_info_request_cb (TpConnection *connection,
       contact_maybe_set_info (self, contact_info);
     }
 
-  g_simple_async_result_complete (data->result);
+  g_simple_async_result_complete_in_idle (data->result);
   data->call = NULL;
 }
 
-static gboolean
-contact_info_request_cancelled_idle_cb (gpointer user_data)
+static void
+contact_info_request_cancelled_cb (GCancellable *cancellable,
+    ContactInfoRequestData *data)
 {
-  ContactInfoRequestData *data = user_data;
   GError *error = NULL;
 
-  data->cancel_idle_id = 0;
-
-  if (!g_cancellable_set_error_if_cancelled (data->cancellable, &error))
-    return FALSE;
+  g_assert (g_cancellable_set_error_if_cancelled (data->cancellable, &error));
 
   DEBUG ("Request ContactInfo cancelled");
 
   g_simple_async_result_set_from_error (data->result, error);
-  g_simple_async_result_complete (data->result);
+  g_simple_async_result_complete_in_idle (data->result);
   g_clear_error (&error);
 
   g_assert (data->call != NULL);
   tp_proxy_pending_call_cancel (data->call);
-
-  return FALSE;
-}
-
-static void
-contact_info_request_cancelled_cb (GCancellable *cancellable,
-    ContactInfoRequestData *data)
-{
-  /* Cancellable is locked here, and g_cancellable_disconnect() also try to get
-   * the lock. That could then make deadlock. To be safe we cancel the DBus call
-   * in an idle callback. Use G_PRIORITY_HIGH to be sure our callback comes
-   * before the DBus reply otherwise it could be racy. */
-  data->cancel_idle_id = g_idle_add_full (G_PRIORITY_HIGH,
-      contact_info_request_cancelled_idle_cb, data, NULL);
 }
 
 /**
@@ -2637,12 +2617,6 @@ tp_contact_request_contact_info_async (TpContact *self,
   data->result = g_simple_async_result_new (G_OBJECT (self), callback,
       user_data, tp_contact_request_contact_info_finish);
 
-  data->call = tp_cli_connection_interface_contact_info_call_request_contact_info (
-      self->priv->connection, 60*60*1000, self->priv->handle,
-      contact_info_request_cb,
-      data, (GDestroyNotify) contact_info_request_data_free,
-      NULL);
-
   if (cancellable != NULL)
     {
       data->cancellable = g_object_ref (cancellable);
@@ -2650,6 +2624,11 @@ tp_contact_request_contact_info_async (TpContact *self,
           G_CALLBACK (contact_info_request_cancelled_cb), data, NULL);
     }
 
+  data->call = tp_cli_connection_interface_contact_info_call_request_contact_info (
+      self->priv->connection, 60*60*1000, self->priv->handle,
+      contact_info_request_cb,
+      data, (GDestroyNotify) contact_info_request_data_free,
+      NULL);
 }
 
 /**
-- 
1.7.1