core: delay bus name synchronization after reload/reexec into a later event loop...

Previously, we'd synchronize bus names immediately when we succeeded
connecting to the bus, potentially even before coldplugging the units.
This was problematic, as synchronizing bus names meant invoking the
per-unit name change handler function which might change the unit's
state — which will result in consistency when done before we coldplug
things.

With this change we instead enqueue a job for the event loop to resync
the names in a later loop iteration, i.e. at a point where we know
coldplugging has finished.

diff --git a/src/core/dbus.c b/src/core/dbus.c
index 87739f1..5e6dd0d 100644 (file)
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -727,17 +727,29 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void
         return 0;
 }
 
-int manager_sync_bus_names(Manager *m, sd_bus *bus) {
+static int manager_dispatch_sync_bus_names(sd_event_source *es, void *userdata) {
         _cleanup_strv_free_ char **names = NULL;
+        Manager *m = userdata;
         const char *name;
         Iterator i;
         Unit *u;
         int r;
 
+        assert(es);
         assert(m);
-        assert(bus);
+        assert(m->sync_bus_names_event_source == es);
+
+        /* First things first, destroy the defer event so that we aren't triggered again */
+        m->sync_bus_names_event_source = sd_event_source_unref(m->sync_bus_names_event_source);
+
+        /* Let's see if there's anything to do still? */
+        if (!m->api_bus)
+                return 0;
+        if (hashmap_isempty(m->watch_bus))
+                return 0;
 
-        r = sd_bus_list_names(bus, &names, NULL);
+        /* OK, let's sync up the names. Let's see which names are currently on the bus. */
+        r = sd_bus_list_names(m->api_bus, &names, NULL);
         if (r < 0)
                 return log_error_errno(r, "Failed to get initial list of names: %m");
 
@@ -761,7 +773,7 @@ int manager_sync_bus_names(Manager *m, sd_bus *bus) {
                         const char *unique;
 
                         /* If it is, determine its current owner */
-                        r = sd_bus_get_name_creds(bus, name, SD_BUS_CREDS_UNIQUE_NAME, &creds);
+                        r = sd_bus_get_name_creds(m->api_bus, name, SD_BUS_CREDS_UNIQUE_NAME, &creds);
                         if (r < 0) {
                                 log_full_errno(r == -ENXIO ? LOG_DEBUG : LOG_ERR, r, "Failed to get bus name owner %s: %m", name);
                                 continue;
@@ -795,6 +807,34 @@ int manager_sync_bus_names(Manager *m, sd_bus *bus) {
         return 0;
 }
 
+int manager_enqueue_sync_bus_names(Manager *m) {
+        int r;
+
+        assert(m);
+
+        /* Enqueues a request to synchronize the bus names in a later event loop iteration. The callers generally don't
+         * want us to invoke ->bus_name_owner_change() unit calls from their stack frames as this might result in event
+         * dispatching on its own creating loops, hence we simply create a defer event for the event loop and exit. */
+
+        if (m->sync_bus_names_event_source)
+                return 0;
+
+        r = sd_event_add_defer(m->event, &m->sync_bus_names_event_source, manager_dispatch_sync_bus_names, m);
+        if (r < 0)
+                return log_error_errno(r, "Failed to create bus name synchronization event: %m");
+
+        r = sd_event_source_set_priority(m->sync_bus_names_event_source, SD_EVENT_PRIORITY_IDLE);
+        if (r < 0)
+                return log_error_errno(r, "Failed to set event priority: %m");
+
+        r = sd_event_source_set_enabled(m->sync_bus_names_event_source, SD_EVENT_ONESHOT);
+        if (r < 0)
+                return log_error_errno(r, "Failed to set even to oneshot: %m");
+
+        (void) sd_event_source_set_description(m->sync_bus_names_event_source, "manager-sync-bus-names");
+        return 0;
+}
+
 static int bus_setup_api(Manager *m, sd_bus *bus) {
         Iterator i;
         char *name;
@@ -840,11 +880,8 @@ static int bus_setup_api(Manager *m, sd_bus *bus) {
         if (r < 0)
                 return log_error_errno(r, "Failed to request name: %m");
 
-        r = manager_sync_bus_names(m, bus);
-        if (r < 0)
-                return r;
-
         log_debug("Successfully connected to API bus.");
+
         return 0;
 }
 
@@ -882,6 +919,10 @@ int bus_init_api(Manager *m) {
         m->api_bus = bus;
         bus = NULL;
 
+        r = manager_enqueue_sync_bus_names(m);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 

diff --git a/src/core/dbus.h b/src/core/dbus.h
index fb4988d..3051ec0 100644 (file)
--- a/src/core/dbus.h
+++ b/src/core/dbus.h
@@ -38,7 +38,7 @@ 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 manager_sync_bus_names(Manager *m, sd_bus *bus);
+int manager_enqueue_sync_bus_names(Manager *m);
 
 int bus_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata);
 

diff --git a/src/core/manager.c b/src/core/manager.c
index e83a0dd..3e3cc7e 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1203,6 +1203,7 @@ Manager* manager_free(Manager *m) {
         sd_event_source_unref(m->jobs_in_progress_event_source);
         sd_event_source_unref(m->run_queue_event_source);
         sd_event_source_unref(m->user_lookup_event_source);
+        sd_event_source_unref(m->sync_bus_names_event_source);
 
         safe_close(m->signal_fd);
         safe_close(m->notify_fd);
@@ -3182,8 +3183,9 @@ int manager_reload(Manager *m) {
         manager_recheck_dbus(m);
 
         /* Sync current state of bus names with our set of listening units */
-        if (m->api_bus)
-                manager_sync_bus_names(m, m->api_bus);
+        q = manager_enqueue_sync_bus_names(m);
+        if (q < 0 && r >= 0)
+                r = q;
 
         assert(m->n_reloading > 0);
         m->n_reloading--;

diff --git a/src/core/manager.h b/src/core/manager.h
index b731e6e..d4eaaa1 100644 (file)
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -182,6 +182,8 @@ struct Manager {
         int user_lookup_fds[2];
         sd_event_source *user_lookup_event_source;
 
+        sd_event_source *sync_bus_names_event_source;
+
         UnitFileScope unit_file_scope;
         LookupPaths lookup_paths;
         Set *unit_path_cache;