[libgdata] core: Don't use a deallocated mutex for early-cancelled messages

[Philip Withnall <pwithnall src gnome org>]

commit 3a045028895789efdf1248c10a483996426d6003
Author: Philip Withnall <philip tecnocode co uk>
Date:   Thu Dec 30 15:07:09 2010 +0000

    core: Don't use a deallocated mutex for early-cancelled messages
    
    In the case that a message is cancelled before it's queued in the session
    (i.e. the cancellation callback is called immediately by
    g_cancellable_connect()), ensure we don't free the mutex which we later
    attempt to lock.

 gdata/gdata-service.c |   34 +++++++++++++---------------------
 1 files changed, 13 insertions(+), 21 deletions(-)
---
diff --git a/gdata/gdata-service.c b/gdata/gdata-service.c
index dec5ec9..53b7b60 100644
--- a/gdata/gdata-service.c
+++ b/gdata/gdata-service.c
@@ -966,16 +966,6 @@ typedef struct {
 } MessageData;
 
 static void
-message_data_free (MessageData *data)
-{
-	g_object_unref (data->message);
-	g_object_unref (data->session);
-	g_static_mutex_free (&(data->mutex));
-
-	g_slice_free (MessageData, data);
-}
-
-static void
 message_cancel_cb (GCancellable *cancellable, MessageData *data)
 {
 	g_static_mutex_lock (&(data->mutex));
@@ -998,18 +988,17 @@ message_request_queued_cb (SoupSession *session, SoupMessage *message, MessageDa
 void
 _gdata_service_actually_send_message (SoupSession *session, SoupMessage *message, GCancellable *cancellable, GError **error)
 {
-	MessageData *data;
+	MessageData data;
 	gulong cancel_signal = 0, request_queued_signal = 0;
 
 	/* Listen for cancellation */
 	if (cancellable != NULL) {
-		data = g_slice_new (MessageData);
-		g_static_mutex_init (&(data->mutex));
-		data->session = g_object_ref (session);
-		data->message = g_object_ref (message);
+		g_static_mutex_init (&(data.mutex));
+		data.session = session;
+		data.message = message;
 
-		cancel_signal = g_cancellable_connect (cancellable, (GCallback) message_cancel_cb, data, (GDestroyNotify) message_data_free);
-		request_queued_signal = g_signal_connect (session, "request-queued", (GCallback) message_request_queued_cb, data);
+		cancel_signal = g_cancellable_connect (cancellable, (GCallback) message_cancel_cb, &data, NULL);
+		request_queued_signal = g_signal_connect (session, "request-queued", (GCallback) message_request_queued_cb, &data);
 
 		/* We lock this mutex until the message has been queued by the session (i.e. it's unlocked in the request-queued callback), and require
 		 * the mutex to be held to cancel the message. Consequently, if the message is cancelled (in another thread) any time between this lock
@@ -1017,7 +1006,7 @@ _gdata_service_actually_send_message (SoupSession *session, SoupMessage *message
 		 * This is a little ugly, but is the only way I can think of to avoid a race condition between calling soup_session_cancel_message()
 		 * and soup_session_send_message(), as the former doesn't have any effect until the request has been queued, and once the latter has
 		 * returned, all network activity has been finished so cancellation is pointless. */
-		g_static_mutex_lock (&(data->mutex));
+		g_static_mutex_lock (&(data.mutex));
 	}
 
 	/* Only send the message if it hasn't already been cancelled. There is no race condition here for the above reasons: if the cancellable has
@@ -1031,11 +1020,14 @@ _gdata_service_actually_send_message (SoupSession *session, SoupMessage *message
 		soup_message_set_status (message, SOUP_STATUS_CANCELLED);
 
 	/* Clean up the cancellation code */
-	if (cancellable != NULL)
+	if (cancellable != NULL) {
 		g_signal_handler_disconnect (session, request_queued_signal);
 
-	if (cancel_signal != 0)
-		g_cancellable_disconnect (cancellable, cancel_signal);
+		if (cancel_signal != 0)
+			g_cancellable_disconnect (cancellable, cancel_signal);
+
+		g_static_mutex_free (&(data.mutex));
+	}
 
 	/* Set the cancellation error if applicable */
 	g_assert (message->status_code != SOUP_STATUS_NONE);