1
0
mirror of https://github.com/systemd/systemd synced 2025-11-21 01:34:44 +01:00

Compare commits

...

7 Commits

Author SHA1 Message Date
Mike Yuan
e2b38ddd5f core/manager: restore bus track deserialization cleanup in manager_reload()
There's zero explanation why it got (spuriously) removed in
8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98...

(cherry picked from commit 34f4b817f67b002eae7e2c09b19bf4b66c4791b6)
2025-10-09 13:22:30 +02:00
Mike Yuan
8bf0ceba3d core/manager: drop duplicate bus track deserialization
bus_init_api() now does this internally
(after 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98).

(cherry picked from commit af0e10354e567bfd0b9521376b2aad55f12a4e3d)
2025-10-09 13:22:30 +02:00
Mike Yuan
9e66436138 bus-util: do not reset the count returned by sd_bus_track_count_name()
Follow-up for 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98

While at it, turn the retval check for sd_bus_track_count_name()
into assertion, given we're working with already established tracks
(service_name_is_valid() should never yield false in this case).

Addresses https://github.com/systemd/systemd/pull/35406#discussion_r1912066774

(cherry picked from commit 33eeea4128f31df7ab4bd8866b582062d70114ae)
2025-10-09 13:22:30 +02:00
Ronan Pigott
977f43d903 manager: s/deserialized_subscribed/subscribed_as_strv
Now that this field may get populated at runtime, the deserialized name
is misleading. Change the name to reflect its updated purpose.

(cherry picked from commit e1315a621ae26473fcc9cd0d6013836f5f498d40)
2025-10-09 13:22:30 +02:00
Ronan Pigott
3e4d34a9f8 dbus: stash the subscriber list when we disconenct from the bus
If we unexpectly disconnect from the bus, systemd would end up dropping
the list of subscribers, which breaks the ability of clients like logind
to monitor the state of units.

Stash the list of subscribers into the deserialized state in the event
of a disconnect so that when we recover we can renew the broken
subscriptions.

(cherry picked from commit 8402ca04d1a063c3d8a9e3d5c16df8bb8778ae98)
2025-10-09 13:22:30 +02:00
Michal Sekletar
129b522575 sd-bus/bus-track: use install_callback in sd_bus_track_add_name()
Previously we didn't provide any install_callback to
sd_bus_add_match_async() so in case AddMatch() method call timed out we
destroyed the bus connection. This seems overly aggressive and simply
updating the sd_bus_track object accordingly should be enough.

Follow-up for 37ce3fd2b7dd8f81f6f4bca2003961a92b2963dc.

Fixes #32381

(cherry picked from commit dcf42d1ee21222ee698e5e0ab3ecf3411b63da40)
2025-10-09 13:22:30 +02:00
Yu Watanabe
0ca0efee86 core: do not disconnect from bus when failed to install signal match
If bus_add_match_full() is called without install callback and we failed
to install the signal match e.g. by timeout, then add_match_callback()
will disconnect from the bus.
Let's use a custom install handler and handle failures gracefully.

This does not *solve* the root cause of issue #30573, but should improve
the situation when the issue is triggered.

(cherry picked from commit db6b214f95aa42f9a9fa3d94a3c6492cc57b58fb)
2025-10-09 13:22:30 +02:00
9 changed files with 103 additions and 24 deletions

View File

