test/bus: Break up dispatch test into three separate tests

This is really three separate test-cases: one for traditional
activation as a direct child process of the dbus-daemon, and two for
traditional activation (successful and failing) via the setuid
dbus-daemon-launch-helper on Unix.

The ones where activation succeeds are extremely slow, as a result of the
instrumentation for simulating malloc() failures combined with a large
number of memory operations, particularly when using AddressSanitizer.

Splitting up "OOM" tests like these has a disproportionately good impact
on the time they take, because the simulated malloc() failure
instrumentation repeats the entire test making the first malloc() fail,
then making the second malloc() fail, and so on. For allocation failures
in the second half of the test, this means we repeat the first half of
the test with no malloc() failures a very large number of times, which
is not a good use of time, because we already tested it successfully.

Even when not using the "OOM" instrumentation, splitting up these tests
lets them run in parallel, which is also a major time saving.

Needless to say, this speeds up testing considerably. On my modern
but unexceptional x86 laptop, in a typical debug build with Meson on
the 1.15.x branch, the old dispatch test took just over 21 minutes,
which drops to about 40 seconds each for the new normal-activation and
helper-activation tests (and for most of that time, they're running
in parallel, so the wall-clock time taken for the whole test suite is
somewhere around a minute).

In a debug build with Meson, gcc and AddressSanitizer on the 1.15.x
branch, the old dispatch test takes longer than my patience will allow,
and the new separate tests take about 5-6 minutes each. Reduce their
timeout accordingly, but not as far as the default for slow tests (5
minutes) to allow some headroom for AddressSanitizer or slower systems.

A side benefit of this is that it makes our testing more realistic.  Each
call to bus_dispatch_test_conf() creates a new BusContext, which has the
side-effect of saving the initial file descriptor rlimit and expanding the
soft limit to match the hard limit. When we call bus_dispatch_test_conf()
more than once per test process, the second and subsequent calls will save
the increased rlimit as though it had been the original, and restore it
before exec'ing subprocesses, resulting in a potentially extremely high
soft limit which means our debug instrumentation takes a very long time
to iterate through all possible fds (see dbus#527).

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit bef88fd5627f2d990915c9009982107a8e329ef5)
Mitigates: https://gitlab.freedesktop.org/dbus/dbus/-/issues/527

diff --git a/bus/dispatch.c b/bus/dispatch.c
index c3019b1e86316734a5caac9cf7ad360b4f593bde..577bb1e08da811b44f659c4323770ed6af67c80e 100644 (file)
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -5036,36 +5036,52 @@ bus_dispatch_test_conf_fail (const DBusString *test_data_dir,
 }
 #endif
 
+#ifdef ENABLE_TRADITIONAL_ACTIVATION
 dbus_bool_t
-bus_dispatch_test (const char *test_data_dir_cstr)
+bus_test_normal_activation (const char *test_data_dir_cstr)
 {
   DBusString test_data_dir;
 
   _dbus_string_init_const (&test_data_dir, test_data_dir_cstr);
 
-#ifdef ENABLE_TRADITIONAL_ACTIVATION
-  /* run normal activation tests */
-  _dbus_verbose ("Normal activation tests\n");
   if (!bus_dispatch_test_conf (&test_data_dir,
-                              "valid-config-files/debug-allow-all.conf", FALSE))
+                               "valid-config-files/debug-allow-all.conf", FALSE))
     return FALSE;
 
+  return TRUE;
+}
+
 #ifndef DBUS_WIN
-  /* run launch-helper activation tests */
-  _dbus_verbose ("Launch helper activation tests\n");
+dbus_bool_t
+bus_test_helper_activation (const char *test_data_dir_cstr)
+{
+  DBusString test_data_dir;
+
+  _dbus_string_init_const (&test_data_dir, test_data_dir_cstr);
+
   if (!bus_dispatch_test_conf (&test_data_dir,
-                              "valid-config-files-system/debug-allow-all-pass.conf", TRUE))
+                               "valid-config-files-system/debug-allow-all-pass.conf", TRUE))
     return FALSE;
 
+  return TRUE;
+}
+
+dbus_bool_t
+bus_test_failed_helper_activation (const char *test_data_dir_cstr)
+{
+  DBusString test_data_dir;
+
+  _dbus_string_init_const (&test_data_dir, test_data_dir_cstr);
+
   /* run select launch-helper activation tests on broken service files */
   if (!bus_dispatch_test_conf_fail (&test_data_dir,
-                                   "valid-config-files-system/debug-allow-all-fail.conf"))
+                                    "valid-config-files-system/debug-allow-all-fail.conf"))
     return FALSE;
