[glib] gdbus: fix deadlock on message cancel/timeout



commit 7e8d4145af745e6ac51337ffcc65872791e7299f
Author: Dan Winship <danw gnome org>
Date:   Mon Apr 6 10:09:04 2015 -0400

    gdbus: fix deadlock on message cancel/timeout
    
    The gdbus GTask port introduced a deadlock because some code had been
    using g_simple_async_result_complete_in_idle() to ensure that the
    callback didn't run until after a mutex was unlocked, but in the gtask
    version, the callback was being run immediately. Fix it to drop the
    mutex before calling g_task_return*(). Also, tweak
    tests/gdbus-connection to test this.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=747349

 gio/gdbusconnection.c        |   58 ++++++++++++++++++++---------------------
 gio/tests/gdbus-connection.c |   15 +++++++++++
 2 files changed, 43 insertions(+), 30 deletions(-)
---
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 50f7920..9ebf6d2 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -1803,7 +1803,7 @@ send_message_with_reply_cleanup (GTask *task, gboolean remove)
 
 /* ---------------------------------------------------------------------------------------------------- */
 
-/* Can be called from any thread with lock held */
+/* Called from GDBus worker thread with lock held */
 static void
 send_message_data_deliver_reply_unlocked (GTask           *task,
                                           GDBusMessage    *reply)
@@ -1821,29 +1821,41 @@ send_message_data_deliver_reply_unlocked (GTask           *task,
   ;
 }
 
-/* ---------------------------------------------------------------------------------------------------- */
-
 /* Called from a user thread, lock is not held */
-static gboolean
-send_message_with_reply_cancelled_idle_cb (gpointer user_data)
+static void
+send_message_data_deliver_error (GTask      *task,
+                                 GQuark      domain,
+                                 gint        code,
+                                 const char *message)
 {
-  GTask *task = user_data;
   GDBusConnection *connection = g_task_get_source_object (task);
   SendMessageData *data = g_task_get_task_data (task);
 
   CONNECTION_LOCK (connection);
   if (data->delivered)
-    goto out;
-
-  g_task_return_new_error (task,
-                           G_IO_ERROR,
-                           G_IO_ERROR_CANCELLED,
-                           _("Operation was cancelled"));
+    {
+      CONNECTION_UNLOCK (connection);
+      return;
+    }
 
+  g_object_ref (task);
   send_message_with_reply_cleanup (task, TRUE);
-
- out:
   CONNECTION_UNLOCK (connection);
+
+  g_task_return_new_error (task, domain, code, "%s", message);
+  g_object_unref (task);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
+/* Called from a user thread, lock is not held */
+static gboolean
+send_message_with_reply_cancelled_idle_cb (gpointer user_data)
+{
+  GTask *task = user_data;
+
+  send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_CANCELLED,
+                                   _("Operation was cancelled"));
   return FALSE;
 }
 
@@ -1871,23 +1883,9 @@ static gboolean
 send_message_with_reply_timeout_cb (gpointer user_data)
 {
   GTask *task = user_data;
-  GDBusConnection *connection = g_task_get_source_object (task);
-  SendMessageData *data = g_task_get_task_data (task);
-
-  CONNECTION_LOCK (connection);
-  if (data->delivered)
-    goto out;
-
-  g_task_return_new_error (task,
-                           G_IO_ERROR,
-                           G_IO_ERROR_TIMED_OUT,
-                           _("Timeout was reached"));
-
-  send_message_with_reply_cleanup (task, TRUE);
-
- out:
-  CONNECTION_UNLOCK (connection);
 
+  send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
+                                   _("Timeout was reached"));
   return FALSE;
 }
 
diff --git a/gio/tests/gdbus-connection.c b/gio/tests/gdbus-connection.c
index 8d0b0df..80cec22 100644
--- a/gio/tests/gdbus-connection.c
+++ b/gio/tests/gdbus-connection.c
@@ -291,6 +291,9 @@ msg_cb_expect_error_disconnected (GDBusConnection *connection,
   GError *error;
   GVariant *result;
 
+  /* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
+  g_dbus_connection_get_last_serial (connection);
+
   error = NULL;
   result = g_dbus_connection_call_finish (connection,
                                           res,
@@ -311,6 +314,9 @@ msg_cb_expect_error_unknown_method (GDBusConnection *connection,
   GError *error;
   GVariant *result;
 
+  /* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
+  g_dbus_connection_get_last_serial (connection);
+
   error = NULL;
   result = g_dbus_connection_call_finish (connection,
                                           res,
@@ -331,6 +337,9 @@ msg_cb_expect_success (GDBusConnection *connection,
   GError *error;
   GVariant *result;
 
+  /* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
+  g_dbus_connection_get_last_serial (connection);
+
   error = NULL;
   result = g_dbus_connection_call_finish (connection,
                                           res,
@@ -350,6 +359,9 @@ msg_cb_expect_error_cancelled (GDBusConnection *connection,
   GError *error;
   GVariant *result;
 
+  /* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
+  g_dbus_connection_get_last_serial (connection);
+
   error = NULL;
   result = g_dbus_connection_call_finish (connection,
                                           res,
@@ -370,6 +382,9 @@ msg_cb_expect_error_cancelled_2 (GDBusConnection *connection,
   GError *error;
   GVariant *result;
 
+  /* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
+  g_dbus_connection_get_last_serial (connection);
+
   error = NULL;
   result = g_dbus_connection_call_finish (connection,
                                           res,


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