[libgdata] core: Fix cancellation handling for writing to a GDataUploadStream



commit 80be38cfb29922a4b0846c1b5496ba3cd3f1f5f4
Author: Philip Withnall <philip tecnocode co uk>
Date:   Thu Dec 16 16:07:10 2010 +0000

    core: Fix cancellation handling for writing to a GDataUploadStream
    
    There was a minor locking issue with write_finished, and deadlocks could have
    occurred if the cancellable was in the process of being cancelled when
    g_cancellable_disconnect() was called (as it was being called with the same
    mutex locked that the cancellation callback attempts to lock).
    
    Helps: bgo#637036

 gdata/gdata-upload-stream.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
---
diff --git a/gdata/gdata-upload-stream.c b/gdata/gdata-upload-stream.c
index e5b021e..0b27648 100644
--- a/gdata/gdata-upload-stream.c
+++ b/gdata/gdata-upload-stream.c
@@ -415,7 +415,7 @@ gdata_upload_stream_write (GOutputStream *stream, const void *buffer, gsize coun
 		priv->response_error = NULL;
 		g_static_mutex_unlock (&(priv->response_mutex));
 
-		if (cancellable != NULL)
+		if (cancelled_signal != 0)
 			g_cancellable_disconnect (cancellable, cancelled_signal);
 
 		return -1;
@@ -423,7 +423,9 @@ gdata_upload_stream_write (GOutputStream *stream, const void *buffer, gsize coun
 	g_static_mutex_unlock (&(priv->response_mutex));
 
 	/* Set write_finished so we know if the write operation has finished before we reach write_cond */
+	g_static_mutex_lock (&(priv->write_mutex));
 	priv->write_finished = FALSE;
+	g_static_mutex_unlock (&(priv->write_mutex));
 
 	/* Handle the more common case of the network thread already having been created first */
 	if (priv->network_thread != NULL) {
@@ -456,7 +458,7 @@ gdata_upload_stream_write (GOutputStream *stream, const void *buffer, gsize coun
 	/* Create the thread and let the writing commence! */
 	create_network_thread (GDATA_UPLOAD_STREAM (stream), error);
 	if (priv->network_thread == NULL) {
-		if (cancellable != NULL)
+		if (cancelled_signal != 0)
 			g_cancellable_disconnect (cancellable, cancelled_signal);
 		return -1;
 	}
@@ -470,10 +472,6 @@ write:
 
 	g_static_mutex_lock (&(priv->response_mutex));
 
-	/* Disconnect from the cancelled signal so we can't receive any more cancel events before we handle errors */
-	if (cancellable != NULL)
-		g_cancellable_disconnect (cancellable, cancelled_signal);
-
 	/* Check for an error and return if necessary */
 	if (priv->response_error != NULL) {
 		g_propagate_error (error, priv->response_error);
@@ -483,6 +481,11 @@ write:
 
 	g_static_mutex_unlock (&(priv->response_mutex));
 
+	/* Disconnect from the cancelled signal. Note that we have to do this with @response_mutex not held, as g_cancellable_disconnect() blocks
+	 * until any outstanding cancellation callbacks return, and they will block on @response_mutex. */
+	if (cancelled_signal != 0)
+		g_cancellable_disconnect (cancellable, cancelled_signal);
+
 	return length_written;
 }
 



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