@ -859,6 +859,8 @@ int bus_init_api(Manager *m) {
if (r < 0)
return log_error_errno(r, "Failed to set up API bus: %m");
(void) bus_track_coldplug(bus, &m->subscribed, /* recursive= */ false, m->subscribed_as_strv);
m->subscribed_as_strv = strv_free(m->subscribed_as_strv);
m->api_bus = TAKE_PTR(bus);
return 0;
@ -1002,8 +1004,17 @@ static void destroy_bus(Manager *m, sd_bus **bus) {
}
/* Get rid of tracked clients on this bus */
if (m->subscribed && sd_bus_track_get_bus(m->subscribed) == *bus)
if (m->subscribed && sd_bus_track_get_bus(m->subscribed) == *bus) {
_cleanup_strv_free_ char **subscribed = NULL;
int r;
r = bus_track_to_strv(m->subscribed, &subscribed);
if (r < 0)
log_warning_errno(r, "Failed to serialize api subscribers, ignoring: %m");
strv_free_and_replace(m->subscribed_as_strv, subscribed);
m->subscribed = sd_bus_track_unref(m->subscribed);
}
HASHMAP_FOREACH(j, m->jobs)
if (j->bus_track && sd_bus_track_get_bus(j->bus_track) == *bus)
@ -1062,7 +1073,6 @@ void bus_done(Manager *m) {
assert(!m->subscribed);
m->deserialized_subscribed = strv_free(m->deserialized_subscribed);
m->polkit_registry = hashmap_free(m->polkit_registry);
}
@ -1151,20 +1161,19 @@ void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix) {
}
}
int bus_track_coldplug(Manager *m, sd_bus_track **t, bool recursive, char **l) {
int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, bool recursive, char **l) {
int r;
assert(m);
assert(t);
if (strv_isempty(l))
return 0;
if (!m->api_bus)
if (!bus)
return 0;
if (!*t) {
r = sd_bus_track_new(m->api_bus, t, NULL, NULL);
r = sd_bus_track_new(bus, t, NULL, NULL);
if (r < 0)
return r;
}

View File

@ -19,7 +19,7 @@ void bus_done(Manager *m);
int bus_fdset_add_all(Manager *m, FDSet *fds);
void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix);
int bus_track_coldplug(Manager *m, sd_bus_track **t, bool recursive, char **l);
int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, bool recursive, char **l);
int bus_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata);

View File

