core: reload cache if it's dirty when starting a UNIT_NOT_FOUND unit

The time-based cache allows starting a new unit without an expensive
daemon-reload, unless there was already a reference to it because of
a dependency or ordering from another unit.
If the cache is out of date, check again if we can load the
fragment.

src/core/manager.c

@@ -50,6 +50,7 @@
 #include "io-util.h"
 #include "label.h"
 #include "locale-setup.h"
+#include "load-fragment.h"
 #include "log.h"
 #include "macro.h"
 #include "manager.h"
@@ -1929,6 +1930,12 @@ unsigned manager_dispatch_load_queue(Manager *m) {
         return n;
 }
 
+static bool manager_unit_cache_needs_refresh(Manager *m) {
+        assert(m);
+
+        return m->unit_cache_mtime > 0 && !lookup_paths_mtime_good(&m->lookup_paths, m->unit_cache_mtime);
+}
+
 int manager_load_unit_prepare(
                 Manager *m,
                 const char *name,
@@ -1969,18 +1976,27 @@ int manager_load_unit_prepare(
 
         ret = manager_get_unit(m, name);
         if (ret) {
-                *_ret = ret;
-                return 1;
+                /* 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. */
+                if (ret->load_state == UNIT_NOT_FOUND && manager_unit_cache_needs_refresh(m))
+                        ret->load_state = UNIT_STUB;
+                else {
+                        *_ret = ret;
+                        return 1;
+                }
+        } else {
+                ret = cleanup_ret = unit_new(m, unit_vtable[t]->object_size);
+                if (!ret)
+                        return -ENOMEM;
         }
 
-        ret = cleanup_ret = unit_new(m, unit_vtable[t]->object_size);
-        if (!ret)
-                return -ENOMEM;
-
         if (path) {
-                ret->fragment_path = strdup(path);
-                if (!ret->fragment_path)
-                        return -ENOMEM;
+                r = free_and_strdup(&ret->fragment_path, path);
+                if (r < 0)
+                        return r;
         }
 
         r = unit_add_name(ret, name);

src/shared/unit-file.c

@@ -199,7 +199,7 @@ static bool lookup_paths_mtime_exclude(const LookupPaths *lp, const char *path)
                streq_ptr(path, lp->runtime_control);
 }
 
-static bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
+bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
         char **dir;
 
         STRV_FOREACH(dir, (char**) lp->search_path) {

src/shared/unit-file.h

@@ -43,6 +43,7 @@ bool unit_type_may_template(UnitType type) _const_;
 int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation);
 int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);
 
+bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime);
 int unit_file_build_name_map(
                 const LookupPaths *lp,
                 usec_t *ret_time,

test/TEST-48-START-STOP-NO-RELOAD/Makefile

@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile

test/TEST-48-START-STOP-NO-RELOAD/test.sh

@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -e
+TEST_DESCRIPTION="test StartStopNoReload"
+. $TEST_BASE_DIR/test-functions
+
+do_test "$@" 48

test/units/testsuite-48.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=TEST-48-START-STOP-NO-RELOAD
+
+[Service]
+ExecStartPre=rm -f /failed /testok
+ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh
+Type=oneshot

test/units/testsuite-48.sh

@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -ex
+
+cat > /run/systemd/system/testservice-48.target <<EOF
+[Unit]
+Wants=testservice-48.service
+EOF
+
+systemctl daemon-reload
+
+systemctl start testservice-48.target
+
+# The filesystem on the test image, despite being ext4, seems to have a mtime
+# granularity of one second, which means the manager's unit cache won't be
+# marked as dirty when writing the unit file, unless we wait at least a full
+# second after the previous daemon-reload.
+# May 07 23:12:20 systemd-testsuite testsuite-48.sh[30]: + cat
+# May 07 23:12:20 systemd-testsuite testsuite-48.sh[30]: + ls -l --full-time /etc/systemd/system/testservice-48.service
+# May 07 23:12:20 systemd-testsuite testsuite-48.sh[52]: -rw-r--r-- 1 root root 50 2020-05-07 23:12:20.000000000 +0100 /
+# May 07 23:12:20 systemd-testsuite testsuite-48.sh[30]: + stat -f --format=%t /etc/systemd/system/testservice-48.servic
+# May 07 23:12:20 systemd-testsuite testsuite-48.sh[53]: ef53
+sleep 1.1
+
+cat > /run/systemd/system/testservice-48.service <<EOF
+[Service]
+ExecStart=/bin/sleep infinity
+Type=exec
+EOF
+
+systemctl start testservice-48.service
+
+systemctl is-active testservice-48.service
+
+echo OK > /testok
+
+exit 0