From 5c66205723d16659cd6a4131c22a7e5ac57c6b5d Mon Sep 17 00:00:00 2001 From: Manish Narang Date: Thu, 25 Jan 2018 11:39:44 +0000 Subject: [PATCH 2/3] DBusPendingCall: Only update ->completed under the connection lock If one thread is blocking on a pending call, and another thread is dispatching the connection, then we need them to agree on the value of the completed flag by protecting all accesses with a lock. Reads for this member seem to have the connection lock already, so it's sufficient to make sure that the only write also happens under the connection lock. We already set the completed flag before calling the callback, so it seems OK to stretch it to meaning that some thread has merely *taken responsibility for* calling the callback. The completed flag shares a bitfield with timeout_added, but that flag is protected by the connection lock already. Based on suggestions from Simon McVittie on . Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839 [smcv: Revert indentation changes; add commit message] Reviewed-by: Simon McVittie --- dbus/dbus-connection.c | 5 +++-- dbus/dbus-pending-call-internal.h | 5 ++++- dbus/dbus-pending-call.c | 17 +++++++++++++---- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 2b0a5507..c525b6dc 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2325,10 +2325,11 @@ complete_pending_call_and_unlock (DBusConnection *connection, { _dbus_pending_call_set_reply_unlocked (pending, message); _dbus_pending_call_ref_unlocked (pending); /* in case there's no app with a ref held */ + _dbus_pending_call_start_completion_unlocked(pending); _dbus_connection_detach_pending_call_and_unlock (connection, pending); - + /* Must be called unlocked since it invokes app callback */ - _dbus_pending_call_complete (pending); + _dbus_pending_call_finish_completion (pending); dbus_pending_call_unref (pending); } diff --git a/dbus/dbus-pending-call-internal.h b/dbus/dbus-pending-call-internal.h index a6d7fba7..4f379b4f 100644 --- a/dbus/dbus-pending-call-internal.h +++ b/dbus/dbus-pending-call-internal.h @@ -41,7 +41,10 @@ void _dbus_pending_call_set_reply_serial_unlocked (DBusPendingCal DBusConnection * _dbus_pending_call_get_connection_and_lock (DBusPendingCall *pending); DBusConnection * _dbus_pending_call_get_connection_unlocked (DBusPendingCall *pending); dbus_bool_t _dbus_pending_call_get_completed_unlocked (DBusPendingCall *pending); -void _dbus_pending_call_complete (DBusPendingCall *pending); + +void _dbus_pending_call_start_completion_unlocked (DBusPendingCall *pending); +void _dbus_pending_call_finish_completion (DBusPendingCall *pending); + void _dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending, DBusMessage *message); void _dbus_pending_call_queue_timeout_error_unlocked (DBusPendingCall *pending, diff --git a/dbus/dbus-pending-call.c b/dbus/dbus-pending-call.c index be534105..1bc5d1e5 100644 --- a/dbus/dbus-pending-call.c +++ b/dbus/dbus-pending-call.c @@ -194,18 +194,27 @@ _dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending, } /** - * Calls notifier function for the pending call - * and sets the call to completed. + * Sets the pending call to completed * * @param pending the pending call - * */ void -_dbus_pending_call_complete (DBusPendingCall *pending) +_dbus_pending_call_start_completion_unlocked (DBusPendingCall *pending) { _dbus_assert (!pending->completed); pending->completed = TRUE; +} + +/** + * Call the notifier function for the pending call. + * + * @param pending the pending call + */ +void +_dbus_pending_call_finish_completion (DBusPendingCall *pending) +{ + _dbus_assert (pending->completed); if (pending->function) { -- 2.15.1