(RHEL-73780) Stash the subscriber list when we disconenct from the bus

src/core/dbus-cgroup.c

@@ -7,6 +7,7 @@
 #include "bpf-firewall.h"
 #include "bpf-foreign.h"
 #include "bus-get-properties.h"
+#include "bus-message-util.h"
 #include "bus-util.h"
 #include "cgroup-util.h"
 #include "cgroup.h"

src/core/dbus-manager.c

@@ -11,6 +11,7 @@
 #include "bus-common-errors.h"
 #include "bus-get-properties.h"
 #include "bus-log-control-api.h"
+#include "bus-message-util.h"
 #include "bus-util.h"
 #include "chase.h"
 #include "confidential-virt.h"

src/core/dbus.c

@@ -5,6 +5,7 @@
 #include <unistd.h>
 
 #include "sd-bus.h"
+#include "sd-id128.h"
 
 #include "alloc-util.h"
 #include "bus-common-errors.h"
@@ -774,6 +775,24 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void
         return 0;
 }
 
+static int bus_track_coldplug(sd_bus *bus, sd_bus_track **t, char * const *l) {
+        int r;
+
+        assert(bus);
+        assert(t);
+
+        if (strv_isempty(l))
+                return 0;
+
+        if (!*t) {
+                r = sd_bus_track_new(bus, t, NULL, NULL);
+                if (r < 0)
+                        return r;
+        }
+
+        return bus_track_add_name_many(*t, l);
+}
+
 static int bus_setup_api(Manager *m, sd_bus *bus) {
         char *name;
         Unit *u;
@@ -860,8 +879,15 @@ int bus_init_api(Manager *m) {
         if (r < 0)
                 return log_error_errno(r, "Failed to set up API bus: %m");
 
-        m->api_bus = TAKE_PTR(bus);
+        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;
 
+        m->api_bus = TAKE_PTR(bus);
         return 0;
 }
 
@@ -1004,8 +1030,20 @@ 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->deserialized_bus_id = m->bus_id;
+                m->bus_id = SD_ID128_NULL;
+
                 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)
@@ -1064,7 +1102,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);
 }
 
@@ -1153,31 +1190,6 @@ 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 r;
-
-        assert(m);
-        assert(t);
-
-        if (strv_isempty(l))
-                return 0;
-
-        if (!m->api_bus)
-                return 0;
-
-        if (!*t) {
-                r = sd_bus_track_new(m->api_bus, t, NULL, NULL);
-                if (r < 0)
-                        return r;
-        }
-
-        r = sd_bus_track_set_recursive(*t, recursive);
-        if (r < 0)
-                return r;
-
-        return bus_track_add_name_many(*t, l);
-}
-
 uint64_t manager_bus_n_queued_write(Manager *m) {
         uint64_t c = 0;
         sd_bus *b;

src/core/dbus.h

@@ -19,7 +19,6 @@ 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_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata);
 

src/core/manager-serialize.c

