From d356e69465ce3b3bd3a37b1dad7e2f3eb32e641a Mon Sep 17 00:00:00 2001 From: Johannes Carlsson Date: Wed, 9 Jun 2010 11:54:39 +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. --- dbus/dbus-connection-internal.h | 1 + dbus/dbus-connection.c | 60 +++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index 721b5d7..cdf3f59 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 9526d3c..b3cfa3d 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, -- 1.7.0.3