-#endif
-#endif
 
   return TRUE;
 }
+#endif  /* !DBUS_WIN */
+#endif  /* ENABLE_TRADITIONAL_ACTIVATION */
 
 dbus_bool_t
 bus_dispatch_sha1_test (const char *test_data_dir_cstr)

diff --git a/bus/test.h b/bus/test.h
index 6f1f6b0d7c8e18135af3974208f7d1a6beb6b422..a6658cab0243d1236d9dc927297fd291cc9c486a 100644 (file)
--- a/bus/test.h
+++ b/bus/test.h
@@ -36,7 +36,9 @@
 typedef dbus_bool_t (* BusConnectionForeachFunction) (DBusConnection *connection,
                                                       void           *data);
 
-dbus_bool_t bus_dispatch_test         (const char                   *test_data_dir_cstr);
+dbus_bool_t bus_test_normal_activation (const char                  *test_data_dir_cstr);
+dbus_bool_t bus_test_helper_activation (const char                  *test_data_dir_cstr);
+dbus_bool_t bus_test_failed_helper_activation (const char           *test_data_dir_cstr);
 dbus_bool_t bus_dispatch_sha1_test    (const char                   *test_data_dir_cstr);
 dbus_bool_t bus_config_parser_test    (const char                   *test_data_dir_cstr);
 dbus_bool_t bus_config_parser_trivial_test (const char              *test_data_dir_cstr);

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index abf31b55ba1b839daa78e33f6d814d07f1a29920..9308f433f4e83d44a541e5731b2d59b5b1c9a471 100644 (file)
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -149,9 +149,12 @@ if(DBUS_ENABLE_EMBEDDED_TESTS)
     add_test_executable(test-bus "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
     set_target_properties(test-bus PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS})
 
-    set(SOURCES bus/dispatch.c bus/common.c bus/common.h)
-    add_test_executable(test-bus-dispatch "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
-    set_target_properties(test-bus-dispatch PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS})
+    if(ENABLE_TRADITIONAL_ACTIVATION)
+        set(SOURCES bus/normal-activation.c bus/common.c bus/common.h)
+        add_test_executable(test-bus-normal-activation "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
+        set_target_properties(test-bus-normal-activation PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS})
+    endif()
+
 
     set(SOURCES bus/dispatch-sha1.c bus/common.c bus/common.h)
     add_test_executable(test-bus-dispatch-sha1 "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
@@ -163,6 +166,14 @@ if(DBUS_ENABLE_EMBEDDED_TESTS)
         if(ENABLE_TRADITIONAL_ACTIVATION)
             add_test_executable(test-bus-launch-helper-oom bus/launch-helper-oom.c launch-helper-internal dbus-testutils)
             add_helper_executable(dbus-daemon-launch-helper-for-tests bus/launch-helper-for-tests.c launch-helper-internal)
+
+            set(SOURCES bus/failed-helper-activation.c bus/common.c bus/common.h)
+            add_test_executable(test-bus-failed-helper-activation "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
+            set_target_properties(test-bus-failed-helper-activation PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS})
+
+            set(SOURCES bus/helper-activation.c bus/common.c bus/common.h)
+            add_test_executable(test-bus-helper-activation "${SOURCES}" dbus-daemon-internal dbus-testutils ${EXPAT_LIBRARIES})
+            set_target_properties(test-bus-helper-activation PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS})
         endif()
     endif()
 endif()

diff --git a/test/Makefile.am b/test/Makefile.am
index 23a6ca13ad3fab75aef6747411e2db3aecebc0d2..6783985c5a9cdfc267d807d2585b49776aa2c9bf 100644 (file)
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -86,13 +86,16 @@ endif
 
 uninstallable_test_programs += \
        test-bus \
-       test-bus-dispatch \
        test-bus-dispatch-sha1 \
        test-marshal-recursive \
        test-message-internals \
        test-strings \
        $(NULL)
 
+if ENABLE_TRADITIONAL_ACTIVATION
+uninstallable_test_programs += test-normal-activation
+endif
+
 if DBUS_UNIX
 test_counter_SOURCES = internals/counter.c
 test_counter_LDADD = libdbus-testutils.la
@@ -100,6 +103,8 @@ test_counter_LDADD = libdbus-testutils.la
 if ENABLE_TRADITIONAL_ACTIVATION
 uninstallable_test_programs += test-bus-launch-helper-oom
 uninstallable_test_programs += test-bus-system
+uninstallable_test_programs += test-failed-helper-activation
+uninstallable_test_programs += test-helper-activation
 # this is used by the tests but is not, itself, a test
 TEST_BINARIES += dbus-daemon-launch-helper-for-tests
 endif ENABLE_TRADITIONAL_ACTIVATION
