[libgdata] core: Don't assume that messages can't be cancelled externally



commit 2ef5875aad2d628578c2f6e4c9f01769594acbd1
Author: Philip Withnall <philip tecnocode co uk>
Date:   Mon May 23 23:08:05 2011 +0100

    core: Don't assume that messages can't be cancelled externally
    
    In _gdata_service_actually_send_message() we were previously assuming that
    we were the only code which could cancel an in-progress message's request.
    
    This isn't true.
    
    soup_session_abort() can be called from anywhere else and will explicitly
    cancel all in-progress requests. This could happen if the GDataService is
    disposed of, which would cause the SoupSession to be disposed of, which
    causes it to abort all in-progress requests. Alternatively, it could happen
    if the GDataService's (and hence SoupSession's) proxy URI is changed.
    
    To fix this, we:
     â?¢ now hold a reference to the session and message in
       _gdata_service_actually_send_message() so that they're kept alive
       throughout the request; and we
     â?¢ no longer assume that a cancelled message was cancelled by us, removing
       the assertion which caused the crash in bgo#650835.
    
    Fixes: bgo#650835

 gdata/gdata-service.c |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)
---
diff --git a/gdata/gdata-service.c b/gdata/gdata-service.c
index 35e112a..6f93931 100644
--- a/gdata/gdata-service.c
+++ b/gdata/gdata-service.c
@@ -1009,6 +1009,12 @@ _gdata_service_actually_send_message (SoupSession *session, SoupMessage *message
 	MessageData data;
 	gulong cancel_signal = 0, request_queued_signal = 0;
 
+	/* Hold references to the session and message so they can't be freed by other threads. For example, if the SoupSession was freed by another
+	 * thread while we were making a request, the request would be unexpectedly cancelled. See bgo#650835 for an example of this breaking things.
+	 */
+	g_object_ref (session);
+	g_object_ref (message);
+
 	/* Listen for cancellation */
 	if (cancellable != NULL) {
 		g_static_mutex_init (&(data.mutex));
@@ -1047,10 +1053,22 @@ _gdata_service_actually_send_message (SoupSession *session, SoupMessage *message
 		g_static_mutex_free (&(data.mutex));
 	}
 
-	/* Set the cancellation error if applicable */
+	/* Set the cancellation error if applicable. We can't assume that our GCancellable has been cancelled just because the message has;
+	 * libsoup may internally cancel messages if, for example, the proxy URI of the SoupSession is changed. */
 	g_assert (message->status_code != SOUP_STATUS_NONE);
-	if (message->status_code == SOUP_STATUS_CANCELLED)
-		g_assert (cancellable != NULL && g_cancellable_set_error_if_cancelled (cancellable, error) == TRUE);
+
+	if (message->status_code == SOUP_STATUS_CANCELLED) {
+		/* We hackily create and cancel a new GCancellable so that we can set the error using it and therefore save ourselves a translatable
+		 * string and the associated maintenance. */
+		GCancellable *error_cancellable = g_cancellable_new ();
+		g_cancellable_cancel (error_cancellable);
+		g_assert (g_cancellable_set_error_if_cancelled (error_cancellable, error) == TRUE);
+		g_object_unref (error_cancellable);
+	}
+
+	/* Free things */
+	g_object_unref (message);
+	g_object_unref (session);
 }
 
 guint



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]