From da4182fb2976608bea64d676677681fdf2cd910b Mon Sep 17 00:00:00 2001 From: "John (J5) Palmieri" Date: Thu, 2 Mar 2006 22:24:28 +0000 Subject: 2006-03-02 John (J5) Palmieri * dbus/dbus-connection.c: (_dbus_connection_block_pending_call): Check to see if our data has already been read off the connection by another blocking pending call before we block in poll. (check_for_reply_and_update_dispatch_unlocked): Code taken from _dbus_connection_block_pending_call - checks for an already read reply and updates the dispatch if there is one. * test/name-test/test-pending-call-dispatch.c: New test for making sure we don't get stuck polling a dbus connection which has no data on the socket when blocking out of order on two or more pending calls. --- ChangeLog | 15 ++++ dbus/dbus-connection.c | 58 ++++++++----- test/name-test/Makefile.am | 9 +- test/name-test/run-test.sh | 3 + test/name-test/test-pending-call-dispatch.c | 123 ++++++++++++++++++++++++++++ 5 files changed, 186 insertions(+), 22 deletions(-) create mode 100644 test/name-test/test-pending-call-dispatch.c diff --git a/ChangeLog b/ChangeLog index e0fcc27e..262b4d4a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2006-03-02 John (J5) Palmieri + + * dbus/dbus-connection.c: + (_dbus_connection_block_pending_call): + Check to see if our data has already been read off the connection + by another blocking pending call before we block in poll. + (check_for_reply_and_update_dispatch_unlocked): + Code taken from _dbus_connection_block_pending_call - checks for + an already read reply and updates the dispatch if there is one. + + * test/name-test/test-pending-call-dispatch.c: + New test for making sure we don't get stuck polling a + dbus connection which has no data on the socket when + blocking out of order on two or more pending calls. + 2006-02-28 Thiago Macieira * qt/Makefile.am: Patch by Sjoerd Simons. More .moc issues: diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 0c9fe28d..b84491bc 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2533,6 +2533,36 @@ check_for_reply_unlocked (DBusConnection *connection, return NULL; } +static dbus_bool_t +check_for_reply_and_update_dispatch_unlocked (DBusPendingCall *pending) +{ + DBusMessage *reply; + DBusDispatchStatus status; + DBusConnection *connection; + + connection = pending->connection; + + reply = check_for_reply_unlocked (connection, pending->reply_serial); + if (reply != NULL) + { + _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME); + + _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n"); + + _dbus_pending_call_complete_and_unlock (pending, reply); + dbus_message_unref (reply); + + CONNECTION_LOCK (connection); + status = _dbus_connection_get_dispatch_status_unlocked (connection); + _dbus_connection_update_dispatch_status_and_unlock (connection, status); + dbus_pending_call_unref (pending); + + return TRUE; + } + + return FALSE; +} + /** * When a function that blocks has been called with a timeout, and we * run out of memory, the time to wait for memory is based on the @@ -2616,6 +2646,11 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) start_tv_sec, start_tv_usec, end_tv_sec, end_tv_usec); + /* check to see if we already got the data off the socket */ + /* from another blocked pending call */ + if (check_for_reply_and_update_dispatch_unlocked (pending)) + return; + /* Now we wait... */ /* always block at least once as we know we don't have the reply yet */ _dbus_connection_do_iteration_unlocked (connection, @@ -2645,27 +2680,8 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) } if (status == DBUS_DISPATCH_DATA_REMAINS) - { - DBusMessage *reply; - - reply = check_for_reply_unlocked (connection, client_serial); - if (reply != NULL) - { - _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME); - - _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n"); - - _dbus_pending_call_complete_and_unlock (pending, reply); - dbus_message_unref (reply); - - CONNECTION_LOCK (connection); - status = _dbus_connection_get_dispatch_status_unlocked (connection); - _dbus_connection_update_dispatch_status_and_unlock (connection, status); - dbus_pending_call_unref (pending); - - return; - } - } + if (check_for_reply_and_update_dispatch_unlocked (pending)) + return; _dbus_get_current_time (&tv_sec, &tv_usec); diff --git a/test/name-test/Makefile.am b/test/name-test/Makefile.am index 7e931c8a..34dd85a8 100644 --- a/test/name-test/Makefile.am +++ b/test/name-test/Makefile.am @@ -16,11 +16,18 @@ if DBUS_BUILD_TESTS ## we use noinst_PROGRAMS not check_PROGRAMS for TESTS so that we ## build even when not doing "make check" -noinst_PROGRAMS=test-names +noinst_PROGRAMS=test-names test-pending-call-dispatch test_names_SOURCES= \ test-names.c test_names_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la + +test_pending_call_dispatch_SOURCES = \ + test-pending-call-dispatch.c + +test_pending_call_dispatch_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la + + endif diff --git a/test/name-test/run-test.sh b/test/name-test/run-test.sh index 5171b18c..706d5950 100755 --- a/test/name-test/run-test.sh +++ b/test/name-test/run-test.sh @@ -28,3 +28,6 @@ if test -z "$DBUS_TEST_NAME_IN_RUN_TEST"; then fi echo "running test-names" libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-names || die "test-client failed" + +echo "running test-pending-call-dispatch" +libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-pending-call-dispatch || die "test-client failed" diff --git a/test/name-test/test-pending-call-dispatch.c b/test/name-test/test-pending-call-dispatch.c new file mode 100644 index 00000000..4df2ed37 --- /dev/null +++ b/test/name-test/test-pending-call-dispatch.c @@ -0,0 +1,123 @@ +/** +* Test to make sure we don't get stuck polling a dbus connection +* which has no data on the socket. This was an issue where +* one pending call would read all the data off the bus +* and the second pending call would not check to see +* if its data had already been read before polling the connection +* and blocking. +**/ + +#include +#include +#include +#include + +static void +_run_iteration (DBusConnection *conn) +{ + DBusPendingCall *echo_pending; + DBusPendingCall *dbus_pending; + DBusMessage *method; + DBusMessage *reply; + char *echo = "echo"; + + /* send the first message */ + method = dbus_message_new_method_call ("org.freedesktop.DBus.TestSuiteEchoService", + "/org/freedesktop/TestSuite", + "org.freedesktop.TestSuite", + "Echo"); + + dbus_message_append_args (method, DBUS_TYPE_STRING, &echo, NULL); + dbus_connection_send_with_reply (conn, method, &echo_pending, -1); + dbus_message_unref (method); + + /* send the second message */ + method = dbus_message_new_method_call (DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + "org.freedesktop.Introspectable", + "Introspect"); + + dbus_connection_send_with_reply (conn, method, &dbus_pending, -1); + dbus_message_unref (method); + + /* block on the second message (should return immediately) */ + dbus_pending_call_block (dbus_pending); + + /* block on the first message */ + /* if it does not return immediately chances + are we hit the block in poll bug */ + dbus_pending_call_block (echo_pending); + + /* check the reply only to make sure we + are not getting errors unrelated + to the block in poll bug */ + reply = dbus_pending_call_steal_reply (echo_pending); + + if (reply == NULL) + { + printf ("Failed: Reply is NULL ***\n"); + exit (1); + } + + if (dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_ERROR) + { + printf ("Failed: Reply is error: %s ***\n", dbus_message_get_error_name (reply)); + exit (1); + } + + dbus_message_unref (reply); + dbus_pending_call_unref (dbus_pending); + dbus_pending_call_unref (echo_pending); + +} + +int +main (int argc, char *argv[]) +{ + long start_tv_sec, start_tv_usec; + long end_tv_sec, end_tv_usec; + int i; + DBusMessage *method; + DBusConnection *conn; + DBusError error; + + /* Time each iteration and make sure it doesn't take more than 5 seconds + to complete. Outside influences may cause connections to take longer + but if it does and we are stuck in a poll call then we know the + stuck in poll bug has come back to haunt us */ + + printf ("*** Testing stuck in poll\n"); + + dbus_error_init (&error); + + conn = dbus_bus_get (DBUS_BUS_SESSION, &error); + + /* run 100 times to make sure */ + for (i = 0; i < 100; i++) + { + long delta; + + _dbus_get_current_time (&start_tv_sec, &start_tv_usec); + _run_iteration (conn); + _dbus_get_current_time (&end_tv_sec, &end_tv_usec); + + /* we just care about seconds */ + delta = end_tv_sec - start_tv_sec; + printf ("Iter %i: %is\n", i, delta); + if (delta >= 5) + { + printf ("Failed: looks like we might have been be stuck in poll ***\n"); + exit (1); + } + } + + method = dbus_message_new_method_call ("org.freedesktop.TestSuiteEchoService", + "/org/freedesktop/TestSuite", + "org.freedesktop.TestSuite", + "Exit"); + dbus_connection_send (conn, method, NULL); + dbus_message_unref (method); + + printf ("Success ***\n"); + exit (0); +} -- cgit