[gnome-online-accounts/wip/rishi/gtask: 7/8] httpclient: Make the GCancellable handling thread-safe

[Debarshi Ray <debarshir src gnome org>]

commit 750fbe9d70d2fd0117cbb209734602fd5af9aabc
Author: Debarshi Ray <debarshir gnome org>
Date:   Tue Nov 27 16:25:31 2018 +0100

    httpclient: Make the GCancellable handling thread-safe
    
    A GCancellable can be cancelled from a different thread than the one
    where the GTask for the goa_http_client_check operation was created.
    Since the GCancellable::cancelled handler is invoked in the same thread
    as g_cancellable_call, unguarded access to the GError in CheckData is
    unsafe.
    
    Instead of introducing a GMutex, the GError can be set in the response
    callback which is guaranteed to be invoked in the same thread where the
    GTask was created. Ensuring that the GError is always accessed from the
    same thread makes it considerably easier to reason about its state.
    eg., it can be guaranteed that it won't be set if the status of the
    response isn't SOUP_STATUS_CANCELLED.

 src/goabackend/goahttpclient.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
---
diff --git a/src/goabackend/goahttpclient.c b/src/goabackend/goahttpclient.c
index c763d3f1..06cab1d0 100644
--- a/src/goabackend/goahttpclient.c
+++ b/src/goabackend/goahttpclient.c
@@ -143,19 +143,10 @@ http_client_check_cancelled_cb (GCancellable *cancellable, gpointer user_data)
 
   data = g_task_get_task_data (task);
 
-  if (data->error == NULL)
-    {
-      gboolean cancelled;
-
-      cancelled = g_cancellable_set_error_if_cancelled (cancellable, &data->error);
-
-      /* The callback will be invoked after we have returned to the
-       * main loop.
-       */
-      soup_session_abort (data->session);
-
-      g_return_if_fail (cancelled);
-    }
+  /* The callback will be invoked after we have returned to the main
+   * loop.
+   */
+  soup_session_abort (data->session);
 }
 
 static void
@@ -171,15 +162,22 @@ http_client_check_response_cb (SoupSession *session, SoupMessage *msg, gpointer
    */
   if (msg->status_code == SOUP_STATUS_CANCELLED)
     {
-      g_return_if_fail (data->error != NULL);
+      /* If we are being aborted by the GCancellable then there might
+       * or might not be an error. The GCancellable can be triggered
+       * from a different thread, so it depends on the exact ordering
+       * of events across threads.
+       */
+      if (data->error == NULL)
+        g_cancellable_set_error_if_cancelled (cancellable, &data->error);
+
       goto out;
     }
   else if (msg->status_code != SOUP_STATUS_OK)
     {
       g_warning ("goa_http_client_check() failed: %u — %s", msg->status_code, msg->reason_phrase);
-      if (data->error == NULL)
-        goa_utils_set_error_soup (&data->error, msg);
+      g_return_if_fail (data->error == NULL);
 
+      goa_utils_set_error_soup (&data->error, msg);
       goto out;
     }