From 1166f4472d7669c8008c158a178dd6f76b601fe1 Mon Sep 17 00:00:00 2001 From: Sinity Date: Sat, 11 Jul 2026 16:15:31 +0200 Subject: [PATCH] core/dbus: do not block the manager on GetId during bus (re-)connection bus_init_api() issued a synchronous GetId call on every API bus (re-)connection to decide whether saved subscription state could be coldplugged onto the new connection. If the D-Bus socket unit is listening while the message bus daemon behind it is gone, connect() succeeds against the socket backlog but nothing answers the authentication handshake. The synchronous call then blocks PID 1 for BUS_AUTH_TIMEOUT (90 seconds by default), and queued bus operations can trigger repeated reconnection attempts. This was observed during shutdown as roughly 15 minutes of teardown progressing only in 90-second intervals. Query the instance ID asynchronously on every connection. Defer API setup until the reply is processed, so saved subscriptions are validated and coldplugged before new subscription requests can arrive. If the query cannot be queued or its reply is invalid, discard the unvalidated state and expose the API without blocking the manager. Reset the live bus ID on every connection and serialize pending bus ID and subscription state across reload and reexec. During daemon-reload, preserve state that was already awaiting the asynchronous reply while discarding the duplicate state produced by the reload itself. Also remove the now-unused synchronous bus_get_instance_id() helper. --- src/core/dbus.c | 105 +++++++++++++++++++++++++++++++---- src/core/manager-serialize.c | 4 +- src/core/manager.c | 14 ++++- src/shared/bus-util.c | 23 -------- src/shared/bus-util.h | 2 - 5 files changed, 109 insertions(+), 39 deletions(-) diff --git a/src/core/dbus.c b/src/core/dbus.c index 659965e4e6c..5e5c3ccf1bf 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -817,6 +817,68 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { return 0; } +static int api_bus_instance_id_reply(sd_bus_message *reply, void *userdata, sd_bus_error *reterr_error) { + Manager *m = ASSERT_PTR(userdata); + const sd_bus_error *e; + sd_bus *bus; + const char *id; + int r; + + assert(reply); + assert_se(bus = sd_bus_message_get_bus(reply)); + + /* Whatever the outcome, we get only one shot at coldplugging the saved subscription state onto + * this connection: consume it. */ + _cleanup_strv_free_ char **subscribed_as_strv = TAKE_PTR(m->subscribed_as_strv); + sd_id128_t deserialized_bus_id = m->deserialized_bus_id; + m->deserialized_bus_id = SD_ID128_NULL; + + e = sd_bus_message_get_error(reply); + if (e) { + r = sd_bus_error_get_errno(e); + log_full_errno(subscribed_as_strv ? LOG_WARNING : LOG_DEBUG, r, + "Failed to query API bus instance ID%s: %s", + subscribed_as_strv ? ", not deserializing subscriptions" : ", ignoring", + bus_error_message(e, r)); + goto setup_api; + } + + r = sd_bus_message_read_basic(reply, 's', &id); + if (r < 0) { + log_full_errno(subscribed_as_strv ? LOG_WARNING : LOG_DEBUG, r, + "Failed to read API bus instance ID%s: %m", + subscribed_as_strv ? ", not deserializing subscriptions" : ", ignoring"); + goto setup_api; + } + + r = sd_id128_from_string(id, &m->bus_id); + if (r < 0) { + log_full_errno(subscribed_as_strv ? LOG_WARNING : LOG_DEBUG, r, + "API bus instance ID not in expected format%s: %m", + subscribed_as_strv ? ", not deserializing subscriptions" : ", ignoring"); + goto setup_api; + } + + /* Coldplug the subscriptions deserialized from a previous reload/reexec (or saved when the + * previous connection was torn down), but only onto the same bus instance they were recorded on. + * If no instance ID was recorded, retain the historical behavior and coldplug without comparison. */ + if (sd_id128_is_null(deserialized_bus_id) || sd_id128_equal(m->bus_id, deserialized_bus_id)) + (void) bus_track_coldplug(bus, &m->subscribed, subscribed_as_strv); + +setup_api: + r = bus_setup_api(m, bus); + if (r < 0) { + log_error_errno(r, "Failed to set up API bus: %m"); + + if (m->system_bus == bus) + bus_done_system(m); + if (m->api_bus == bus) + bus_done_api(m); + } + + return 0; +} + int bus_init_api(Manager *m) { _cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL; int r; @@ -846,17 +908,40 @@ int bus_init_api(Manager *m) { return r; } - r = bus_setup_api(m, bus); - if (r < 0) - return log_error_errno(r, "Failed to set up API bus: %m"); + /* Query the bus instance ID asynchronously, and let the reply handler decide whether the + * subscription state deserialized from a previous reload/reexec (or saved when the previous + * connection was torn down) may be coldplugged onto this connection. Set up the API only after the + * reply is processed, so new subscriptions cannot be mixed with the pending state before its bus + * instance is validated. A synchronous call is not OK here: this is reached on every + * (re-)connection attempt, and if the D-Bus socket unit is + * listening while the message bus daemon behind it is gone (e.g. it crashed during shutdown while + * we still consider its service to be up), a synchronous call over the never-answered connection + * blocks the whole manager for BUS_AUTH_TIMEOUT — repeatedly, since every queued bus operation + * triggers another reconnection attempt. The previous connection's instance ID is meaningless for + * this connection, hence reset it: only ever populate the live ID from the reply, while any pending + * validation ID and subscription state remain serializable across a racing reload/reexec. */ + m->bus_id = SD_ID128_NULL; + r = sd_bus_call_method_async( + bus, + /* ret_slot= */ NULL, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + "GetId", + api_bus_instance_id_reply, + m, + /* types= */ NULL); + if (r < 0) { + log_full_errno(m->subscribed_as_strv ? LOG_WARNING : LOG_DEBUG, r, + "Failed to enqueue API bus instance ID query%s: %m", + m->subscribed_as_strv ? ", not deserializing subscriptions" : ", ignoring"); + m->subscribed_as_strv = strv_free(m->subscribed_as_strv); + m->deserialized_bus_id = SD_ID128_NULL; - r = bus_get_instance_id(bus, &m->bus_id); - if (r < 0) - log_warning_errno(r, "Failed to query API bus instance ID, not deserializing subscriptions: %m"); - else if (sd_id128_is_null(m->deserialized_bus_id) || sd_id128_equal(m->bus_id, m->deserialized_bus_id)) - (void) bus_track_coldplug(bus, &m->subscribed, m->subscribed_as_strv); - m->subscribed_as_strv = strv_free(m->subscribed_as_strv); - m->deserialized_bus_id = SD_ID128_NULL; + r = bus_setup_api(m, bus); + if (r < 0) + return log_error_errno(r, "Failed to set up API bus: %m"); + } m->api_bus = TAKE_PTR(bus); return 0; diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c index efa9f65bfb4..7703d63e7a4 100644 --- a/src/core/manager-serialize.c +++ b/src/core/manager-serialize.c @@ -200,8 +200,10 @@ int manager_serialize( (void) serialize_ratelimit(f, "reload-reexec-ratelimit", &m->reload_reexec_ratelimit); (void) serialize_ratelimit(f, "event-loop-ratelimit", &m->event_loop_ratelimit); - (void) serialize_id128(f, "bus-id", m->bus_id); + (void) serialize_id128(f, "bus-id", + sd_id128_is_null(m->bus_id) ? m->deserialized_bus_id : m->bus_id); bus_track_serialize(m->subscribed, f, "subscribed"); + (void) serialize_strv(f, "subscribed", m->subscribed_as_strv); r = dynamic_user_serialize(m, f, fds); if (r < 0) diff --git a/src/core/manager.c b/src/core/manager.c index f5daa59e1ab..ad525539ce5 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3917,8 +3917,10 @@ int manager_override_watchdog_pretimeout_governor(Manager *m, const char *govern int manager_reload(Manager *m) { _unused_ _cleanup_(manager_reloading_stopp) Manager *reloading = NULL; + _cleanup_strv_free_ char **saved_subscribed_as_strv = NULL; _cleanup_fdset_free_ FDSet *fds = NULL; _cleanup_fclose_ FILE *f = NULL; + sd_id128_t saved_deserialized_bus_id; int r; assert(m); @@ -3974,6 +3976,10 @@ int manager_reload(Manager *m) { manager_enumerate(m); /* Second, deserialize our stored data */ + saved_subscribed_as_strv = TAKE_PTR(m->subscribed_as_strv); + saved_deserialized_bus_id = m->deserialized_bus_id; + m->deserialized_bus_id = SD_ID128_NULL; + r = manager_deserialize(m, f, fds); if (r < 0) log_warning_errno(r, "Deserialization failed, proceeding anyway: %m"); @@ -3987,10 +3993,12 @@ int manager_reload(Manager *m) { (void) manager_setup_handoff_timestamp_fd(m); (void) manager_setup_pidref_transport_fd(m); - /* Clean up deserialized bus track information. They're never consumed during reload (as opposed to - * reexec) since we do not disconnect from the bus. */ + /* Discard the bus track information produced by this reload, since the bus stays connected. Preserve + * any validation state that was already pending before the reload, so its asynchronous GetId reply can + * still consume it when we return to the event loop. */ m->subscribed_as_strv = strv_free(m->subscribed_as_strv); - m->deserialized_bus_id = SD_ID128_NULL; + m->subscribed_as_strv = TAKE_PTR(saved_subscribed_as_strv); + m->deserialized_bus_id = saved_deserialized_bus_id; /* Third, fire things up! */ manager_coldplug(m); diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 444c4df0fb4..f8ff9778b99 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -10,7 +10,6 @@ #include "sd-bus.h" #include "sd-daemon.h" #include "sd-event.h" -#include "sd-id128.h" #include "alloc-util.h" #include "bus-common-errors.h" @@ -916,28 +915,6 @@ int bus_query_sender_pidref( return bus_creds_get_pidref(creds, ret); } -int bus_get_instance_id(sd_bus *bus, sd_id128_t *ret) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - int r; - - assert(bus); - assert(ret); - - r = sd_bus_call_method(bus, - "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "GetId", - /* reterr_error= */ NULL, &reply, NULL); - if (r < 0) - return r; - - const char *id; - - r = sd_bus_message_read_basic(reply, 's', &id); - if (r < 0) - return r; - - return sd_id128_from_string(id, ret); -} - static const char* const bus_transport_table[] = { [BUS_TRANSPORT_LOCAL] = "local", [BUS_TRANSPORT_REMOTE] = "remote", diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index c69ed456173..12c2c80081d 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -70,6 +70,4 @@ int bus_register_malloc_status(sd_bus *bus, const char *destination); int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); -int bus_get_instance_id(sd_bus *bus, sd_id128_t *ret); - DECLARE_STRING_TABLE_LOOKUP_TO_STRING(bus_transport, BusTransport); -- 2.47.3