daandemeyer / rpms / systemd

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