Corrected thread problem causing some calls to hang for 25s

Since the connection lock is released for a short while in
_dbus_connection_acquire_io_path there can already be a method return
received by another thread. The fix is to do an extra check after the
I/O path has been aquired both.

Approved-by: Thiago Macieira

2 files changed, 59 insertions, 2 deletions

diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index 721b5d73..cdf3f59d 100644
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -75,6 +75,7 @@ void              _dbus_connection_toggle_timeout_unlocked     (DBusConnection
                                                                 dbus_bool_t         enabled);
 DBusConnection*   _dbus_connection_new_for_transport           (DBusTransport      *transport);
 void              _dbus_connection_do_iteration_unlocked       (DBusConnection     *connection,
+                                                                DBusPendingCall    *pending,
                                                                 unsigned int        flags,
                                                                 int                 timeout_milliseconds);
 void              _dbus_connection_close_possibly_shared       (DBusConnection     *connection);
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 9526d3cc..b3cfa3d9 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -320,6 +320,8 @@ static void               _dbus_connection_release_dispatch                  (DB
 static DBusDispatchStatus _dbus_connection_flush_unlocked                    (DBusConnection     *connection);
 static void               _dbus_connection_close_possibly_shared_and_unlock  (DBusConnection     *connection);
 static dbus_bool_t        _dbus_connection_get_is_connected_unlocked         (DBusConnection     *connection);
+static dbus_bool_t        _dbus_connection_peek_for_reply_unlocked           (DBusConnection     *connection,
+                                                                              dbus_uint32_t       client_serial);
 
 static DBusMessageFilter *
 _dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -1137,14 +1139,22 @@ _dbus_connection_release_io_path (DBusConnection *connection)
  * you specify DBUS_ITERATION_BLOCK; in that case the function
  * returns immediately.
  *
+ * If pending is not NULL then a check is made if the pending call
+ * is completed after the io path has been required. If the call
+ * has been completed nothing is done. This must be done since
+ * the _dbus_connection_acquire_io_path releases the connection
+ * lock for a while.
+ *
  * Called with connection lock held.
  * 
  * @param connection the connection.
+ * @param pending the pending call that should be checked or NULL
  * @param flags iteration flags.
  * @param timeout_milliseconds maximum blocking time, or -1 for no limit.
  */
 void
 _dbus_connection_do_iteration_unlocked (DBusConnection *connection,
+                                        DBusPendingCall *pending,
                                         unsigned int    flags,
                                         int             timeout_milliseconds)
 {
@@ -1160,8 +1170,22 @@ _dbus_connection_do_iteration_unlocked (DBusConnection *connection,
     {
       HAVE_LOCK_CHECK (connection);
       
-      _dbus_transport_do_iteration (connection->transport,
-				    flags, timeout_milliseconds);
+      if ( (pending != NULL) && _dbus_pending_call_get_completed_unlocked(pending))
+        {
+          _dbus_verbose ("pending call completed while acquiring I/O path");
+        }
+      else if ( (pending != NULL) &&
+                _dbus_connection_peek_for_reply_unlocked (connection,
+                                                          _dbus_pending_call_get_reply_serial_unlocked (pending)))
+        {
+          _dbus_verbose ("pending call completed while acquiring I/O path (reply found in queue)");
+        }
+      else
+        {
+          _dbus_transport_do_iteration (connection->transport,
+                                        flags, timeout_milliseconds);
+        }
+
       _dbus_connection_release_io_path (connection);
     }
 
@@ -1989,6 +2013,7 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection       *con
    * out immediately, and otherwise get them queued up
    */
   _dbus_connection_do_iteration_unlocked (connection,
+                                          NULL,
                                           DBUS_ITERATION_DO_WRITING,
                                           -1);
 
@@ -2157,6 +2182,32 @@ generate_local_error_message (dbus_uint32_t serial,
   return message;
 }
 
+/*
+ * Peek the incoming queue to see if we got reply for a specific serial
+ */
+static dbus_bool_t
+_dbus_connection_peek_for_reply_unlocked (DBusConnection *connection,
+                                          dbus_uint32_t   client_serial)
+{
+  DBusList *link;
+  HAVE_LOCK_CHECK (connection);
+
+  link = _dbus_list_get_first_link (&connection->incoming_messages);
+
+  while (link != NULL)
+    {
+      DBusMessage *reply = link->data;
+
+      if (dbus_message_get_reply_serial (reply) == client_serial)
+        {
+          _dbus_verbose ("%s reply to %d found in queue\n", _DBUS_FUNCTION_NAME, client_serial);
+          return TRUE;
+        }
+      link = _dbus_list_get_next_link (&connection->incoming_messages, link);
+    }
+
+  return FALSE;
+}
 
 /* This is slightly strange since we can pop a message here without
  * the dispatch lock.
@@ -2333,6 +2384,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
   /* Now we wait... */
   /* always block at least once as we know we don't have the reply yet */
   _dbus_connection_do_iteration_unlocked (connection,
+                                          pending,
                                           DBUS_ITERATION_DO_READING |
                                           DBUS_ITERATION_BLOCK,
                                           timeout_milliseconds);
@@ -2399,6 +2451,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
         {          
           /* block again, we don't have the reply buffered yet. */
           _dbus_connection_do_iteration_unlocked (connection,
+                                                  pending,
                                                   DBUS_ITERATION_DO_READING |
                                                   DBUS_ITERATION_BLOCK,
                                                   timeout_milliseconds - elapsed_milliseconds);
@@ -2426,6 +2479,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,
                                                   DBUS_ITERATION_DO_READING |
                                                   DBUS_ITERATION_BLOCK,
                                                   timeout_milliseconds - elapsed_milliseconds);
@@ -3403,6 +3457,7 @@ _dbus_connection_flush_unlocked (DBusConnection *connection)
       _dbus_verbose ("doing iteration in %s\n", _DBUS_FUNCTION_NAME);
       HAVE_LOCK_CHECK (connection);
       _dbus_connection_do_iteration_unlocked (connection,
+                                              NULL,
                                               DBUS_ITERATION_DO_READING |
                                               DBUS_ITERATION_DO_WRITING |
                                               DBUS_ITERATION_BLOCK,
@@ -3489,6 +3544,7 @@ _dbus_connection_read_write_dispatch (DBusConnection *connection,
         {
           _dbus_verbose ("doing iteration in %s\n", _DBUS_FUNCTION_NAME);
           _dbus_connection_do_iteration_unlocked (connection,
+                                                  NULL,
                                                   DBUS_ITERATION_DO_READING |
                                                   DBUS_ITERATION_DO_WRITING |
                                                   DBUS_ITERATION_BLOCK,