Attachment 35253 Details for Bug 27811 – Proposed solution

[patch] Proposed solution

dbus.patch (text/plain), 7.84 KB, created by Johan Redestig on 2010-04-23 03:52:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Johan Redestig

Created: 2010-04-23 03:52:50 UTC

Size: 7.84 KB

patch

obsolete

>From 57bab6c358c276d5c61b46701cd6083a208fa97b Mon Sep 17 00:00:00 2001
>From: Johannes Carlsson <johannes.carlsson.x@sonyericsson.com>
>Date: Tue, 30 Mar 2010 16:37:41 +0200
>Subject: [PATCH] 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.
>
>Change-Id: I9b285f71a08cde349ac0c32081e6d4d5bc0bb13b
>---
> dbus/dbus-connection-internal.h |    1 +
> dbus/dbus-connection.c          |   59 +++++++++++++++++++++++++++++++++++++-
> 2 files changed, 58 insertions(+), 2 deletions(-)
>
>diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
>index 94026a2..fb5e696 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 6a3de5b..1ad054f 100644
>--- a/dbus/dbus-connection.c
>+++ b/dbus/dbus-connection.c
>@@ -316,6 +316,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)
>@@ -1113,14 +1115,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)
> {
>@@ -1136,8 +1146,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);
>     }
> 
>@@ -1923,6 +1947,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);
> 
>@@ -2091,6 +2116,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.
>@@ -2261,6 +2312,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);
>@@ -2332,6 +2384,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);
>@@ -3228,6 +3281,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,
>@@ -3309,6 +3363,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,
>-- 
>1.5.6.3
>

Actions: View | Splinter Review

Attachments on bug 27811: 35253 | 36176 | 36177

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.