core: create/remove unit bus name slots always together

When a service unit watches a bus name (i.e. because of BusName= being
set), then we do two things: we install a match slot to watch how its
ownership changes, and we inquire about the current owner. Make sure we
always do both together or neither.

This in particular fixes a corner-case memleak when destroying bus
connections, since we never freed the GetNameOwner() bus slots when
destroying a bus when they were still ongoing.

diff --git a/src/core/dbus.c b/src/core/dbus.c
index 3c40f29694401e4525c07b9fd8636f8209eeb6e7..cef17894cd022c6f4ee8fed2dc029c1723cf5385 100644 (file)
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -1051,13 +1051,10 @@ static void destroy_bus(Manager *m, sd_bus **bus) {
 
         /* Make sure all bus slots watching names are released. */
         HASHMAP_FOREACH(u, m->watch_bus, i) {
-                if (!u->match_bus_slot)
-                        continue;
-
-                if (sd_bus_slot_get_bus(u->match_bus_slot) != *bus)
-                        continue;
-
-                u->match_bus_slot = sd_bus_slot_unref(u->match_bus_slot);
+                if (u->match_bus_slot && sd_bus_slot_get_bus(u->match_bus_slot) == *bus)
+                        u->match_bus_slot = sd_bus_slot_unref(u->match_bus_slot);
+                if (u->get_name_owner_slot && sd_bus_slot_get_bus(u->get_name_owner_slot) == *bus)
+                        u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot);
         }
 
         /* Get rid of tracked clients on this bus */

diff --git a/src/core/unit.c b/src/core/unit.c
index 5cf16c6a79589b65a8cc3fadcba6334eac960edc..8781132a7ae5499eddd456ab7e8d2641baec513c 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -3238,12 +3238,13 @@ static int get_name_owner_handler(sd_bus_message *message, void *userdata, sd_bu
 
 int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) {
         const char *match;
+        int r;
 
         assert(u);
         assert(bus);
         assert(name);
 
-        if (u->match_bus_slot)
+        if (u->match_bus_slot || u->get_name_owner_slot)
                 return -EBUSY;
 
         match = strjoina("type='signal',"
@@ -3253,19 +3254,27 @@ int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) {
                          "member='NameOwnerChanged',"
                          "arg0='", name, "'");
 
-        int r = sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u);
+        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);
+        r = 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);
+        if (r < 0) {
+                u->match_bus_slot = sd_bus_slot_unref(u->match_bus_slot);
+                return r;
+        }
+
+        log_unit_debug(u, "Watching D-Bus name '%s'.", name);
+        return 0;
 }
 
 int unit_watch_bus_name(Unit *u, const char *name) {
@@ -3288,6 +3297,7 @@ int unit_watch_bus_name(Unit *u, const char *name) {
         r = hashmap_put(u->manager->watch_bus, name, u);
         if (r < 0) {
                 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);
                 return log_warning_errno(r, "Failed to put bus name to hashmap: %m");
         }