From: Andrew Stone <a@stne.dev>
Date: Mon, 22 Aug 2022 22:03:42 +0000 (-0700)
Subject: job: Don't discard propagated restart jobs when unit is activating
X-Git-Tag: v252-rc1~256^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dc06321fe3e44915b732248bbf93d01b2bb7819a;p=thirdparty%2Fsystemd.git

job: Don't discard propagated restart jobs when unit is activating

When a service unit Requires= a socket, and the socket is restarted while the
service is in state=activating, the propagated restart is being discarded. This
is contrary to the documentation for Requires=, which states "this unit will be
stopped (or restarted) if one of the other units is explicitly stopped (or
restarted)".
---

diff --git a/src/core/job.c b/src/core/job.c
index 6653dbde84b..9a75abb9e9a 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -400,8 +400,22 @@ bool job_type_is_redundant(JobType a, UnitActiveState b) {
                         b == UNIT_RELOADING;
 
         case JOB_RESTART:
-                return
-                        b == UNIT_ACTIVATING;
+                /* Restart jobs must always be kept.
+                 *
+                 * For ACTIVE/RELOADING units, this is obvious.
+                 *
+                 * For ACTIVATING units, it's more subtle:
+                 *
+                 * Generally, if a service Requires= another unit, restarts of
+                 * the unit must be propagated to the service. If the service is
+                 * ACTIVATING, it must still be restarted since it might have
+                 * stale information regarding the other unit.
+                 *
+                 * For example, consider a service that Requires= a socket: if
+                 * the socket is restarted, but the service is still ACTIVATING,
+                 * it's necessary to restart the service so that it gets the new
+                 * socket. */
+                return false;
 
         case JOB_NOP:
                 return true;
@@ -417,8 +431,12 @@ JobType job_type_collapse(JobType t, Unit *u) {
         switch (t) {
 
         case JOB_TRY_RESTART:
+                /* Be sure to keep the restart job even if the unit is
+                 * ACTIVATING.
+                 *
+                 * See the job_type_is_redundant(JOB_RESTART) for more info */
                 s = unit_active_state(u);
-                if (!UNIT_IS_ACTIVE_OR_RELOADING(s))
+                if (!UNIT_IS_ACTIVE_OR_ACTIVATING(s))
                         return JOB_NOP;
 
                 return JOB_RESTART;
diff --git a/test/testsuite-03.units/always-activating.service b/test/testsuite-03.units/always-activating.service
new file mode 100644
index 00000000000..93ddb85c75f
--- /dev/null
+++ b/test/testsuite-03.units/always-activating.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Service that never leaves state ACTIVATING
+Requires=always-activating.socket
+After=always-activating.socket
+
+[Service]
+Type=notify
+ExecStart=bash -c 'sleep infinity'
diff --git a/test/testsuite-03.units/always-activating.socket b/test/testsuite-03.units/always-activating.socket
new file mode 100644
index 00000000000..c76fed2ca8d
--- /dev/null
+++ b/test/testsuite-03.units/always-activating.socket
@@ -0,0 +1,5 @@
+[Unit]
+Description=Socket that activates always-activating.service
+
+[Socket]
+ListenStream=/run/test-03-always-activating.sock
diff --git a/test/units/testsuite-03.sh b/test/units/testsuite-03.sh
index 7c5a3b8f193..4b3b4097605 100755
--- a/test/units/testsuite-03.sh
+++ b/test/units/testsuite-03.sh
@@ -50,6 +50,15 @@ systemctl stop hello.service sleep.service hello-after-sleep.target
 
 # TODO: add more job queueing/merging tests here.
 
+# Test that restart propagates to activating units
+systemctl -T --no-block start always-activating.service
+systemctl list-jobs | grep 'always-activating.service'
+ACTIVATING_ID_PRE=$(systemctl show -P InvocationID always-activating.service)
+systemctl -T start always-activating.socket # Wait for the socket to come up
+systemctl -T restart always-activating.socket
+ACTIVATING_ID_POST=$(systemctl show -P InvocationID always-activating.service)
+[ "$ACTIVATING_ID_PRE" != "$ACTIVATING_ID_POST" ] || exit 1
+
 # Test for irreversible jobs
 systemctl start unstoppable.service