From: Lennart Poettering Date: Wed, 7 Feb 2018 21:36:51 +0000 (+0100) Subject: core: delay bus name synchronization after reload/reexec into a later event loop... X-Git-Tag: v238~101^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5f109056d536849f6079df9321042cf4838a41d3;p=thirdparty%2Fsystemd.git core: delay bus name synchronization after reload/reexec into a later event loop iteration 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 87739f1b74d..5e6dd0dee56 100644 --- 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 fb4988dacec..3051ec06658 100644 --- 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 e83a0dd5e6e..3e3cc7ed807 100644 --- 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 b731e6e8f15..d4eaaa1c4bb 100644 --- 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;