core: refresh unit cache when building a transaction if UNIT_NOT_FOUND

When a command asks to load a unit directly and it is in state
UNIT_NOT_FOUND, and the cache is outdated, we refresh it and
attempto to load again.
Use the same logic when building up a transaction and a dependency in
UNIT_NOT_FOUND state is encountered.
Update the unit test to exercise this code path.

4 files changed, 59 insertions, 5 deletions

diff --git a/src/core/manager.c b/src/core/manager.c
index 7b199f5175..743ef6b4fc 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1932,11 +1932,19 @@ unsigned manager_dispatch_load_queue(Manager *m) {
         return n;
 }
 
-static bool manager_unit_cache_needs_refresh(Manager *m, Unit *u) {
-        assert(m);
+bool manager_unit_file_maybe_loadable_from_cache(Unit *u) {
+        assert(u);
+
+        if (u->load_state != UNIT_NOT_FOUND)
+                return false;
+
+        if (u->manager->unit_cache_mtime == 0)
+                return false;
+
+        if (u->manager->unit_cache_mtime > u->fragment_loadtime)
+                return true;
 
-        return m->unit_cache_mtime > 0 &&
-                (m->unit_cache_mtime > u->fragment_loadtime || !lookup_paths_mtime_good(&m->lookup_paths, m->unit_cache_mtime));
+        return !lookup_paths_mtime_good(&u->manager->lookup_paths, u->manager->unit_cache_mtime);
 }
 
 int manager_load_unit_prepare(
@@ -1988,7 +1996,7 @@ int manager_load_unit_prepare(
                  * we need to try again - even if the cache is current, it might have been
                  * updated in a different context before we had a chance to retry loading
                  * this particular unit. */
-                if (ret->load_state == UNIT_NOT_FOUND && manager_unit_cache_needs_refresh(m, ret))
+                if (manager_unit_file_maybe_loadable_from_cache(ret))
                         ret->load_state = UNIT_STUB;
                 else {
                         *_ret = ret;
diff --git a/src/core/manager.h b/src/core/manager.h
index 2cd0dacdb0..81b0c13a95 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -463,6 +463,7 @@ Unit *manager_get_unit(Manager *m, const char *name);
 
 int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j);
 
+bool manager_unit_file_maybe_loadable_from_cache(Unit *u);
 int manager_load_unit_prepare(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret);
 int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret);
 int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret);
diff --git a/src/core/transaction.c b/src/core/transaction.c
index ef6470656c..4a57b8e3f9 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -954,6 +954,24 @@ int transaction_add_job_and_dependencies(
 
         if (type != JOB_STOP) {
                 r = bus_unit_validate_load_state(unit, e);
+                /* The time-based cache allows to start new units without daemon-reload,
+                 * but if they are already referenced (because of dependencies or ordering)
+                 * then we have to force a load of the fragment. As an optimization, check
+                 * first if anything in the usual paths was modified since the last time
+                 * the cache was loaded. Also check if the last time an attempt to load the
+                 * unit was made was before the most recent cache refresh, so that we know
+                 * we need to try again - even if the cache is current, it might have been
+                 * updated in a different context before we had a chance to retry loading
+                 * this particular unit.
+                 * Given building up the transaction is a synchronous operation, attempt
+                 * to load the unit immediately. */
+                if (r < 0 && manager_unit_file_maybe_loadable_from_cache(unit)) {
+                        unit->load_state = UNIT_STUB;
+                        r = unit_load(unit);
+                        if (r < 0 || unit->load_state == UNIT_STUB)
+                                unit->load_state = UNIT_NOT_FOUND;
+                        r = bus_unit_validate_load_state(unit, e);
+                }
                 if (r < 0)
                         return r;
         }
diff --git a/test/units/testsuite-48.sh b/test/units/testsuite-48.sh
index 93e5e98e42..03231e71b1 100755
--- a/test/units/testsuite-48.sh
+++ b/test/units/testsuite-48.sh
@@ -53,6 +53,33 @@ systemctl start testservice-48.service
 
 systemctl is-active testservice-48.service
 
+# Stop and remove, and try again to exercise the transaction setup code path by
+# having the target pull in the unloaded but available unit
+systemctl stop testservice-48.service testservice-48.target
+rm -f /run/systemd/system/testservice-48.service /run/systemd/system/testservice-48.target
+systemctl daemon-reload
+
+sleep 3.1
+
+cat > /run/systemd/system/testservice-48.target <<EOF
+[Unit]
+Conflicts=shutdown.target
+Wants=testservice-48.service
+EOF
+
+systemctl daemon-reload
+
+systemctl start testservice-48.target
+
+cat > /run/systemd/system/testservice-48.service <<EOF
+[Service]
+ExecStart=/bin/sleep infinity
+EOF
+
+systemctl restart testservice-48.target
+
+systemctl is-active testservice-48.service
+
 echo OK > /testok
 
 exit 0