Commit f541722f authored by Simon McVittie's avatar Simon McVittie

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
parent dab9cbc5
......@@ -1619,7 +1619,12 @@ bus_pending_reply_send_no_reply (BusConnections *connections,
DBUS_ERROR_NO_REPLY))
goto out;
errmsg = "Message did not receive a reply (timeout by message bus)";
/* If you change these messages, adjust test/dbus-daemon.c to match */
if (pending->will_send_reply == NULL)
errmsg = "Message recipient disconnected from message bus without replying";
else
errmsg = "Message did not receive a reply (timeout by message bus)";
dbus_message_iter_init_append (message, &iter);
if (!dbus_message_iter_append_basic (&iter, DBUS_TYPE_STRING, &errmsg))
goto out;
......
......@@ -1779,6 +1779,7 @@ dbus-1.pc
dbus-1-uninstalled.pc
test/data/valid-config-files/debug-allow-all.conf
test/data/valid-config-files/debug-allow-all-sha1.conf
test/data/valid-config-files/finite-timeout.conf
test/data/valid-config-files/incoming-limit.conf
test/data/valid-config-files-system/debug-allow-all-pass.conf
test/data/valid-config-files-system/debug-allow-all-fail.conf
......
......@@ -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 \
......
<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
<busconfig>
<!-- Our well-known bus type, don't change this -->
<type>session</type>
<listen>@TEST_LISTEN@</listen>
<policy context="default">
<!-- Allow everything to be sent -->
<allow send_destination="*" eavesdrop="true"/>
<!-- Allow everything to be received -->
<allow eavesdrop="true"/>
<!-- Allow anyone to own anything -->
<allow own="*"/>
</policy>
<!-- Forcibly time out method calls after 100ms -->
<limit name="reply_timeout">100</limit>
</busconfig>
......@@ -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 ();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment