]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
core/dbus: do not block the manager on GetId during bus (re-)connection
authorSinity <ezo.dev@gmail.com>
Sat, 11 Jul 2026 14:15:31 +0000 (16:15 +0200)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Mon, 20 Jul 2026 02:53:34 +0000 (11:53 +0900)
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
src/core/manager-serialize.c
src/core/manager.c
src/shared/bus-util.c
src/shared/bus-util.h

index 659965e4e6c50e40eacc27f0e3b13e2bd91eb584..5e5c3ccf1bfa0e5ea7b64941cfdd5de792681bbc 100644 (file)
@@ -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;
index efa9f65bfb4b55d3ff658ad9b70f1c6b9d31fa62..7703d63e7a4979b3f58103212bdbb621680d716b 100644 (file)
@@ -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)
index f5daa59e1abf440dbe8d15f2e9b98e960675b141..ad525539ce59ef28a39c02ba6553e66d05024acd 100644 (file)
@@ -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);
index 444c4df0fb43646be4f8169365be080bf7778aa1..f8ff9778b998a0eb4cb2f2bfd85a34cadb163246 100644 (file)
@@ -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",
index c69ed45617309f265670aa59934507b064c16542..12c2c80081d54fec48424c4384c123270b53c4fe 100644 (file)
@@ -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);