daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
aed857
From 36226a9afe96bdffce9d0697be020f2ca9d7fe6f Mon Sep 17 00:00:00 2001
aed857
From: Michal Sekletar <msekletar@users.noreply.github.com>
aed857
Date: Fri, 23 Mar 2018 15:28:06 +0100
aed857
Subject: [PATCH] core: delay adding target dependencies until all units are
aed857
 loaded and aliases resolved (#8381)
aed857
aed857
Currently we add target dependencies while we are loading units. This
aed857
can create ordering loops even if configuration doesn't contain any
aed857
loop. Take for example following configuration,
aed857
aed857
$ systemctl get-default
aed857
multi-user.target
aed857
aed857
$ cat /etc/systemd/system/test.service
aed857
[Unit]
aed857
After=default.target
aed857
aed857
[Service]
aed857
ExecStart=/bin/true
aed857
aed857
[Install]
aed857
WantedBy=multi-user.target
aed857
aed857
If we encounter such unit file early during manager start-up (e.g. load
aed857
queue is dispatched while enumerating devices due to SYSTEMD_WANTS in
aed857
udev rules) we would add stub unit default.target and we order it Before
aed857
test.service. At the same time we add implicit Before to
aed857
multi-user.target. Later we merge two units and we create ordering cycle
aed857
in the process.
aed857
aed857
To fix the issue we will now never add any target dependencies until we
aed857
loaded all the unit files and resolved all the aliases.
aed857
aed857
(cherry picked from commit 19496554e23ea4861ce780430052dcf86a2ffcba)
aed857
aed857
Resolves: #1368856
aed857
---
aed857
 src/core/manager.c | 40 ++++++++++++++++++++++++++++++++++++++++
aed857
 src/core/manager.h |  3 +++
aed857
 src/core/unit.c    | 46 ++++++++++++++++------------------------------
aed857
 src/core/unit.h    |  5 +++++
aed857
 4 files changed, 64 insertions(+), 30 deletions(-)
aed857
aed857
diff --git a/src/core/manager.c b/src/core/manager.c
aed857
index 9c406bb5b..0466e4bb8 100644
aed857
--- a/src/core/manager.c
aed857
+++ b/src/core/manager.c
aed857
@@ -1373,6 +1373,41 @@ Unit *manager_get_unit(Manager *m, const char *name) {
aed857
         return hashmap_get(m->units, name);
aed857
 }
aed857
 
aed857
+static int manager_dispatch_target_deps_queue(Manager *m) {
aed857
+        Unit *u;
aed857
+        unsigned k;
aed857
+        int r = 0;
aed857
+
aed857
+        static const UnitDependency deps[] = {
aed857
+                UNIT_REQUIRED_BY,
aed857
+                UNIT_REQUIRED_BY_OVERRIDABLE,
aed857
+                UNIT_WANTED_BY,
aed857
+                UNIT_BOUND_BY
aed857
+        };
aed857
+
aed857
+        assert(m);
aed857
+
aed857
+        while ((u = m->target_deps_queue)) {
aed857
+                assert(u->in_target_deps_queue);
aed857
+
aed857
+                LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
aed857
+                u->in_target_deps_queue = false;
aed857
+
aed857
+                for (k = 0; k < ELEMENTSOF(deps); k++) {
aed857
+                        Unit *target;
aed857
+                        Iterator i;
aed857
+
aed857
+                        SET_FOREACH(target, u->dependencies[deps[k]], i) {
aed857
+                                r = unit_add_default_target_dependency(u, target);
aed857
+                                if (r < 0)
aed857
+                                        return r;
aed857
+                        }
aed857
+                }
aed857
+        }
aed857
+
aed857
+        return r;
aed857
+}
aed857
+
aed857
 unsigned manager_dispatch_load_queue(Manager *m) {
aed857
         Unit *u;
aed857
         unsigned n = 0;
aed857
@@ -1396,6 +1431,11 @@ unsigned manager_dispatch_load_queue(Manager *m) {
aed857
         }
aed857
 
aed857
         m->dispatching_load_queue = false;
aed857
+
aed857
+        /* Dispatch the units waiting for their target dependencies to be added now, as all targets that we know about
aed857
+         * should be loaded and have aliases resolved */
aed857
+        (void) manager_dispatch_target_deps_queue(m);
aed857
+
aed857
         return n;
aed857
 }
aed857
 
aed857
diff --git a/src/core/manager.h b/src/core/manager.h
aed857
index 90d2d982e..b0e4cad1f 100644
aed857
--- a/src/core/manager.h
aed857
+++ b/src/core/manager.h
aed857
@@ -114,6 +114,9 @@ struct Manager {
aed857
         /* Units that should be realized */
aed857
         LIST_HEAD(Unit, cgroup_queue);
aed857
 
aed857
+        /* Target units whose default target dependencies haven't been set yet */
aed857
+        LIST_HEAD(Unit, target_deps_queue);
aed857
+
aed857
         sd_event *event;
aed857
 
aed857
         /* We use two hash tables here, since the same PID might be
aed857
diff --git a/src/core/unit.c b/src/core/unit.c
aed857
index 22d9beed7..cfddce34d 100644
aed857
--- a/src/core/unit.c
aed857
+++ b/src/core/unit.c
aed857
@@ -519,6 +519,9 @@ void unit_free(Unit *u) {
aed857
                 u->manager->n_in_gc_queue--;
aed857
         }
aed857
 
aed857
+        if (u->in_target_deps_queue)
aed857
+                LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
aed857
+
aed857
         if (u->in_cgroup_queue)
aed857
                 LIST_REMOVE(cgroup_queue, u->manager->cgroup_queue, u);
aed857
 
aed857
@@ -1065,6 +1068,18 @@ int unit_load_fragment_and_dropin_optional(Unit *u) {
aed857
         return 0;
aed857
 }
aed857
 
aed857
+void unit_add_to_target_deps_queue(Unit *u) {
aed857
+        Manager *m = u->manager;
aed857
+
aed857
+        assert(u);
aed857
+
aed857
+        if (u->in_target_deps_queue)
aed857
+                return;
aed857
+
aed857
+        LIST_PREPEND(target_deps_queue, m->target_deps_queue, u);
aed857
+        u->in_target_deps_queue = true;
aed857
+}
aed857
+
aed857
 int unit_add_default_target_dependency(Unit *u, Unit *target) {
aed857
         assert(u);
aed857
         assert(target);
aed857
@@ -1091,32 +1106,6 @@ int unit_add_default_target_dependency(Unit *u, Unit *target) {
aed857
         return unit_add_dependency(target, UNIT_AFTER, u, true);
aed857
 }
aed857
 
aed857
-static int unit_add_target_dependencies(Unit *u) {
aed857
-
aed857
-        static const UnitDependency deps[] = {
aed857
-                UNIT_REQUIRED_BY,
aed857
-                UNIT_REQUIRED_BY_OVERRIDABLE,
aed857
-                UNIT_WANTED_BY,
aed857
-                UNIT_BOUND_BY
aed857
-        };
aed857
-
aed857
-        Unit *target;
aed857
-        Iterator i;
aed857
-        unsigned k;
aed857
-        int r = 0;
aed857
-
aed857
-        assert(u);
aed857
-
aed857
-        for (k = 0; k < ELEMENTSOF(deps); k++)
aed857
-                SET_FOREACH(target, u->dependencies[deps[k]], i) {
aed857
-                        r = unit_add_default_target_dependency(u, target);
aed857
-                        if (r < 0)
aed857
-                                return r;
aed857
-                }
aed857
-
aed857
-        return r;
aed857
-}
aed857
-
aed857
 static int unit_add_slice_dependencies(Unit *u) {
aed857
         assert(u);
aed857
 
aed857
@@ -1217,10 +1206,7 @@ int unit_load(Unit *u) {
aed857
         }
aed857
 
aed857
         if (u->load_state == UNIT_LOADED) {
aed857
-
aed857
-                r = unit_add_target_dependencies(u);
aed857
-                if (r < 0)
aed857
-                        goto fail;
aed857
+                unit_add_to_target_deps_queue(u);
aed857
 
aed857
                 r = unit_add_slice_dependencies(u);
aed857
                 if (r < 0)
aed857
diff --git a/src/core/unit.h b/src/core/unit.h
aed857
index 480e2e95f..dfec9cea0 100644
aed857
--- a/src/core/unit.h
aed857
+++ b/src/core/unit.h
aed857
@@ -162,6 +162,9 @@ struct Unit {
aed857
         /* CGroup realize members queue */
aed857
         LIST_FIELDS(Unit, cgroup_queue);
aed857
 
aed857
+        /* Target dependencies queue */
aed857
+        LIST_FIELDS(Unit, target_deps_queue);
aed857
+
aed857
         /* PIDs we keep an eye on. Note that a unit might have many
aed857
          * more, but these are the ones we care enough about to
aed857
          * process SIGCHLD for */
aed857
@@ -228,6 +231,7 @@ struct Unit {
aed857
         bool in_cleanup_queue:1;
aed857
         bool in_gc_queue:1;
aed857
         bool in_cgroup_queue:1;
aed857
+        bool in_target_deps_queue:1;
aed857
 
aed857
         bool sent_dbus_new_signal:1;
aed857
 
aed857
@@ -498,6 +502,7 @@ void unit_add_to_load_queue(Unit *u);
aed857
 void unit_add_to_dbus_queue(Unit *u);
aed857
 void unit_add_to_cleanup_queue(Unit *u);
aed857
 void unit_add_to_gc_queue(Unit *u);
aed857
+void unit_add_to_target_deps_queue(Unit *u);
aed857
 
aed857
 int unit_merge(Unit *u, Unit *other);
aed857
 int unit_merge_by_name(Unit *u, const char *other);