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.
This commit is contained in:
Sinity
2026-07-11 16:15:31 +02:00
committed by Yu Watanabe
parent d751846043
commit 1166f4472d
5 changed files with 109 additions and 39 deletions

View File

@@ -817,6 +817,68 @@ static int bus_setup_api(Manager *m, sd_bus *bus) {
return 0; 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) { int bus_init_api(Manager *m) {
_cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL;
int r; int r;
@@ -846,17 +908,40 @@ int bus_init_api(Manager *m) {
return r; return r;
} }
r = bus_setup_api(m, bus); /* Query the bus instance ID asynchronously, and let the reply handler decide whether the
if (r < 0) * subscription state deserialized from a previous reload/reexec (or saved when the previous
return log_error_errno(r, "Failed to set up API bus: %m"); * 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); r = bus_setup_api(m, bus);
if (r < 0) if (r < 0)
log_warning_errno(r, "Failed to query API bus instance ID, not deserializing subscriptions: %m"); return log_error_errno(r, "Failed to set up API bus: %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;
m->api_bus = TAKE_PTR(bus); m->api_bus = TAKE_PTR(bus);
return 0; return 0;

View File

@@ -200,8 +200,10 @@ int manager_serialize(
(void) serialize_ratelimit(f, "reload-reexec-ratelimit", &m->reload_reexec_ratelimit); (void) serialize_ratelimit(f, "reload-reexec-ratelimit", &m->reload_reexec_ratelimit);
(void) serialize_ratelimit(f, "event-loop-ratelimit", &m->event_loop_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"); bus_track_serialize(m->subscribed, f, "subscribed");
(void) serialize_strv(f, "subscribed", m->subscribed_as_strv);
r = dynamic_user_serialize(m, f, fds); r = dynamic_user_serialize(m, f, fds);
if (r < 0) if (r < 0)

View File

@@ -3917,8 +3917,10 @@ int manager_override_watchdog_pretimeout_governor(Manager *m, const char *govern
int manager_reload(Manager *m) { int manager_reload(Manager *m) {
_unused_ _cleanup_(manager_reloading_stopp) Manager *reloading = NULL; _unused_ _cleanup_(manager_reloading_stopp) Manager *reloading = NULL;
_cleanup_strv_free_ char **saved_subscribed_as_strv = NULL;
_cleanup_fdset_free_ FDSet *fds = NULL; _cleanup_fdset_free_ FDSet *fds = NULL;
_cleanup_fclose_ FILE *f = NULL; _cleanup_fclose_ FILE *f = NULL;
sd_id128_t saved_deserialized_bus_id;
int r; int r;
assert(m); assert(m);
@@ -3974,6 +3976,10 @@ int manager_reload(Manager *m) {
manager_enumerate(m); manager_enumerate(m);
/* Second, deserialize our stored data */ /* 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); r = manager_deserialize(m, f, fds);
if (r < 0) if (r < 0)
log_warning_errno(r, "Deserialization failed, proceeding anyway: %m"); 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_handoff_timestamp_fd(m);
(void) manager_setup_pidref_transport_fd(m); (void) manager_setup_pidref_transport_fd(m);
/* Clean up deserialized bus track information. They're never consumed during reload (as opposed to /* Discard the bus track information produced by this reload, since the bus stays connected. Preserve
* reexec) since we do not disconnect from the bus. */ * 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->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! */ /* Third, fire things up! */
manager_coldplug(m); manager_coldplug(m);

View File

@@ -10,7 +10,6 @@
#include "sd-bus.h" #include "sd-bus.h"
#include "sd-daemon.h" #include "sd-daemon.h"
#include "sd-event.h" #include "sd-event.h"
#include "sd-id128.h"
#include "alloc-util.h" #include "alloc-util.h"
#include "bus-common-errors.h" #include "bus-common-errors.h"
@@ -916,28 +915,6 @@ int bus_query_sender_pidref(
return bus_creds_get_pidref(creds, ret); 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[] = { static const char* const bus_transport_table[] = {
[BUS_TRANSPORT_LOCAL] = "local", [BUS_TRANSPORT_LOCAL] = "local",
[BUS_TRANSPORT_REMOTE] = "remote", [BUS_TRANSPORT_REMOTE] = "remote",

View 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_creds_get_pidref(sd_bus_creds *c, PidRef *ret);
int bus_query_sender_pidref(sd_bus_message *m, 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); DECLARE_STRING_TABLE_LOOKUP_TO_STRING(bus_transport, BusTransport);