core: Avoid race when starting dbus services

In high load scenarios it is possible for services to be started
before the NameOwnerChanged signal is properly installed.

Emulate a callback by also queuing a GetNameOwner when the match is
installed.

Fixes: #12956

diff --git a/src/core/dbus.c b/src/core/dbus.c
index a8ce9ac44793c5720ddc1875f7842eb30048b299..bbfad1be745c7372f1ebd677227eb6fa3b9d8565 100644 (file)
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -784,7 +784,7 @@ static int manager_dispatch_sync_bus_names(sd_event_source *es, void *userdata)
                          * changed, so synthesize a name owner changed signal. */
 
                         if (!streq_ptr(unique, s->bus_name_owner))
-                                UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, unique);
+                                UNIT_VTABLE(u)->bus_name_owner_change(u, s->bus_name_owner, unique);
                 } else {
                         /* So, the name we're watching is not on the bus.
                          * This either means it simply hasn't appeared yet,
@@ -793,7 +793,7 @@ static int manager_dispatch_sync_bus_names(sd_event_source *es, void *userdata)
                          * and synthesize a name loss signal in this case. */
 
                         if (s->bus_name_owner)
-                                UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, NULL);
+                                UNIT_VTABLE(u)->bus_name_owner_change(u, s->bus_name_owner, NULL);
                 }
         }
 

diff --git a/src/core/service.c b/src/core/service.c
index bfbfa4be655f57ca04a2afc63ee7fcdddf284590..d1893228ece4f35d3b02611219814a9c3099a6e9 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -4067,7 +4067,6 @@ static int service_get_timeout(Unit *u, usec_t *timeout) {
 
 static void service_bus_name_owner_change(
                 Unit *u,
-                const char *name,
                 const char *old_owner,
                 const char *new_owner) {
 
@@ -4075,17 +4074,15 @@ static void service_bus_name_owner_change(
         int r;
 
         assert(s);
-        assert(name);
 
-        assert(streq(s->bus_name, name));
         assert(old_owner || new_owner);
 
         if (old_owner && new_owner)
-                log_unit_debug(u, "D-Bus name %s changed owner from %s to %s", name, old_owner, new_owner);
+                log_unit_debug(u, "D-Bus name %s changed owner from %s to %s", s->bus_name, old_owner, new_owner);
         else if (old_owner)
-                log_unit_debug(u, "D-Bus name %s no longer registered by %s", name, old_owner);
+                log_unit_debug(u, "D-Bus name %s no longer registered by %s", s->bus_name, old_owner);
         else
-                log_unit_debug(u, "D-Bus name %s now registered by %s", name, new_owner);
+                log_unit_debug(u, "D-Bus name %s now registered by %s", s->bus_name, new_owner);
 
         s->bus_name_good = !!new_owner;
 
@@ -4118,11 +4115,11 @@ static void service_bus_name_owner_change(
 
                 /* Try to acquire PID from bus service */
 
-                r = sd_bus_get_name_creds(u->manager->api_bus, name, SD_BUS_CREDS_PID, &creds);
+                r = sd_bus_get_name_creds(u->manager->api_bus, s->bus_name, SD_BUS_CREDS_PID, &creds);
                 if (r >= 0)
                         r = sd_bus_creds_get_pid(creds, &pid);
                 if (r >= 0) {
-                        log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, name, pid);
+                        log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, pid);
 
                         service_set_main_pid(s, pid);
                         unit_watch_pid(UNIT(s), pid, false);

diff --git a/src/core/unit.c b/src/core/unit.c
index 00181941fd9be13ffd49ecae1f62bfb483685ebb..9dc75223a72f025bec318462a8e287575081e589 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -3242,7 +3242,46 @@ static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd
         new_owner = empty_to_null(new_owner);
 
         if (UNIT_VTABLE(u)->bus_name_owner_change)
-                UNIT_VTABLE(u)->bus_name_owner_change(u, name, old_owner, new_owner);
+                UNIT_VTABLE(u)->bus_name_owner_change(u, old_owner, new_owner);
+
+        return 0;
+}
+
+static int get_name_owner_handler(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        const sd_bus_error *e;
+        const char *new_owner;
+        Unit *u = userdata;
+        int r;
+
+        assert(message);
+        assert(u);
+
+        u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot);
+
+        if (sd_bus_error_is_set(error)) {
+                log_error("Failed to get name owner from bus: %s", error->message);
+                return 0;
+        }
+
+        e = sd_bus_message_get_error(message);
+        if (sd_bus_error_has_name(e, "org.freedesktop.DBus.Error.NameHasNoOwner"))
+                return 0;
+
+        if (e) {
+                log_error("Unexpected error response from GetNameOwner: %s", e->message);
+                return 0;
+        }
+
+        r = sd_bus_message_read(message, "s", &new_owner);
+        if (r < 0) {
+                bus_log_parse_error(r);
+                return 0;
+        }
+
+        new_owner = empty_to_null(new_owner);
+
+        if (UNIT_VTABLE(u)->bus_name_owner_change)
+                UNIT_VTABLE(u)->bus_name_owner_change(u, NULL, new_owner);
 
         return 0;
 }
@@ -3264,7 +3303,19 @@ int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) {
                          "member='NameOwnerChanged',"
                          "arg0='", name, "'");
 
-        return sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u);
+        int r = sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u);
+        if (r < 0)
+                return r;
+
+        return sd_bus_call_method_async(bus,
+                                        &u->get_name_owner_slot,
+                                        "org.freedesktop.DBus",
+                                        "/org/freedesktop/DBus",
+                                        "org.freedesktop.DBus",
+                                        "GetNameOwner",
+                                        get_name_owner_handler,
+                                        u,
+                                        "s", name);
 }
 
 int unit_watch_bus_name(Unit *u, const char *name) {
@@ -3299,6 +3350,7 @@ void unit_unwatch_bus_name(Unit *u, const char *name) {
 
         (void) hashmap_remove_value(u->manager->watch_bus, name, u);
         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);
 }
 
 bool unit_can_serialize(Unit *u) {

diff --git a/src/core/unit.h b/src/core/unit.h
index 47ec9877a67ad042555017abfee252332b33d8f2..9ad86fddf6bbffa8be997c43257461c814567f36 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -146,6 +146,7 @@ typedef struct Unit {
 
         /* The slot used for watching NameOwnerChanged signals */
         sd_bus_slot *match_bus_slot;
+        sd_bus_slot *get_name_owner_slot;
 
         /* References to this unit from clients */
         sd_bus_track *bus_track;
@@ -528,7 +529,7 @@ typedef struct UnitVTable {
         void (*notify_message)(Unit *u, const struct ucred *ucred, char **tags, FDSet *fds);
 
         /* Called whenever a name this Unit registered for comes or goes away. */
-        void (*bus_name_owner_change)(Unit *u, const char *name, const char *old_owner, const char *new_owner);
+        void (*bus_name_owner_change)(Unit *u, const char *old_owner, const char *new_owner);
 
         /* Called for each property that is being set */
         int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);