From a5b07847950c603605acf85b472b210cd2da40fb Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 23 Dec 2019 16:48:18 +0100
Subject: [PATCH] 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.
---
 src/core/dbus.c | 11 ++++-------
 src/core/unit.c | 32 +++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/core/dbus.c b/src/core/dbus.c
index 3c40f29..cef1789 100644
--- 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 5cf16c6..8781132 100644
--- 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");
         }
 
-- 
1.8.3.1