@@ -213,12 +218,6 @@ test_bus_LDADD = \
        libdbus-testutils.la \
        $(NULL)
 
-test_bus_dispatch_SOURCES = bus/dispatch.c bus/common.c bus/common.h
-test_bus_dispatch_LDADD = \
-       $(top_builddir)/bus/libdbus-daemon-internal.la \
-       libdbus-testutils.la \
-       $(NULL)
-
 test_bus_dispatch_sha1_SOURCES = bus/dispatch-sha1.c bus/common.c bus/common.h
 test_bus_dispatch_sha1_LDADD = \
        $(top_builddir)/bus/libdbus-daemon-internal.la \
@@ -228,6 +227,26 @@ test_bus_dispatch_sha1_LDADD = \
 test_hash_SOURCES = internals/hash.c
 test_hash_LDADD = libdbus-testutils.la
 
+test_failed_helper_activation_SOURCES = \
+       bus/failed-helper-activation.c \
+       bus/common.c \
+       bus/common.h \
+       $(NULL)
+test_failed_helper_activation_LDADD = \
+       $(top_builddir)/bus/libdbus-daemon-internal.la \
+       libdbus-testutils.la \
+       $(NULL)
+
+test_helper_activation_SOURCES = \
+       bus/helper-activation.c \
+       bus/common.c \
+       bus/common.h \
+       $(NULL)
+test_helper_activation_LDADD = \
+       $(top_builddir)/bus/libdbus-daemon-internal.la \
+       libdbus-testutils.la \
+       $(NULL)
+
 test_marshal_recursive_SOURCES = \
        internals/dbus-marshal-recursive-util.c \
        internals/dbus-marshal-recursive-util.h \
@@ -265,6 +284,16 @@ test_misc_internals_SOURCES = \
        $(NULL)
 test_misc_internals_LDADD = libdbus-testutils.la
 
+test_normal_activation_SOURCES = \
+       bus/normal-activation.c \
+       bus/common.c \
+       bus/common.h \
+       $(NULL)
+test_normal_activation_LDADD = \
+       $(top_builddir)/bus/libdbus-daemon-internal.la \
+       libdbus-testutils.la \
+       $(NULL)
+
 EXTRA_DIST += dbus-test-runner
 
 testexecdir = $(libexecdir)/installed-tests/dbus

diff --git a/test/bus/failed-helper-activation.c b/test/bus/failed-helper-activation.c
new file mode 100644 (file)
index 0000000..044ab42
--- /dev/null
+++ b/test/bus/failed-helper-activation.c
@@ -0,0 +1,37 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
+/*
+ * Copyright 2003-2009 Red Hat, Inc.
+ * Copyright 2011-2018 Collabora Ltd.
+ *
+ * Licensed under the Academic Free License version 2.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <config.h>
+#include "test/bus/common.h"
+
+static DBusTestCase test =
+{
+  "failed-helper-activation",
+  bus_test_failed_helper_activation
+};
+
+int
+main (int argc, char **argv)
+{
+  return bus_test_main (argc, argv, 1, &test);
+}

diff --git a/test/bus/dispatch.c b/test/bus/helper-activation.c
similarity index 93%
rename from test/bus/dispatch.c
rename to test/bus/helper-activation.c
index bd86419fa0eb2cc55949723811868da1d79a37c7..f4c9a37d28e01b1bf5b671d07851feaade6c1dd1 100644 (file)
--- a/test/bus/dispatch.c
+++ b/test/bus/helper-activation.c
@@ -24,7 +24,7 @@
 #include <config.h>
 #include "test/bus/common.h"
 
-static DBusTestCase test = { "dispatch", bus_dispatch_test };
+static DBusTestCase test = { "helper-activation", bus_test_helper_activation };
 
 int
 main (int argc, char **argv)

diff --git a/test/bus/normal-activation.c b/test/bus/normal-activation.c
new file mode 100644 (file)
index 0000000..ce935a5
--- /dev/null
+++ b/test/bus/normal-activation.c
@@ -0,0 +1,33 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
+/*
+ * Copyright 2003-2009 Red Hat, Inc.
+ * Copyright 2011-2018 Collabora Ltd.
+ *
+ * Licensed under the Academic Free License version 2.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <config.h>
+#include "test/bus/common.h"
+
+static DBusTestCase test = { "normal-activation", bus_test_normal_activation };
+
+int
+main (int argc, char **argv)
+{
+  return bus_test_main (argc, argv, 1, &test);
+}