[glib] gdbus: fix deadlock on message cancel/timeout
- From: Dan Winship <danw src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib] gdbus: fix deadlock on message cancel/timeout
- Date: Mon, 6 Apr 2015 16:22:16 +0000 (UTC)
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]