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
<https://bugs.freedesktop.org/show_bug.cgi?id=102839>.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839
[smcv: Revert indentation changes; add commit message]
Reviewed-by: Simon McVittie <smcv@collabora.com>

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 2b0a5507ca4df536610213ab46a4535242a8598c..c525b6dc19fae70a15f669bd139b0aaa6ca4856f 100644 (file)
--- 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 a6d7fba7c2190b8bc65e4b9ff4c6d391bb76f173..4f379b4fa48e3cc126c7ad82664b77d0f5f5ca55 100644 (file)
--- 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 be5341058ad9b1d3a45259bc074370b86bd0dfa4..1bc5d1e51b6a13798d40ceb7059f62b80a1d8cb9 100644 (file)
--- 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)
     {