From 50592945355df9c24cbb1592edd65ec3ef9509cd Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 19 Sep 2025 12:54:09 +0100 Subject: [PATCH 1/4] gio/tests: Factor out connection_wait_for_bus() from gdbus-subscribe Signed-off-by: Simon McVittie --- gio/tests/gdbus-subscribe.c | 29 ----------------------------- gio/tests/gdbus-tests.c | 36 ++++++++++++++++++++++++++++++++++++ gio/tests/gdbus-tests.h | 2 ++ 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c index f04ff2063..ce72915ce 100644 --- a/gio/tests/gdbus-subscribe.c +++ b/gio/tests/gdbus-subscribe.c @@ -620,35 +620,6 @@ typedef struct guint finished_subscription; } Fixture; -/* Wait for asynchronous messages from @conn to have been processed - * by the message bus, as a sequence point so that we can make - * "happens before" and "happens after" assertions relative to this. - * The easiest way to achieve this is to call a message bus method that has - * no arguments and wait for it to return: because the message bus processes - * messages in-order, anything we sent before this must have been processed - * by the time this call arrives. */ -static void -connection_wait_for_bus (GDBusConnection *conn) -{ - GError *error = NULL; - GVariant *call_result; - - call_result = g_dbus_connection_call_sync (conn, - DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "GetId", - NULL, /* arguments */ - NULL, /* result type */ - G_DBUS_CALL_FLAGS_NONE, - -1, - NULL, - &error); - g_assert_no_error (error); - g_assert_nonnull (call_result); - g_variant_unref (call_result); -} - /* * Called when the subscriber receives a message from any connection * announcing that it has emitted all the signals that it plans to emit. diff --git a/gio/tests/gdbus-tests.c b/gio/tests/gdbus-tests.c index 857de0132..7b329fd39 100644 --- a/gio/tests/gdbus-tests.c +++ b/gio/tests/gdbus-tests.c @@ -27,6 +27,8 @@ #include "gdbus-tests.h" +#include "gdbusprivate.h" + /* ---------------------------------------------------------------------------------------------------- */ typedef struct @@ -155,6 +157,40 @@ ensure_gdbus_testserver_up (GDBusConnection *connection, g_main_context_pop_thread_default (context); } +/* + * connection_wait_for_bus: + * @conn: A connection + * + * Wait for asynchronous messages from @conn to have been processed + * by the message bus, as a sequence point so that we can make + * "happens before" and "happens after" assertions relative to this. + * The easiest way to achieve this is to call a message bus method that has + * no arguments and wait for it to return: because the message bus processes + * messages in-order, anything we sent before this must have been processed + * by the time this call arrives. + */ +void +connection_wait_for_bus (GDBusConnection *conn) +{ + GError *error = NULL; + GVariant *call_result; + + call_result = g_dbus_connection_call_sync (conn, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetId", + NULL, /* arguments */ + NULL, /* result type */ + G_DBUS_CALL_FLAGS_NONE, + -1, + NULL, + &error); + g_assert_no_error (error); + g_assert_nonnull (call_result); + g_variant_unref (call_result); +} + /* ---------------------------------------------------------------------------------------------------- */ typedef struct diff --git a/gio/tests/gdbus-tests.h b/gio/tests/gdbus-tests.h index 9cca55656..9cf188f23 100644 --- a/gio/tests/gdbus-tests.h +++ b/gio/tests/gdbus-tests.h @@ -119,6 +119,8 @@ GDBusConnection *_g_bus_get_priv (GBusType bus_type, void ensure_gdbus_testserver_up (GDBusConnection *connection, GMainContext *context); +void connection_wait_for_bus (GDBusConnection *conn); + G_END_DECLS #endif /* __TESTS_H__ */ From f866de57bf1e4e7cf3358b0b34960b383d4ddb90 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 19 Sep 2025 13:07:59 +0100 Subject: [PATCH 2/4] gio/tests: Avoid a race condition We have two things happening in parallel: 1. The GDBus worker thread writes out an AddMatch call to the socket, the message bus reads that method call from the other end of the socket, and the message bus responds by adding the match rule for the signal subscription 2. The main thread forks, and the child execs gdbus-connection-flush-helper, which sends the signal that we are expecting; the message bus receives that signal, and broadcasts it to subscribers, if any Normally (1.) wins the race because exec'ing a child process is more time-consuming than IPC, and the test passes. However, it is possible for (2.) to win the race. If so, we will never receive the expected signal (because it was received by the message bus before the AddMatch() method call, so at the time it was received, the test was not yet a subscriber); the test waits until the timeout and then gives up, and the test fails. For whatever reason, Debian's s390x buildd seems to be reliably failing this test since this week, having previously passed. I don't know what changed. I can (very rarely) reproduce the race condition described above on a developer-accessible s390x machine by repeatedly running the /gdbus/connection/flush test in a loop. Bug-Debian: https://bugs.debian.org/1115617 Signed-off-by: Simon McVittie --- gio/tests/gdbus-connection-slow.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gio/tests/gdbus-connection-slow.c b/gio/tests/gdbus-connection-slow.c index 0f445d43d..55e8b35cf 100644 --- a/gio/tests/gdbus-connection-slow.c +++ b/gio/tests/gdbus-connection-slow.c @@ -82,6 +82,16 @@ test_connection_flush (void) NULL); g_assert_cmpint (signal_handler_id, !=, 0); + /* We need to wait for the subscription to have actually taken effect + * before forking the subprocess, otherwise there is a race condition + * between the message bus adding the match rule and the subprocess + * sending the signal. If the message bus wins the race, then the test + * passes, but if the subprocess wins the race, it will be too late + * for this process to receive the signal because it already happened. + * The easiest way to avoid this race is to do a round-trip to the + * message bus and back. */ + connection_wait_for_bus (connection); + flush_helper = g_test_get_filename (G_TEST_BUILT, "gdbus-connection-flush-helper", NULL); for (n = 0; n < 50; n++) { From b8f88557ab86625368b28de09c5a1ae944be7177 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 19 Sep 2025 13:09:22 +0100 Subject: [PATCH 3/4] gio/tests: Convert the time to wait for expected signal into a constant No functional change. Signed-off-by: Simon McVittie --- gio/tests/gdbus-connection-slow.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gio/tests/gdbus-connection-slow.c b/gio/tests/gdbus-connection-slow.c index 55e8b35cf..ec71f9079 100644 --- a/gio/tests/gdbus-connection-slow.c +++ b/gio/tests/gdbus-connection-slow.c @@ -28,6 +28,8 @@ #include "gdbus-tests.h" +#define WAIT_FOR_FLUSH_MSEC 1000 + /* all tests rely on a shared mainloop */ static GMainLoop *loop = NULL; @@ -49,7 +51,7 @@ static gboolean test_connection_flush_on_timeout (gpointer user_data) { guint iteration = GPOINTER_TO_UINT (user_data); - g_printerr ("Timeout waiting 1000 msec on iteration %d\n", iteration); + g_printerr ("Timeout waiting %d msec on iteration %d\n", WAIT_FOR_FLUSH_MSEC, iteration); g_assert_not_reached (); return G_SOURCE_REMOVE; } @@ -118,7 +120,7 @@ test_connection_flush (void) g_assert_no_error (error); g_assert_true (ret); - timeout_mainloop_id = g_timeout_add (1000, test_connection_flush_on_timeout, GUINT_TO_POINTER (n)); + timeout_mainloop_id = g_timeout_add (WAIT_FOR_FLUSH_MSEC, test_connection_flush_on_timeout, GUINT_TO_POINTER (n)); g_main_loop_run (loop); g_source_remove (timeout_mainloop_id); } From 209dff23f118407c52a7446af22411c8daf9f914 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 19 Sep 2025 13:12:29 +0100 Subject: [PATCH 4/4] gio/tests: Wait up to 10 seconds for a signal to be received If the build/test machine is slow, heavily-loaded or otherwise inconvenienced, it might take a few seconds for the signal to be sent by the subprocess, received by the message bus, re-broadcasted by the message bus and received by the test code. Wait a few more seconds before giving up. If this test is successful, increasing this timeout will not slow it down: we stop waiting for the signal as soon as we receive it. This will only make any difference if the test would have failed. Signed-off-by: Simon McVittie --- gio/tests/gdbus-connection-slow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/tests/gdbus-connection-slow.c b/gio/tests/gdbus-connection-slow.c index ec71f9079..061463f7f 100644 --- a/gio/tests/gdbus-connection-slow.c +++ b/gio/tests/gdbus-connection-slow.c @@ -28,7 +28,7 @@ #include "gdbus-tests.h" -#define WAIT_FOR_FLUSH_MSEC 1000 +#define WAIT_FOR_FLUSH_MSEC 10000 /* all tests rely on a shared mainloop */ static GMainLoop *loop = NULL;