From 00cf56abbd520183e3293a1fcb50dfd59053e53a Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 5 Jun 2026 12:39:07 +0800 Subject: [PATCH 1/3] core: Add SERVICE_RUNNING_REVALIDATING state For a Type=dbus service we sometimes want to keep treating it as running while its bus name has briefly gone away (for example, due to a broker restart). SERVICE_RUNNING_REVALIDATING is active and keeps its main process like SERVICE_RUNNING, but it's deliberately not part of the runtime timer set, so RuntimeMaxSec= is paused while we sit in it. --- src/basic/unit-def.c | 1 + src/basic/unit-def.h | 1 + src/core/manager.c | 1 + src/core/service.c | 13 ++++++++++--- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index 57a67af163..c9d6a5a6f0 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -233,6 +233,7 @@ static const char* const service_state_table[_SERVICE_STATE_MAX] = { [SERVICE_START] = "start", [SERVICE_START_POST] = "start-post", [SERVICE_RUNNING] = "running", + [SERVICE_RUNNING_REVALIDATING] = "running-revalidating", [SERVICE_EXITED] = "exited", [SERVICE_REFRESH_EXTENSIONS] = "refresh-extensions", [SERVICE_REFRESH_CREDENTIALS] = "refresh-credentials", diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h index 9192d8c4a5..babc31444f 100644 --- a/src/basic/unit-def.h +++ b/src/basic/unit-def.h @@ -132,6 +132,7 @@ typedef enum ServiceState { SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + SERVICE_RUNNING_REVALIDATING, /* Temporarily waiting for the D-Bus name to reappear */ SERVICE_EXITED, /* Nothing is running anymore, but RemainAfterExit is true hence this is OK */ SERVICE_REFRESH_EXTENSIONS, /* Refreshing extensions for a reload request */ SERVICE_REFRESH_CREDENTIALS, /* ditto, but for credentials */ diff --git a/src/core/manager.c b/src/core/manager.c index 0ccbaa61c5..b26365cd49 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2048,6 +2048,7 @@ static bool manager_dbus_is_running(Manager *m, bool deserialized) { return false; if (!IN_SET(deserialized ? SERVICE(u)->deserialized_state : SERVICE(u)->state, SERVICE_RUNNING, + SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, diff --git a/src/core/service.c b/src/core/service.c index 535c833e25..bf6e91016c 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -77,6 +77,7 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_START] = UNIT_ACTIVATING, [SERVICE_START_POST] = UNIT_ACTIVATING, [SERVICE_RUNNING] = UNIT_ACTIVE, + [SERVICE_RUNNING_REVALIDATING] = UNIT_ACTIVE, [SERVICE_EXITED] = UNIT_ACTIVE, [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, [SERVICE_REFRESH_CREDENTIALS] = UNIT_REFRESHING, @@ -111,6 +112,7 @@ static const UnitActiveState state_translation_table_idle[_SERVICE_STATE_MAX] = [SERVICE_START] = UNIT_ACTIVE, [SERVICE_START_POST] = UNIT_ACTIVE, [SERVICE_RUNNING] = UNIT_ACTIVE, + [SERVICE_RUNNING_REVALIDATING] = UNIT_ACTIVE, [SERVICE_EXITED] = UNIT_ACTIVE, [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, [SERVICE_REFRESH_CREDENTIALS] = UNIT_REFRESHING, @@ -153,7 +155,7 @@ static void service_set_state(Service *s, ServiceState state); static bool SERVICE_STATE_WITH_MAIN_PROCESS(ServiceState state) { return IN_SET(state, SERVICE_START, SERVICE_START_POST, - SERVICE_RUNNING, + SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_MOUNTING, @@ -175,7 +177,7 @@ static bool SERVICE_STATE_WITH_CONTROL_PROCESS(ServiceState state) { static bool SERVICE_STATE_WITH_WATCHDOG(ServiceState state) { return IN_SET(state, SERVICE_START_POST, - SERVICE_RUNNING, + SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_MOUNTING); @@ -3677,6 +3679,7 @@ static int service_stop(Unit *u) { return 0; case SERVICE_RUNNING: + case SERVICE_RUNNING_REVALIDATING: case SERVICE_EXITED: service_enter_stop(s, SERVICE_SUCCESS); return 1; @@ -3695,7 +3698,7 @@ static int service_stop(Unit *u) { static int service_reload(Unit *u) { Service *s = ASSERT_PTR(SERVICE(u)); - assert(IN_SET(s->state, SERVICE_RUNNING, SERVICE_EXITED)); + assert(IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_EXITED)); s->reload_result = SERVICE_SUCCESS; s->refreshed_mask = 0; @@ -4593,6 +4596,7 @@ static void service_notify_cgroup_empty_event(Unit *u) { break; case SERVICE_RUNNING: + case SERVICE_RUNNING_REVALIDATING: /* service_enter_running() will figure out what to do */ service_enter_running(s, SERVICE_SUCCESS); break; @@ -4654,6 +4658,7 @@ static void service_notify_cgroup_oom_event(Unit *u, bool managed_oom) { case SERVICE_EXITED: case SERVICE_RUNNING: + case SERVICE_RUNNING_REVALIDATING: if (s->oom_policy == OOM_STOP) service_enter_stop(s, SERVICE_FAILURE_OOM_KILL); else if (s->oom_policy == OOM_KILL) @@ -4818,6 +4823,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { _fallthrough_; case SERVICE_RUNNING: + case SERVICE_RUNNING_REVALIDATING: service_enter_running(s, f); break; @@ -6009,6 +6015,7 @@ static bool service_needs_console(Unit *u) { SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, From fdf0876bc7f547c1c1e571a42f010bac4231b3f8 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Thu, 4 Jun 2026 12:08:57 +0800 Subject: [PATCH 2/3] core: Don't racily stop Type=dbus services when the bus broker restarts If dbus dies, it is then immediately reactivated by dbus.socket as soon as a client reconnects. In that case pid1's own connection to the bus drops and is reestablished by manager_recheck_dbus() (which comes from the unit_notify() when dbus.services goes back to being running). As part of bringing the bus back up, bus_setup_api() issues GetNameOwner() for every name we watch on behalf of a BusName= unit. For a Type=dbus service this is a problem. Consider this scenario: 1. spearmint.service is Type=dbus, owns com.example.Spearmint, and is running. 2. The broker crashes and is re-activated through dbus.socket. 3. spearmint's client connection dropped too, so it is now reconnecting to the new broker to grab its name again. 4. pid1 racily reconnects first given it is triggered the moment dbus.service reaches SERVICE_RUNNING, whereas spearmint is an ordinary client that must first notice its own disconnection and thus when we do GetNameOwner() com.example.Spearmint is reported as having no owner. 5. service_bus_name_owner_change() sets bus_name_good = false, and since service_good() returns false for a Type=dbus service without its name, service_enter_running() stops the (perfectly healthy, other than dbus kicking the bucket temporarily) service. In other words, a broker restart can tear down running Type=dbus services that did nothing wrong, really. So, to fix this, we can use the fact that a NameOwnerChanged signal is definitive. That is, the name's owner changed while we were connected, so a service that crashes or drops its name can still be stopped immediately. By contrast a GetNameOwner() reply is less authoritative. There is, importantly, no change to how losing the name on a live connection works, we only give grace on reconnect. --- src/core/service.c | 122 ++++++++++++++++++++++++++++++++++++++------- src/core/service.h | 2 + src/core/unit.c | 9 ++-- src/core/unit.h | 2 +- 4 files changed, 113 insertions(+), 22 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index bf6e91016c..5c097c139d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -70,6 +70,8 @@ #define SERVICE_FD_STORE_POPULATED(s) (!!(s)->fd_store) +#define SERVICE_BUS_NAME_GRACE_USEC (2 * USEC_PER_SEC) + static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_DEAD] = UNIT_INACTIVE, [SERVICE_CONDITION] = UNIT_ACTIVATING, @@ -142,6 +144,7 @@ static int service_dispatch_inotify_io(sd_event_source *source, int fd, uint32_t static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void *userdata); static int service_dispatch_exec_io(sd_event_source *source, int fd, uint32_t events, void *userdata); +static int service_dispatch_bus_name_grace(sd_event_source *source, usec_t usec, void *userdata); static void service_enter_signal(Service *s, ServiceState state, ServiceResult f); @@ -215,6 +218,7 @@ static void service_init(Unit *u) { s->oom_policy = _OOM_POLICY_INVALID; s->reload_begin_usec = USEC_INFINITY; + s->revalidate_runtime_begin_usec = USEC_INFINITY; s->reload_signal = SIGHUP; s->fd_store_preserve_mode = EXEC_PRESERVE_RESTART; @@ -620,6 +624,7 @@ static void service_done(Unit *u) { service_stop_watchdog(s); s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); + s->bus_name_grace_event_source = sd_event_source_disable_unref(s->bus_name_grace_event_source); s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source); s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot); @@ -1670,6 +1675,9 @@ static void service_set_state(Service *s, ServiceState state) { SERVICE_CLEANING)) s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); + if (state != SERVICE_RUNNING_REVALIDATING) + s->bus_name_grace_event_source = sd_event_source_disable_unref(s->bus_name_grace_event_source); + if (!SERVICE_STATE_WITH_MAIN_PROCESS(state)) { service_unwatch_main_pid(s); s->main_command = NULL; @@ -1783,6 +1791,14 @@ static int service_coldplug(Unit *u) { if (r < 0) return r; + if (s->deserialized_state == SERVICE_RUNNING_REVALIDATING) { + r = unit_arm_timer(UNIT(s), &s->bus_name_grace_event_source, /* relative= */ true, + SERVICE_BUS_NAME_GRACE_USEC, + service_dispatch_bus_name_grace); + if (r < 0) + return r; + } + if (pidref_is_set(&s->main_pid) && pidref_is_unwaited(&s->main_pid) > 0 && SERVICE_STATE_WITH_MAIN_PROCESS(s->deserialized_state)) { @@ -2741,14 +2757,11 @@ static void service_enter_stop(Service *s, ServiceResult f) { service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_SUCCESS); } -static bool service_good(Service *s) { +static bool service_good_except_bus_name(Service *s) { int main_pid_ok; assert(s); - if (s->type == SERVICE_DBUS && !s->bus_name_good) - return false; - main_pid_ok = main_pid_good(s); if (main_pid_ok > 0) /* It's alive */ return true; @@ -2762,6 +2775,26 @@ static bool service_good(Service *s) { return cgroup_good(s) != 0; } +static bool service_good(Service *s) { + assert(s); + + if (s->type == SERVICE_DBUS && !s->bus_name_good) + return false; + + return service_good_except_bus_name(s); +} + +static void service_enter_exited_or_stop(Service *s) { + assert(s); + + s->revalidate_runtime_begin_usec = USEC_INFINITY; + + if (s->remain_after_exit) + service_set_state(s, SERVICE_EXITED); + else + service_enter_stop(s, SERVICE_SUCCESS); +} + static void service_enter_running(Service *s, ServiceResult f) { int r; @@ -2784,7 +2817,17 @@ static void service_enter_running(Service *s, ServiceResult f) { else { service_set_state(s, SERVICE_RUNNING); - r = service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); + /* We're resuming. Recompute the budget from the current config (which was measured + * from when we entered revalidation). */ + usec_t remaining = USEC_INFINITY; + if (s->revalidate_runtime_begin_usec != USEC_INFINITY) + remaining = usec_sub_unsigned(service_running_timeout(s), s->revalidate_runtime_begin_usec); + s->revalidate_runtime_begin_usec = USEC_INFINITY; + + if (remaining != USEC_INFINITY) + r = service_arm_timer(s, /* relative= */ true, remaining); + else + r = service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); service_enter_running(s, SERVICE_FAILURE_RESOURCES); @@ -2792,10 +2835,33 @@ static void service_enter_running(Service *s, ServiceResult f) { } } - } else if (s->remain_after_exit) - service_set_state(s, SERVICE_EXITED); - else - service_enter_stop(s, SERVICE_SUCCESS); + } else if (s->type == SERVICE_DBUS && !s->bus_name_good && service_good_except_bus_name(s)) { + + /* If we're already revalidating, just keep waiting on the timer we armed on the way in, so a + * flapping broker that reconnects repeatedly can't keep pushing the deadline out. */ + if (s->state == SERVICE_RUNNING_REVALIDATING) + return; + + log_unit_warning(UNIT(s), + "D-Bus name %s vanished (possibly from an unsupported broker restart), giving it %s to return.", + s->bus_name, + FORMAT_TIMESPAN(SERVICE_BUS_NAME_GRACE_USEC, USEC_PER_SEC)); + + r = unit_arm_timer(UNIT(s), &s->bus_name_grace_event_source, /* relative= */ true, + SERVICE_BUS_NAME_GRACE_USEC, service_dispatch_bus_name_grace); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to arm D-Bus name grace timer: %m"); + service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + return; + } + + if (s->state == SERVICE_RUNNING) + s->revalidate_runtime_begin_usec = now(CLOCK_MONOTONIC); + + service_set_state(s, SERVICE_RUNNING_REVALIDATING); + + } else + service_enter_exited_or_stop(s); } static void service_enter_start_post(Service *s) { @@ -3561,6 +3627,7 @@ static int service_start(Unit *u) { s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; s->reload_begin_usec = USEC_INFINITY; + s->revalidate_runtime_begin_usec = USEC_INFINITY; s->status_text = mfree(s->status_text); s->status_errno = 0; @@ -3926,6 +3993,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_usec(f, "watchdog-override-usec", s->watchdog_override_usec); (void) serialize_usec(f, "reload-begin-usec", s->reload_begin_usec); + (void) serialize_usec(f, "revalidate-runtime-begin-usec", s->revalidate_runtime_begin_usec); if (s->refreshed_mask > 0) { _cleanup_strv_free_ char **l = NULL; @@ -4361,6 +4429,8 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, } else if (streq(key, "reload-begin-usec")) (void) deserialize_usec(value, &s->reload_begin_usec); + else if (streq(key, "revalidate-runtime-begin-usec")) + (void) deserialize_usec(value, &s->revalidate_runtime_begin_usec); else if (streq(key, "refreshed-mask")) { r = service_refresh_on_reload_from_string_many(value, &s->refreshed_mask); if (r < 0) @@ -5446,7 +5516,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) if (strv_contains(tags, "STOPPING=1")) { s->notify_state = NOTIFY_STOPPING; - if (IN_SET(s->state, SERVICE_RUNNING, + if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY)) service_enter_stop_by_notify(s); @@ -5483,7 +5553,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) /* Valid Type=notify-reload protocol? Then we're all good. */ service_enter_reload_post(s); - else if (s->state == SERVICE_RUNNING) { + else if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING)) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; /* Propagate a reload explicitly for plain RELOADING=1 (semantically equivalent to @@ -5524,6 +5594,8 @@ static void service_notify_message_process_state(Service *s, char * const *tags) * don't need reload propagation nor do we want to restart the timeout. */ service_set_state(s, SERVICE_RELOAD_NOTIFY); + /* The reload will still be preserved by service_enter_running() replaying it once the bus + * name returns. */ if (s->state == SERVICE_RUNNING) service_enter_reload_by_notify(s); } @@ -5558,7 +5630,7 @@ static void service_notify_message( r = service_notify_message_parse_new_pid(u, tags, fds, &new_main_pid); if (r > 0 && - IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_STOP, SERVICE_STOP_SIGTERM) && @@ -5702,7 +5774,7 @@ static void service_notify_message( } /* Interpret RESTART_RESET=1 */ - if (strv_contains(tags, "RESTART_RESET=1") && IN_SET(s->state, SERVICE_RUNNING, SERVICE_STOP)) { + if (strv_contains(tags, "RESTART_RESET=1") && IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_STOP)) { log_unit_struct(u, LOG_NOTICE, LOG_UNIT_MESSAGE(u, "Got RESTART_RESET=1, resetting restart counter from %u.", s->n_restarts), LOG_ITEM("N_RESTARTS=0"), @@ -5877,7 +5949,19 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s return 1; } -static void service_bus_name_owner_change(Unit *u, const char *new_owner) { +static int service_dispatch_bus_name_grace(sd_event_source *source, usec_t usec, void *userdata) { + Service *s = ASSERT_PTR(SERVICE(userdata)); + + assert(source == s->bus_name_grace_event_source); + assert(s->state == SERVICE_RUNNING_REVALIDATING); + + log_unit_warning(UNIT(s), "D-Bus name %s still not owned after grace period, giving up.", s->bus_name); + service_enter_exited_or_stop(s); + + return 0; +} + +static void service_bus_name_owner_change(Unit *u, const char *new_owner, bool from_signal) { Service *s = ASSERT_PTR(SERVICE(u)); int r; @@ -5889,10 +5973,12 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) { s->bus_name_good = new_owner; if (s->type == SERVICE_DBUS) { - /* service_enter_running() will figure out what to do */ - if (s->state == SERVICE_RUNNING) - service_enter_running(s, SERVICE_SUCCESS); - else if (s->state == SERVICE_START && new_owner) + if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING)) { + if (!new_owner && from_signal) + service_enter_exited_or_stop(s); + else + service_enter_running(s, SERVICE_SUCCESS); + } else if (s->state == SERVICE_START && new_owner) service_enter_start_post(s); } else if (new_owner && pick_up_pid_from_bus_name(s)) { diff --git a/src/core/service.h b/src/core/service.h index c028248bba..c6002eb1ba 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -163,6 +163,7 @@ typedef struct Service { bool timeout_abort_set; usec_t runtime_max_usec; usec_t runtime_rand_extra_usec; + usec_t revalidate_runtime_begin_usec; ServiceTimeoutFailureMode timeout_start_failure_mode; ServiceTimeoutFailureMode timeout_stop_failure_mode; @@ -226,6 +227,7 @@ typedef struct Service { bool exec_fd_hot:1; char *bus_name; + sd_event_source *bus_name_grace_event_source; char *status_text; char *status_bus_error; diff --git a/src/core/unit.c b/src/core/unit.c index 1d5fe8859e..31d3740d0f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3593,7 +3593,7 @@ static int signal_name_owner_changed_install_handler(sd_bus_message *message, vo u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot); if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, NULL); + UNIT_VTABLE(u)->bus_name_owner_change(u, NULL, /* from_signal= */ true); return 0; } @@ -3612,7 +3612,7 @@ static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd } if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, empty_to_null(new_owner)); + UNIT_VTABLE(u)->bus_name_owner_change(u, empty_to_null(new_owner), /* from_signal= */ true); return 0; } @@ -3645,8 +3645,11 @@ static int get_name_owner_handler(sd_bus_message *message, void *userdata, sd_bu assert(!isempty(new_owner)); } + /* Given we're in a GetNameOwner() refresh, which we issue right after (re)connecting to the broker, + * a NULL owner here may simply mean the owning client is still getting its bearings, so treat it as + * non-definitive and worth waiting a bit for. */ if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, new_owner); + UNIT_VTABLE(u)->bus_name_owner_change(u, new_owner, /* from_signal= */ false); return 0; } diff --git a/src/core/unit.h b/src/core/unit.h index f96ae27936..c07588ac1e 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -670,7 +670,7 @@ typedef struct UnitVTable { void (*notify_pidref)(Unit *u, PidRef *parent_pidref, PidRef *child_pidref); /* Called whenever a name this Unit registered for comes or goes away. */ - void (*bus_name_owner_change)(Unit *u, const char *new_owner); + void (*bus_name_owner_change)(Unit *u, const char *new_owner, bool from_signal); /* Called for each property that is being set */ int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *reterr_error); From df3c9c9ef052b6c235d4ca860249da5f3e93c181 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Thu, 4 Jun 2026 12:08:57 +0800 Subject: [PATCH 3/3] test: Add a test for Type=dbus services surviving a broker restart --- .../system.d/systemd.test.DBusReconnect.conf | 13 ++ src/test/meson.build | 4 + src/test/test-dbus-name-holder.c | 77 +++++++++ .../units/TEST-23-UNIT-FILE.dbus-reconnect.sh | 161 ++++++++++++++++++ 4 files changed, 255 insertions(+) create mode 100644 mkosi/mkosi.extra/usr/share/dbus-1/system.d/systemd.test.DBusReconnect.conf create mode 100644 src/test/test-dbus-name-holder.c create mode 100755 test/units/TEST-23-UNIT-FILE.dbus-reconnect.sh diff --git a/mkosi/mkosi.extra/usr/share/dbus-1/system.d/systemd.test.DBusReconnect.conf b/mkosi/mkosi.extra/usr/share/dbus-1/system.d/systemd.test.DBusReconnect.conf new file mode 100644 index 0000000000..ab3687743c --- /dev/null +++ b/mkosi/mkosi.extra/usr/share/dbus-1/system.d/systemd.test.DBusReconnect.conf @@ -0,0 +1,13 @@ + + + + + + + + + + diff --git a/src/test/meson.build b/src/test/meson.build index da3148d955..d0d4f25f17 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -315,6 +315,10 @@ executables += [ 'dependencies' : libopenssl_cflags, 'conditions' : ['HAVE_OPENSSL'], }, + test_template + { + 'sources' : files('test-dbus-name-holder.c'), + 'type' : 'manual', + }, test_template + { 'sources' : files('test-display-quota.c'), 'type' : 'manual', diff --git a/src/test/test-dbus-name-holder.c b/src/test/test-dbus-name-holder.c new file mode 100644 index 0000000000..011ce782ea --- /dev/null +++ b/src/test/test-dbus-name-holder.c @@ -0,0 +1,77 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include + +#include "sd-bus.h" +#include "sd-daemon.h" + +#include "log.h" +#include "main-func.h" +#include "time-util.h" + +static int run(int argc, char *argv[]) { + int r; + + if (argc < 3 || argc > 5) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Usage: %s BUSNAME READY_MARKER [RELEASE_MARKER] [NOTIFY_MARKER]", + program_invocation_short_name); + + const char + *name = argv[1], + *marker = argv[2], + *release_marker = argc > 3 ? argv[3] : NULL, + *notify_marker = argc > 4 ? argv[4] : NULL; + + for (;;) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + r = sd_bus_open_system(&bus); + if (r < 0) + return log_error_errno(r, "Failed to open system bus: %m"); + + r = sd_bus_request_name(bus, name, /* flags= */ 0); + if (r < 0) + return log_error_errno(r, "Failed to acquire dbus name '%s': %m", name); + + log_info("Acquired dbus name '%s'.", name); + + for (;;) { + r = sd_bus_process(bus, /* ret= */ NULL); + if (r < 0) + break; + if (r > 0) + continue; + + if (release_marker && access(release_marker, F_OK) >= 0) { + r = sd_bus_release_name(bus, name); + if (r < 0) + return log_error_errno(r, "Failed to release dbus name '%s': %m", name); + + log_info("Released dbus name '%s'.", name); + release_marker = NULL; /* go back to blocking until we're killed */ + } + + r = sd_bus_wait(bus, release_marker ? 100 * USEC_PER_MSEC : USEC_INFINITY); + if (r < 0) + break; + } + + log_info("Disconnected from bus, reconnecting..."); + + while (access(marker, F_OK) < 0) { + if (notify_marker && access(notify_marker, F_OK) >= 0) { + r = sd_notify(/* unset_environment= */ false, "RELOADING=1\nREADY=1"); + if (r < 0) + return log_error_errno(r, "Failed to send reload notification: %m"); + + log_info("Sent reload notification."); + notify_marker = NULL; + } + + usleep_safe(50 * USEC_PER_MSEC); + } + } +} + +DEFINE_MAIN_FUNCTION(run); diff --git a/test/units/TEST-23-UNIT-FILE.dbus-reconnect.sh b/test/units/TEST-23-UNIT-FILE.dbus-reconnect.sh new file mode 100755 index 0000000000..9a64131cd5 --- /dev/null +++ b/test/units/TEST-23-UNIT-FILE.dbus-reconnect.sh @@ -0,0 +1,161 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eux +set -o pipefail + +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + +holder=/usr/lib/systemd/tests/unit-tests/manual/test-dbus-name-holder +name=systemd.test.DBusReconnect +marker=/run/dbus-reconnect-go +release=/run/dbus-reconnect-release +notify=/run/dbus-reconnect-notify +reloaded=/run/dbus-reconnect-reloaded + +at_exit() { + rm -f "$marker" "$release" "$notify" "$reloaded" + systemctl stop \ + dbus-reconnect-recover.service \ + dbus-reconnect-stop.service \ + dbus-reconnect-exit.service \ + dbus-reconnect-drop.service \ + dbus-reconnect-notify.service \ + dbus-reconnect-notify-target.service \ + dbus-reconnect-reexec.service 2>/dev/null || : +} + +trap at_exit EXIT + +# Poll until property $2 of unit $1 reaches value $3 +poll() { + local unit="${1:?}" prop="${2:?}" want="${3:?}" got i + for ((i = 0; i < 150; i++)); do + got="$(systemctl show -p "$prop" --value "$unit")" + if [[ "$got" == "$want" ]]; then + return 0 + fi + sleep 0.2 + done + echo "poll: timed out waiting for $unit $prop=$want (last: $got)" >&2 + return 1 +} + +reset_failed() { + local unit="${1:?}" + + systemctl reset-failed "$unit" 2>/dev/null || : +} + +restart_broker() { + systemctl kill --signal=KILL dbus.service + timeout 90 bash -c ' + until busctl --system status >/dev/null 2>&1; do + systemctl reset-failed dbus.service dbus.socket 2>/dev/null + systemctl start dbus.socket 2>/dev/null + sleep 0.2 + done' +} + +enter_revalidation() { + local unit="${1:?}" + + shift + reset_failed "$unit" + rm -f "$marker" + systemd-run \ + --unit="$unit" \ + -p Type=dbus \ + -p BusName="$name" \ + -p Restart=no \ + "$@" \ + "$holder" "$name" "$marker" + restart_broker + poll "$unit" SubState running-revalidating +} + +# the name comes back within the grace window +unit=dbus-reconnect-recover.service +enter_revalidation "$unit" +main_pid="$(systemctl show -p MainPID --value "$unit")" +assert_neq "$main_pid" "0" +touch "$marker" +poll "$unit" SubState running +assert_eq "$(systemctl show -p MainPID --value "$unit")" "$main_pid" +systemctl stop "$unit" + +# the name never comes back +unit=dbus-reconnect-stop.service +enter_revalidation "$unit" +poll "$unit" ActiveState inactive + +# ditto with RemainAfterExit=yes +unit=dbus-reconnect-exit.service +enter_revalidation "$unit" -p RemainAfterExit=yes +poll "$unit" SubState exited +systemctl stop "$unit" + +# the name is dropped on a live connection and we action it at once +unit=dbus-reconnect-drop.service +reset_failed "$unit" +rm -f "$release" +systemd-run \ + --unit="$unit" \ + -p Type=dbus \ + -p BusName="$name" \ + -p Restart=no \ + "$holder" "$name" "$marker" "$release" +poll "$unit" SubState running +touch "$release" +for ((i = 0; i < 150; i++)); do + sub_state="$(systemctl show -p SubState --value "$unit")" + if [[ "$sub_state" == running-revalidating ]]; then + echo "live drop entered the revalidation window" >&2 + exit 1 + fi + + active_state="$(systemctl show -p ActiveState --value "$unit")" + if [[ "$active_state" == inactive ]]; then + break + fi + + sleep 0.2 +done +assert_eq "$(systemctl show -p ActiveState --value "$unit")" inactive + +# a reload notification is propagated even while the name is absent (revalidating) +target=dbus-reconnect-notify-target.service +reset_failed "$target" +systemd-run \ + --unit="$target" \ + -p Type=oneshot \ + -p RemainAfterExit=yes \ + -p "ExecReload=/usr/bin/touch $reloaded" \ + /usr/bin/true + +unit=dbus-reconnect-notify.service +reset_failed "$unit" +rm -f "$marker" "$release" "$notify" "$reloaded" +systemd-run \ + --unit="$unit" \ + -p Type=dbus \ + -p BusName="$name" \ + -p Restart=no \ + -p NotifyAccess=main \ + -p PropagatesReloadTo="$target" \ + "$holder" "$name" "$marker" "$release" "$notify" +restart_broker +poll "$unit" SubState running-revalidating +touch "$notify" +timeout 30 bash -c "until [[ -e '$reloaded' ]]; do sleep 0.2; done" +touch "$marker" +poll "$unit" SubState running +systemctl stop "$unit" "$target" + +# the revalidation state survives a daemon-reexec and still recovers afterwards +unit=dbus-reconnect-reexec.service +enter_revalidation "$unit" +systemctl daemon-reexec +touch "$marker" +poll "$unit" SubState running +systemctl stop "$unit"