diff --git a/src/core/dbus.c b/src/core/dbus.c index 5e5c3ccf1b..83a4e8912e 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -812,6 +812,8 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { if (r < 0) log_warning_errno(r, "Failed to register MemoryAllocation1, ignoring: %m"); + m->api_bus_ready = true; + log_debug("Successfully connected to API bus."); return 0; @@ -1104,6 +1106,8 @@ static void destroy_bus(Manager *m, sd_bus **bus) { void bus_done_api(Manager *m) { destroy_bus(m, &m->api_bus); + + m->api_bus_ready = false; } void bus_done_system(Manager *m) { diff --git a/src/core/manager.c b/src/core/manager.c index ad525539ce..e82ba36fd5 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2892,6 +2892,14 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { assert(m); + /* If the API bus is connected but not fully set up yet (see bus_init_api()), postpone + * dispatching the queue, otherwise subscribers restored from a previous reexec would miss + * messages. The pending Varlink reload reply does not depend on the API bus, but it is held + * back too, so that the order between D-Bus messages and Varlink replies is preserved for + * clients that monitor both. */ + if (m->api_bus && !m->api_bus_ready) + return 0; + /* When we are reloading, let's not wait with generating signals, since we need to exit the manager as quickly * as we can. There's no point in throttling generation of signals in that case. */ if (MANAGER_IS_RELOADING(m) || m->send_reloading_done || m->pending_reload_message_dbus || m->pending_reload_message_vl) diff --git a/src/core/manager.h b/src/core/manager.h index 12d6cf4470..453ae74668 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -341,6 +341,11 @@ typedef struct Manager { sd_bus_track *subscribed; char **subscribed_as_strv; + /* Set once bus_setup_api() succeeded for the current API bus connection, i.e. after any + * subscriptions deserialized from a previous reload/reexec were coldplugged. Unset when the + * connection is torn down. */ + bool api_bus_ready; + /* The bus id of API bus acquired through org.freedesktop.DBus.GetId, which before deserializing * subscriptions we'd use to verify the bus is still the same instance as before. */ sd_id128_t bus_id, deserialized_bus_id; diff --git a/test/units/TEST-07-PID1.reloading-signal.sh b/test/units/TEST-07-PID1.reloading-signal.sh new file mode 100755 index 0000000000..e55ee174f2 --- /dev/null +++ b/test/units/TEST-07-PID1.reloading-signal.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eux +set -o pipefail + +# Verify that the manager broadcasts the Reloading signal on the API bus, +# both for daemon-reload and, as a one-shot Reloading(false), after a +# daemon-reexec. +# +# The reexec case is a regression test: API bus subscribers from the +# pre-reexec instance are coldplugged only once the asynchronous GetId +# reply is processed (see bus_init_api()). The D-Bus queue dispatch must +# not consume send_reloading_done before that happened, otherwise the +# Reloading(false) signal is lost and clients waiting for it to detect the +# end of the reexec time out. + +MONITOR_LOG=/run/TEST-07-PID1-reloading-signal.log +MONITOR_UNIT= + +at_exit() { + [[ -n "$MONITOR_UNIT" ]] && systemctl stop "$MONITOR_UNIT" 2>/dev/null || : + rm -f "$MONITOR_LOG" +} + +trap at_exit EXIT + +start_monitor() { + # Use a fresh unit name every time, a previous instance might not have + # been garbage-collected yet. busctl monitor sends READY=1 once the + # monitor is installed, so with Type=notify the monitor is guaranteed + # to be running when systemd-run returns. + MONITOR_UNIT="busctl-monitor-$RANDOM.service" + systemd-run --unit="$MONITOR_UNIT" --service-type=notify --quiet \ + --property=StandardOutput=file:"$MONITOR_LOG" \ + busctl monitor \ + --match="type=signal,path=/org/freedesktop/systemd1,interface=org.freedesktop.systemd1.Manager,member=Reloading" +} + +stop_monitor() { + systemctl stop "$MONITOR_UNIT" + MONITOR_UNIT= + rm -f "$MONITOR_LOG" +} + +wait_for_reloading() { + local value=${1:?} + + timeout 30 bash -ec "until grep -F 'BOOLEAN ${value};' $MONITOR_LOG >/dev/null; do sleep .5; done" +} + +# The manager sends the Reloading signal on the API bus only if it has at +# least one subscriber. logind subscribes, and its subscription is carried +# over a daemon-reexec via serialization, so make sure it is running. +systemctl start systemd-logind.service + +# logind's Subscribe call is asynchronous, so it may not have been processed +# by the manager yet when logind announces readiness. Keep reloading until +# the signal shows up in the monitor output. +start_monitor +timeout 30 bash -ec "until grep -F 'BOOLEAN true;' $MONITOR_LOG >/dev/null; do systemctl daemon-reload; sleep 1; done" + +# A daemon-reload sends Reloading(true) followed by Reloading(false). +wait_for_reloading true +wait_for_reloading false +stop_monitor + +# A daemon-reexec must send the one-shot Reloading(false) once the new +# manager instance is up, even though the subscribers of the old instance +# are re-added only after the asynchronous API bus setup finished. +start_monitor +systemctl daemon-reexec +wait_for_reloading false +stop_monitor