From f541722f5ac0b2ee2d9eab6b9a77aa1f1c174817 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 29 Oct 2014 14:10:48 +0000 Subject: Use a better NoReply message for disconnection with reply pending As an implementation detail, dbus-daemon handles this situation by artificially triggering a timeout (even if its configured timeout for method calls is in fact infinite). However, using the same debug message for both is misleading, and can lead people who are debugging a service crash to blame dbus-daemon instead, wasting their time. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=76112 --- test/Makefile.am | 1 + .../data/valid-config-files/finite-timeout.conf.in | 19 +++++ test/dbus-daemon.c | 95 +++++++++++++++++++++- 3 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 test/data/valid-config-files/finite-timeout.conf.in (limited to 'test') diff --git a/test/Makefile.am b/test/Makefile.am index 1ceb5b68..173df74b 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -254,6 +254,7 @@ in_data = \ data/valid-config-files-system/debug-allow-all-pass.conf.in \ data/valid-config-files/debug-allow-all-sha1.conf.in \ data/valid-config-files/debug-allow-all.conf.in \ + data/valid-config-files/finite-timeout.conf.in \ data/valid-config-files/incoming-limit.conf.in \ data/invalid-service-files-system/org.freedesktop.DBus.TestSuiteNoExec.service.in \ data/invalid-service-files-system/org.freedesktop.DBus.TestSuiteNoService.service.in \ diff --git a/test/data/valid-config-files/finite-timeout.conf.in b/test/data/valid-config-files/finite-timeout.conf.in new file mode 100644 index 00000000..7d26d715 --- /dev/null +++ b/test/data/valid-config-files/finite-timeout.conf.in @@ -0,0 +1,19 @@ + + + + session + @TEST_LISTEN@ + + + + + + + + + + + + 100 + diff --git a/test/dbus-daemon.c b/test/dbus-daemon.c index 4b3b61e5..1db4e44b 100644 --- a/test/dbus-daemon.c +++ b/test/dbus-daemon.c @@ -57,6 +57,7 @@ typedef struct { DBusConnection *right_conn; gboolean right_conn_echo; + gboolean wait_forever_called; } Fixture; #define assert_no_error(e) _assert_no_error (e, __FILE__, __LINE__) @@ -157,11 +158,19 @@ echo_filter (DBusConnection *connection, DBusMessage *message, void *user_data) { + Fixture *f = user_data; DBusMessage *reply; if (dbus_message_get_type (message) != DBUS_MESSAGE_TYPE_METHOD_CALL) return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; + /* WaitForever() never replies, emulating a service that has got stuck */ + if (dbus_message_is_method_call (message, "com.example", "WaitForever")) + { + f->wait_forever_called = TRUE; + return DBUS_HANDLER_RESULT_HANDLED; + } + reply = dbus_message_new_method_return (message); if (reply == NULL) @@ -257,7 +266,7 @@ setup (Fixture *f, static void add_echo_filter (Fixture *f) { - if (!dbus_connection_add_filter (f->right_conn, echo_filter, NULL, NULL)) + if (!dbus_connection_add_filter (f->right_conn, echo_filter, f, NULL)) g_error ("OOM"); f->right_conn_echo = TRUE; @@ -342,6 +351,80 @@ pending_call_store_reply (DBusPendingCall *pc, g_assert (*message_p != NULL); } +static void +test_no_reply (Fixture *f, + gconstpointer context) +{ + const Config *config = context; + DBusMessage *m; + DBusPendingCall *pc; + DBusMessage *reply = NULL; + enum { TIMEOUT, DISCONNECT } mode; + gboolean ok; + + if (f->skip) + return; + + g_test_bug ("76112"); + + if (config != NULL && config->config_file != NULL) + mode = TIMEOUT; + else + mode = DISCONNECT; + + m = dbus_message_new_method_call ( + dbus_bus_get_unique_name (f->right_conn), "/", + "com.example", "WaitForever"); + + add_echo_filter (f); + + if (m == NULL) + g_error ("OOM"); + + if (!dbus_connection_send_with_reply (f->left_conn, m, &pc, + DBUS_TIMEOUT_INFINITE) || + pc == NULL) + g_error ("OOM"); + + if (dbus_pending_call_get_completed (pc)) + pending_call_store_reply (pc, &reply); + else if (!dbus_pending_call_set_notify (pc, pending_call_store_reply, &reply, + NULL)) + g_error ("OOM"); + + dbus_pending_call_unref (pc); + dbus_message_unref (m); + + if (mode == DISCONNECT) + { + while (!f->wait_forever_called) + test_main_context_iterate (f->ctx, TRUE); + + dbus_connection_remove_filter (f->right_conn, echo_filter, f); + dbus_connection_close (f->right_conn); + dbus_connection_unref (f->right_conn); + f->right_conn = NULL; + } + + while (reply == NULL) + test_main_context_iterate (f->ctx, TRUE); + + /* using inefficient string comparison for better assertion message */ + g_assert_cmpstr ( + dbus_message_type_to_string (dbus_message_get_type (reply)), ==, + dbus_message_type_to_string (DBUS_MESSAGE_TYPE_ERROR)); + ok = dbus_set_error_from_message (&f->e, reply); + g_assert (ok); + g_assert_cmpstr (f->e.name, ==, DBUS_ERROR_NO_REPLY); + + if (mode == DISCONNECT) + g_assert_cmpstr (f->e.message, ==, + "Message recipient disconnected from message bus without replying"); + else + g_assert_cmpstr (f->e.message, ==, + "Message did not receive a reply (timeout by message bus)"); +} + static void test_creds (Fixture *f, gconstpointer context) @@ -475,7 +558,7 @@ teardown (Fixture *f, { if (f->right_conn_echo) { - dbus_connection_remove_filter (f->right_conn, echo_filter, NULL); + dbus_connection_remove_filter (f->right_conn, echo_filter, f); f->right_conn_echo = FALSE; } @@ -503,6 +586,10 @@ static Config limited_config = { "34393", 10000, "valid-config-files/incoming-limit.conf" }; +static Config finite_timeout_config = { + NULL, 1, "valid-config-files/finite-timeout.conf" +}; + int main (int argc, char **argv) @@ -513,6 +600,10 @@ main (int argc, g_test_add ("/echo/session", Fixture, NULL, setup, test_echo, teardown); g_test_add ("/echo/limited", Fixture, &limited_config, setup, test_echo, teardown); + g_test_add ("/no-reply/disconnect", Fixture, NULL, + setup, test_no_reply, teardown); + g_test_add ("/no-reply/timeout", Fixture, &finite_timeout_config, + setup, test_no_reply, teardown); g_test_add ("/creds", Fixture, NULL, setup, test_creds, teardown); return g_test_run (); -- cgit v1.2.1