@ -493,7 +493,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
(void) exec_shared_runtime_deserialize_one(m, val, fds);
else if ((val = startswith(l, "subscribed="))) {
r = strv_extend(&m->deserialized_subscribed, val);
r = strv_extend(&m->subscribed_as_strv, val);
if (r < 0)
return r;
} else if ((val = startswith(l, "varlink-server-socket-address="))) {

View File

@ -1803,6 +1803,9 @@ Manager* manager_free(Manager *m) {
free(m->switch_root);
free(m->switch_root_init);
sd_bus_track_unref(m->subscribed);
strv_free(m->subscribed_as_strv);
unit_defaults_done(&m->defaults);
FOREACH_ARRAY(map, m->units_needing_mounts_for, _UNIT_MOUNT_DEPENDENCY_TYPE_MAX) {
@ -2161,12 +2164,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo
/* Connect to the bus if we are good for it */
manager_setup_bus(m);
/* Now that we are connected to all possible buses, let's deserialize who is tracking us. */
r = bus_track_coldplug(m, &m->subscribed, false, m->deserialized_subscribed);
if (r < 0)
log_warning_errno(r, "Failed to deserialized tracked clients, ignoring: %m");
m->deserialized_subscribed = strv_free(m->deserialized_subscribed);
r = manager_varlink_init(m);
if (r < 0)
log_warning_errno(r, "Failed to set up Varlink, ignoring: %m");
@ -3830,15 +3827,16 @@ 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. */
m->subscribed_as_strv = strv_free(m->subscribed_as_strv);
/* Third, fire things up! */
manager_coldplug(m);
/* Clean up runtime objects no longer referenced */
manager_vacuum(m);
/* Clean up deserialized tracked clients */
m->deserialized_subscribed = strv_free(m->deserialized_subscribed);
/* Consider the reload process complete now. */
assert(m->n_reloading > 0);
m->n_reloading--;

View File

@ -340,7 +340,7 @@ struct Manager {
considered subscribes, since they last for very short only,
and it is much simpler that way. */
sd_bus_track *subscribed;
char **deserialized_subscribed;
char **subscribed_as_strv;
/* This is used during reloading: before the reload we queue
* the reply message here, and afterwards we send it */

View File

@ -3521,6 +3521,32 @@ int unit_load_related_unit(Unit *u, const char *type, Unit **_found) {
return r;
}
static int signal_name_owner_changed_install_handler(sd_bus_message *message, void *userdata, sd_bus_error *error) {
Unit *u = ASSERT_PTR(userdata);
const sd_bus_error *e;
int r;
e = sd_bus_message_get_error(message);
if (!e) {
log_unit_trace(u, "Successfully installed NameOwnerChanged signal match.");
return 0;
}
r = sd_bus_error_get_errno(e);
log_unit_error_errno(u, r,
"Unexpected error response on installing NameOwnerChanged signal match: %s",
bus_error_message(e, r));
/* If we failed to install NameOwnerChanged signal, also unref the bus slot of GetNameOwner(). */
u->match_bus_slot = sd_bus_slot_unref(u->match_bus_slot);
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);
return 0;
}
static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus_error *error) {
const char *new_owner;
Unit *u = ASSERT_PTR(userdata);
@ -3603,10 +3629,10 @@ int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) {
r = bus_add_match_full(
bus,
&u->match_bus_slot,
true,
/* asynchronous = */ true,
match,
signal_name_owner_changed,
NULL,
signal_name_owner_changed_install_handler,
u,
timeout_usec);
if (r < 0)

View File

@ -3,6 +3,7 @@
#include "sd-bus.h"
#include "alloc-util.h"
#include "bus-error.h"
#include "bus-internal.h"
#include "bus-track.h"
#include "string-util.h"
@ -11,6 +12,7 @@ struct track_item {
unsigned n_ref;
char *name;
sd_bus_slot *slot;
sd_bus_track *track;
};
struct sd_bus_track {
@ -163,18 +165,39 @@ static sd_bus_track *track_free(sd_bus_track *track) {
DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_bus_track, sd_bus_track, track_free);
static int on_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus_error *error) {
sd_bus_track *track = ASSERT_PTR(userdata);
static int on_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) {
struct track_item *item = ASSERT_PTR(userdata);
const char *name;
int r;
assert(message);
assert(item->track);
r = sd_bus_message_read(message, "sss", &name, NULL, NULL);
if (r < 0)
return 0;
bus_track_remove_name_fully(track, name);
bus_track_remove_name_fully(item->track, name);
return 0;
}
static int name_owner_changed_install_callback(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) {
struct track_item *item = ASSERT_PTR(userdata);
const sd_bus_error *e;
int r;
assert(message);
assert(item->track);
assert(item->name);
e = sd_bus_message_get_error(message);
if (!e)
return 0;
r = sd_bus_error_get_errno(e);
log_debug_errno(r, "Failed to install match for tracking name '%s': %s", item->name, bus_error_message(e, r));
bus_track_remove_name_fully(item->track, item->name);
return 0;
}
@ -216,6 +239,7 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) {
*n = (struct track_item) {
.n_ref = 1,
.track = track,
};
n->name = strdup(name);
@ -227,7 +251,7 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) {
bus_track_remove_from_queue(track); /* don't dispatch this while we work in it */
r = sd_bus_add_match_async(track->bus, &n->slot, match, on_name_owner_changed, NULL, track);
r = sd_bus_add_match_async(track->bus, &n->slot, match, on_name_owner_changed, name_owner_changed_install_callback, n);
if (r < 0) {
bus_track_add_to_queue(track);
return r;

View File

@ -698,6 +698,27 @@ int bus_track_add_name_many(sd_bus_track *t, char **l) {
return r;
}
int bus_track_to_strv(sd_bus_track *t, char ***ret) {
_cleanup_strv_free_ char **subscribed = NULL;
int r;
assert(ret);
for (const char *n = sd_bus_track_first(t); n; n = sd_bus_track_next(t)) {
int c = sd_bus_track_count_name(t, n);
assert(c >= 0);
for (int j = 0; j < c; j++) {
r = strv_extend(&subscribed, n);
if (r < 0)
return r;
}
}
*ret = TAKE_PTR(subscribed);
return 0;
}
int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *description) {
_cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL;
const char *e;

View File

@ -63,6 +63,7 @@ int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id,
int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external);
int bus_track_add_name_many(sd_bus_track *t, char **l);
int bus_track_to_strv(sd_bus_track *t, char ***ret);
int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *description);
static inline int bus_open_system_watch_bind(sd_bus **ret) {