From 6498d83db47a7d83f4bbcad7870086ae94edafbd Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 23 May 2011 23:08:05 +0100 Subject: core: Don't assume that messages can't be cancelled externally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 file changed, 21 insertions(+), 3 deletions(-) diff --git a/gdata/gdata-service.c b/gdata/gdata-service.c index 53b7b608..420eec22 100644 --- a/gdata/gdata-service.c +++ b/gdata/gdata-service.c @@ -991,6 +991,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)); @@ -1029,10 +1035,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 -- cgit v1.2.1