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 6653dbde84b79acbf12036b0d78ff926eb6e371f..9a75abb9e9aeebf391eeb12316dde38fcb1ed1cc 100644 (file)
--- 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 (file)
index 0000000..93ddb85
--- /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 (file)
index 0000000..c76fed2
--- /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 7c5a3b8f193a22203a4c06d56339c99c8544f8f9..4b3b409760580b8ad4efe7220f9add61ab03d895 100755 (executable)
--- 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