@@ -158,6 +158,7 @@ int manager_serialize(
         (void) serialize_ratelimit(f, "dump-ratelimit", &m->dump_ratelimit);
         (void) serialize_ratelimit(f, "reload-reexec-ratelimit", &m->reload_reexec_ratelimit);
 
+        (void) serialize_id128(f, "bus-id", m->bus_id);
         bus_track_serialize(m->subscribed, f, "subscribed");
 
         r = dynamic_user_serialize(m, f, fds);
@@ -491,9 +492,14 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         manager_deserialize_gid_refs_one(m, val);
                 else if ((val = startswith(l, "exec-runtime=")))
                         (void) exec_shared_runtime_deserialize_one(m, val, fds);
-                else if ((val = startswith(l, "subscribed="))) {
+                else if ((val = startswith(l, "bus-id="))) {
 
-                        r = strv_extend(&m->deserialized_subscribed, val);
+                        r = sd_id128_from_string(val, &m->deserialized_bus_id);
+                        if (r < 0)
+                                return r;
+                } else if ((val = startswith(l, "subscribed="))) {
+
+                        r = strv_extend(&m->subscribed_as_strv, val);
                         if (r < 0)
                                 return r;
                 } else if ((val = startswith(l, "varlink-server-socket-address="))) {

src/core/manager.c

@@ -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) {
@@ -1813,15 +1816,16 @@ Manager* manager_free(Manager *m) {
         hashmap_free(m->uid_refs);
         hashmap_free(m->gid_refs);
 
-        for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++)
-                m->prefix[dt] = mfree(m->prefix[dt]);
+        FOREACH_ARRAY(i, m->prefix, _EXEC_DIRECTORY_TYPE_MAX)
+                free(*i);
+
         free(m->received_credentials_directory);
         free(m->received_encrypted_credentials_directory);
 
         free(m->watchdog_pretimeout_governor);
         free(m->watchdog_pretimeout_governor_overridden);
 
-        m->fw_ctx = fw_ctx_free(m->fw_ctx);
+        fw_ctx_free(m->fw_ctx);
 
 #if BPF_FRAMEWORK
         bpf_restrict_fs_destroy(m->restrict_fs);
@@ -2138,12 +2142,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");
@@ -3807,15 +3805,17 @@ 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);
+        m->deserialized_bus_id = SD_ID128_NULL;
+
         /* 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--;

src/core/manager.h

@@ -335,12 +335,15 @@ struct Manager {
         int private_listen_fd;
         sd_event_source *private_listen_event_source;
 
-        /* Contains all the clients that are subscribed to signals via
-        the API bus. Note that private bus connections are always
-        considered subscribes, since they last for very short only,
-        and it is much simpler that way. */
+        /* Contains all the clients that are subscribed to signals via the API bus. Note that private bus
+         * connections are always 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;
+
+        /* The bus id of API bus acquired through org.freedesktop.DBus.GetId, which before deserializing
+         * subscriptions we'd use to verify the bus is still the same instance as before. */
+        sd_id128_t bus_id, deserialized_bus_id;
 
         /* This is used during reloading: before the reload we queue
          * the reply message here, and afterwards we send it */

src/core/unit-serialize.c

@@ -117,8 +117,7 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) {
         if (gid_is_valid(u->ref_gid))
                 (void) serialize_item_format(f, "ref-gid", GID_FMT, u->ref_gid);
 
-        if (!sd_id128_is_null(u->invocation_id))
-                (void) serialize_item_format(f, "invocation-id", SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(u->invocation_id));
+        (void) serialize_id128(f, "invocation-id", u->invocation_id);
 
         (void) serialize_item(f, "freezer-state", freezer_state_to_string(u->freezer_state));
 

src/home/homed-manager-bus.c

@@ -4,6 +4,7 @@
 
 #include "alloc-util.h"
 #include "bus-common-errors.h"
+#include "bus-message-util.h"
 #include "bus-polkit.h"
 #include "format-util.h"
 #include "home-util.h"
@@ -704,17 +705,17 @@ static int method_rebalance(sd_bus_message *message, void *userdata, sd_bus_erro
         int r;
 
         r = manager_schedule_rebalance(m, /* immediately= */ true);
-        if (r == 0)
-                return sd_bus_reply_method_errorf(message, BUS_ERROR_REBALANCE_NOT_NEEDED, "No home directories need rebalancing.");
         if (r < 0)
                 return r;
+        if (r == 0)
+                return sd_bus_reply_method_errorf(message, BUS_ERROR_REBALANCE_NOT_NEEDED, "No home directories need rebalancing.");
 
         /* Keep a reference to this message, so that we can reply to it once we are done */
-        r = set_ensure_put(&m->rebalance_queued_method_calls, &bus_message_hash_ops, message);
+        r = set_ensure_consume(&m->rebalance_queued_method_calls, &bus_message_hash_ops, sd_bus_message_ref(message));
         if (r < 0)
                 return log_error_errno(r, "Failed to track rebalance bus message: %m");
+        assert(r > 0);
 
-        sd_bus_message_ref(message);
         return 1;
 }
 

src/hostname/hostnamectl.c

@@ -18,6 +18,7 @@
 #include "bus-error.h"
 #include "bus-locator.h"
 #include "bus-map-properties.h"
+#include "bus-message-util.h"
 #include "format-table.h"
 #include "hostname-setup.h"
 #include "hostname-util.h"

src/login/logind-brightness.c

@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
+#include "bus-message-util.h"
 #include "bus-util.h"
 #include "device-util.h"
 #include "hash-funcs.h"
@@ -173,10 +174,9 @@ static int set_add_message(Set **set, sd_bus_message *message) {
         if (r <= 0)
                 return r;
 
-        r = set_ensure_put(set, &bus_message_hash_ops, message);
+        r = set_ensure_consume(set, &bus_message_hash_ops, sd_bus_message_ref(message));
         if (r <= 0)
                 return r;
-        sd_bus_message_ref(message);
 
         return 1;
 }

src/machine/machinectl.c

@@ -21,6 +21,7 @@
 #include "bus-error.h"
 #include "bus-locator.h"
 #include "bus-map-properties.h"
+#include "bus-message-util.h"
 #include "bus-print-properties.h"
 #include "bus-unit-procs.h"
 #include "bus-unit-util.h"

src/machine/machined-dbus.c

@@ -10,6 +10,7 @@
 #include "bus-common-errors.h"
 #include "bus-get-properties.h"
 #include "bus-locator.h"
+#include "bus-message-util.h"
 #include "bus-polkit.h"
 #include "cgroup-util.h"
 #include "discover-image.h"

src/run/run.c

@@ -15,6 +15,7 @@
 #include "bus-error.h"
 #include "bus-locator.h"
 #include "bus-map-properties.h"
+#include "bus-message-util.h"
 #include "bus-unit-util.h"
 #include "bus-wait-for-jobs.h"
 #include "calendarspec.h"

src/shared/bus-get-properties.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "bus-get-properties.h"
+#include "bus-message-util.h"
 #include "rlimit-util.h"
 #include "stdio-util.h"
 #include "string-util.h"
@@ -164,3 +165,21 @@ int bus_property_get_rlimit(
 
         return sd_bus_message_append(reply, "t", u);
 }
+
+int bus_property_get_string_set(
+                sd_bus *bus,
+                const char *path,
+                const char *interface,
+                const char *property,
+                sd_bus_message *reply,
+                void *userdata,
+                sd_bus_error *error) {
+
+        Set **s = ASSERT_PTR(userdata);
+
+        assert(bus);
+        assert(property);
+        assert(reply);
+
+        return bus_message_append_string_set(reply, *s);
+}

src/shared/bus-get-properties.h

@@ -52,6 +52,8 @@ assert_cc(sizeof(mode_t) == sizeof(uint32_t));
 
 int bus_property_get_rlimit(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error);
 
+int bus_property_get_string_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error);
+
 #define BUS_DEFINE_PROPERTY_GET_GLOBAL(function, bus_type, val)         \
         int function(sd_bus *bus,                                       \
                      const char *path,                                  \

src/shared/bus-map-properties.c

@@ -5,6 +5,7 @@
 #include "bus-util.h"
 #include "strv.h"
 #include "bus-message.h"
+#include "bus-message-util.h"
 
 int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
         sd_id128_t *p = userdata;