[libgdata] core: Prevent deadlock when closing a cancelled GDataUploadStream
- From: Philip Withnall <pwithnall src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [libgdata] core: Prevent deadlock when closing a cancelled GDataUploadStream
- Date: Sat, 27 Nov 2010 23:13:24 +0000 (UTC)
commit f80a20656ec3cdabc9882670769ebd331495d2f6
Author: Philip Withnall <philip tecnocode co uk>
Date: Sat Nov 27 23:00:15 2010 +0000
core: Prevent deadlock when closing a cancelled GDataUploadStream
When closing a GDataUploadStream which has already been cancelled, the
cancellation callback would be executed in the same thread as
gdata_upload_stream_close(), causing a deadlock (as they both tried to hold
the same lock). The only solution I can think of is to move the cancellation
out to the main thread, which ensures that deadlock will only happen if the
application is doing synchronous I/O (i.e. gdata_upload_stream_close() gets
called in the main thread). If they're doing that, they deserve the deadlock.
Spotted by George Stephanos <gaf stephanos gmail com>. Helps: bgo#607620
gdata/gdata-upload-stream.c | 34 ++++++++++++++++++++++++++++++----
1 files changed, 30 insertions(+), 4 deletions(-)
---
diff --git a/gdata/gdata-upload-stream.c b/gdata/gdata-upload-stream.c
index c0eef5a..4289f20 100644
--- a/gdata/gdata-upload-stream.c
+++ b/gdata/gdata-upload-stream.c
@@ -466,16 +466,42 @@ write:
return length_written;
}
-static void
-close_cancelled_cb (GCancellable *cancellable, GDataUploadStream *self)
+typedef struct {
+ GCancellable *cancellable;
+ GDataUploadStream *upload_stream;
+} CloseData;
+
+static gboolean
+close_cancelled_idle_cb (CloseData *data)
{
- GDataUploadStreamPrivate *priv = self->priv;
+ GDataUploadStreamPrivate *priv = data->upload_stream->priv;
/* Set the error and signal that the close operation has finished */
g_static_mutex_lock (&(priv->response_mutex));
- g_cancellable_set_error_if_cancelled (cancellable, &(priv->response_error));
+ g_cancellable_set_error_if_cancelled (data->cancellable, &(priv->response_error));
g_cond_signal (priv->finished_cond);
g_static_mutex_unlock (&(priv->response_mutex));
+
+ g_object_unref (data->upload_stream);
+ g_object_unref (data->cancellable);
+ g_slice_free (CloseData, data);
+
+ return FALSE;
+}
+
+static void
+close_cancelled_cb (GCancellable *cancellable, GDataUploadStream *self)
+{
+ CloseData *data = g_slice_new (CloseData);
+ data->cancellable = g_object_ref (cancellable);
+ data->upload_stream = g_object_ref (self);
+
+ /* We have to do the cancellation in an idle function so that it's guaranteed not to run in the same thread as gdata_upload_stream_close()
+ * (assuming that gdata_upload_stream_close() isn't being run in the main thread, which would only be the case if the program was doing
+ * synchronous I/O in the main thread â?? in which case they deserve the deadlock they'll get). This is necessary so that the call to
+ * g_cancellable_connect() doesn't deadlock by attempting to re-acquire the response_mutex which is already held by
+ * gdata_upload_stream_close(). */
+ g_idle_add ((GSourceFunc) close_cancelled_idle_cb, data);
}
/* It's guaranteed that we set the response_status and are done with *all* network activity before this returns.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]