luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-11-10 03:16:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

gdbus: fix deadlock on message cancel/timeout

Browse Source 
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
This commit is contained in:
Dan Winship 2015-04-06 10:09:04 -04:00
parent 95d300eac5
commit 7e8d4145af
2 changed files with 45 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
gio/gdbusconnection.c
View File

@ -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 static void
send_message_data_deliver_reply_unlocked (GTask *task, send_message_data_deliver_reply_unlocked (GTask *task,
GDBusMessage *reply) GDBusMessage *reply)
@ -1821,6 +1821,31 @@ send_message_data_deliver_reply_unlocked (GTask *task,
; ;
} }
/* Called from a user thread, lock is not held */
static void
send_message_data_deliver_error (GTask *task,
GQuark domain,
gint code,
const char *message)
{
GDBusConnection *connection = g_task_get_source_object (task);
SendMessageData *data = g_task_get_task_data (task);
CONNECTION_LOCK (connection);
if (data->delivered)
{
CONNECTION_UNLOCK (connection);
return;
}
g_object_ref (task);
send_message_with_reply_cleanup (task, TRUE);
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 */ /* Called from a user thread, lock is not held */
@ -1828,22 +1853,9 @@ static gboolean
send_message_with_reply_cancelled_idle_cb (gpointer user_data) send_message_with_reply_cancelled_idle_cb (gpointer user_data)
{ {
GTask *task = 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); send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_CANCELLED,
if (data->delivered) _("Operation was cancelled"));
goto out;
g_task_return_new_error (task,
G_IO_ERROR,
G_IO_ERROR_CANCELLED,
_("Operation was cancelled"));
send_message_with_reply_cleanup (task, TRUE);
out:
CONNECTION_UNLOCK (connection);
return FALSE; return FALSE;
} }
@ -1871,23 +1883,9 @@ static gboolean
send_message_with_reply_timeout_cb (gpointer user_data) send_message_with_reply_timeout_cb (gpointer user_data)
{ {
GTask *task = 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; return FALSE;
} }

15
gio/tests/gdbus-connection.c
View File

@ -291,6 +291,9 @@ msg_cb_expect_error_disconnected (GDBusConnection *connection,
GError *error; GError *error;
GVariant *result; GVariant *result;
/* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
g_dbus_connection_get_last_serial (connection);
error = NULL; error = NULL;
result = g_dbus_connection_call_finish (connection, result = g_dbus_connection_call_finish (connection,
res, res,
@ -311,6 +314,9 @@ msg_cb_expect_error_unknown_method (GDBusConnection *connection,
GError *error; GError *error;
GVariant *result; GVariant *result;
/* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
g_dbus_connection_get_last_serial (connection);
error = NULL; error = NULL;
result = g_dbus_connection_call_finish (connection, result = g_dbus_connection_call_finish (connection,
res, res,
@ -331,6 +337,9 @@ msg_cb_expect_success (GDBusConnection *connection,
GError *error; GError *error;
GVariant *result; GVariant *result;
/* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
g_dbus_connection_get_last_serial (connection);
error = NULL; error = NULL;
result = g_dbus_connection_call_finish (connection, result = g_dbus_connection_call_finish (connection,
res, res,
@ -350,6 +359,9 @@ msg_cb_expect_error_cancelled (GDBusConnection *connection,
GError *error; GError *error;
GVariant *result; GVariant *result;
/* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
g_dbus_connection_get_last_serial (connection);
error = NULL; error = NULL;
result = g_dbus_connection_call_finish (connection, result = g_dbus_connection_call_finish (connection,
res, res,
@ -370,6 +382,9 @@ msg_cb_expect_error_cancelled_2 (GDBusConnection *connection,
GError *error; GError *error;
GVariant *result; GVariant *result;
/* Make sure gdbusconnection isn't holding @connection's lock. (#747349) */
g_dbus_connection_get_last_serial (connection);
error = NULL; error = NULL;
result = g_dbus_connection_call_finish (connection, result = g_dbus_connection_call_finish (connection,
res, res,