]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
core: Add trigger limit for path units 21808/head
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Fri, 17 Dec 2021 19:01:31 +0000 (20:01 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Sat, 18 Dec 2021 10:26:25 +0000 (11:26 +0100)
When conditions fail on a service unit, a path unit can cause
PID 1 to busy loop as it keeps trying to activate the service unit.
To avoid this from happening, add a trigger limit to the path unit,
identical to the trigger limit we have for socket units.

Initially, let's start with a high limit and not make it configurable.
If needed, we can add properties to configure the rate limit similar
to the ones we have for socket units.

src/core/path.c
src/core/path.h
test/testsuite-63.units/test63.service
test/units/testsuite-63.service

index 1b0f115dd8b7ddda2f02caa3b513e1406e8b75d3..f89e35a001c8a37903bffe8125fb316f99f6fbd6 100644 (file)
@@ -265,6 +265,9 @@ static void path_init(Unit *u) {
         assert(u->load_state == UNIT_STUB);
 
         p->directory_mode = 0755;
+
+        p->trigger_limit.interval = 2 * USEC_PER_SEC;
+        p->trigger_limit.burst = 200;
 }
 
 void path_free_specs(Path *p) {
@@ -494,6 +497,12 @@ static void path_enter_running(Path *p) {
         if (unit_stop_pending(UNIT(p)))
                 return;
 
+        if (!ratelimit_below(&p->trigger_limit)) {
+                log_unit_warning(UNIT(p), "Trigger limit hit, refusing further activation.");
+                path_enter_dead(p, PATH_FAILURE_TRIGGER_LIMIT_HIT);
+                return;
+        }
+
         trigger = UNIT_TRIGGER(UNIT(p));
         if (!trigger) {
                 log_unit_error(UNIT(p), "Unit to trigger vanished.");
@@ -836,6 +845,7 @@ static const char* const path_result_table[_PATH_RESULT_MAX] = {
         [PATH_FAILURE_RESOURCES]            = "resources",
         [PATH_FAILURE_START_LIMIT_HIT]      = "start-limit-hit",
         [PATH_FAILURE_UNIT_START_LIMIT_HIT] = "unit-start-limit-hit",
+        [PATH_FAILURE_TRIGGER_LIMIT_HIT]    = "trigger-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);
index 66ae857dc40832305c09ff7dbfb1716aeb427a01..d835c241660b530974864c35340ce4b552e57532 100644 (file)
@@ -46,6 +46,7 @@ typedef enum PathResult {
         PATH_FAILURE_RESOURCES,
         PATH_FAILURE_START_LIMIT_HIT,
         PATH_FAILURE_UNIT_START_LIMIT_HIT,
+        PATH_FAILURE_TRIGGER_LIMIT_HIT,
         _PATH_RESULT_MAX,
         _PATH_RESULT_INVALID = -EINVAL,
 } PathResult;
@@ -61,6 +62,8 @@ struct Path {
         mode_t directory_mode;
 
         PathResult result;
+
+        RateLimit trigger_limit;
 };
 
 void path_free_specs(Path *p);
index 0253943f0ccdfe13c8e1fee82c181b8c69ee75f4..1a8721d82c70da920f69d9820fe998992e97b260 100644 (file)
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
 [Unit]
-ConditionPathExists=!/tmp/nonexistent
+ConditionPathExists=/tmp/nonexistent
 
 [Service]
 ExecStart=true
index 0a8d143be909a785fcb72a6878a36d7e53aee68c..40422127ff4d536563de211a0bb7df7749bf7c67 100644 (file)
@@ -5,13 +5,26 @@ Description=TEST-63-ISSUE-17433
 [Service]
 ExecStartPre=rm -f /failed /testok
 Type=oneshot
+
+# Test that a path unit continuously triggering a service that fails condition checks eventually fails with
+# the trigger-limit-hit error.
 ExecStart=rm -f /tmp/nonexistent
 ExecStart=systemctl start test63.path
 ExecStart=touch /tmp/test63
-# Make sure systemd has sufficient time to hit the start limit for test63.service.
+# Make sure systemd has sufficient time to hit the trigger limit for test63.path.
 ExecStart=sleep 2
-ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = failed'
-ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = start-limit-hit'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success'
 ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = failed'
-ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = unit-start-limit-hit'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = trigger-limit-hit'
+
+# Test that starting the service manually doesn't affect the path unit.
+ExecStart=rm -f /tmp/test63
+ExecStart=systemctl reset-failed
+ExecStart=systemctl start test63.path
+ExecStart=systemctl start test63.service
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = active'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = success'
 ExecStart=sh -x -c 'echo OK >/testok'