From 9d6dca80bcfb13a08087fa1eb87fd9680f389861 Mon Sep 17 00:00:00 2001
From: Manish Narang <Manish.Narang@kpit.com>
Date: Thu, 25 Jan 2018 11:39:44 +0000
Subject: [PATCH 1/3] DBusConnection: Pass a pending call around more often

If a pending call is provided, _dbus_connection_do_iteration_unlocked
checks whether it has completed or has a reply ready as soon as it
acquires the I/O path. If that's the case, then the iteration
terminates without trying to carry out I/O, so that the pending call
can be dispatched immediately, without blocking until a timeout is
reached. This change is believed to be necessary, but not sufficient,
to resolve #102839.

Based on part of a patch from Michael Searle on
<https://bugs.freedesktop.org/show_bug.cgi?id=102839>.
Commit message added by Simon McVittie.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839
Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 30f8a38b3c8f8756744d6b65dd8207302a683acc)
(cherry picked from commit 1572ca928b466031c9822c1083b7bc6ce63b27c6)
---
 dbus/dbus-connection.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index b574207d..a617457b 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -2525,7 +2525,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
         {          
           /* block again, we don't have the reply buffered yet. */
           _dbus_connection_do_iteration_unlocked (connection,
-                                                  NULL,
+                                                  pending,
                                                   DBUS_ITERATION_DO_READING |
                                                   DBUS_ITERATION_BLOCK,
                                                   timeout_milliseconds - elapsed_milliseconds);
-- 
2.36.0


From 66d42af823ce7d72fee1baf9342aa989e1ef6c73 Mon Sep 17 00:00:00 2001
From: Manish Narang <Manish.Narang@kpit.com>
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
<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>

(cherry picked from commit d3e03eb50eefa5a38d87f274c7de73f36468459c)
(cherry picked from commit a6e5364d842d81b959a558c1242b54c021120c15)
---
 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 a617457b..4d735ea8 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 1875eea8..6ce81b9e 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.36.0


From b94e97dc72953df0befc1fbd76c6e0d01a92f872 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 25 Jan 2018 12:35:07 +0000
Subject: [PATCH 3/3] DBusPendingCall: Improve doc-comments around completed
 flag

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 57a0cf1d14c20765bfc7a36234955b14f3811f2a)
(cherry picked from commit 3c73f2e8f379b3d1f4f8c0db9382472b3a8c3fb9)
---
 dbus/dbus-pending-call.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/dbus/dbus-pending-call.c b/dbus/dbus-pending-call.c
index 1bc5d1e5..d057b49b 100644
--- a/dbus/dbus-pending-call.c
+++ b/dbus/dbus-pending-call.c
@@ -75,8 +75,16 @@ struct DBusPendingCall
   
   dbus_uint32_t reply_serial;                     /**< Expected serial of reply */
 
-  unsigned int completed : 1;                     /**< TRUE if completed */
-  unsigned int timeout_added : 1;                 /**< Have added the timeout */
+  /**
+   * TRUE if some thread has taken responsibility for completing this
+   * pending call: either the pending call has completed, or it is about
+   * to be completed. Protected by the connection lock.
+   */
+  unsigned int completed : 1;
+  /**
+   * TRUE if we have added the timeout. Protected by the connection lock.
+   */
+  unsigned int timeout_added : 1;
 };
 
 static void
@@ -196,6 +204,11 @@ _dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending,
 /**
  * Sets the pending call to completed
  *
+ * This method is called with the connection lock held, to protect
+ * pending->completed. It must be paired with a call to
+ * _dbus_pending_call_finish_completion() after the connection lock has
+ * been released.
+ *
  * @param pending the pending call
  */
 void
@@ -209,6 +222,10 @@ _dbus_pending_call_start_completion_unlocked (DBusPendingCall *pending)
 /**
  * Call the notifier function for the pending call.
  *
+ * This method must be called after the connection lock has been
+ * released, and must be paired with a call to
+ * _dbus_pending_call_start_completion_unlocked().
+ *
  * @param pending the pending call
  */
 void
-- 
2.36.0

