service: Don't stop unneeded units needed by restarted service (#7526)

An auto-restarted unit B may depend on unit A with StopWhenUnneeded=yes.
If A stops before B's restart timeout expires, it'll be started again as part
of B's dependent jobs. However, if stopping takes longer than the timeout, B's
running stop job collides start job which also cancels B's start job. Result is
that neither A or B are active.

Currently, when a service with automatic restarting fails, it transitions
through following states:
        1) SERVICE_FAILED or SERVICE_DEAD to indicate the failure,
        2) SERVICE_AUTO_RESTART while restart timer is running.

The StopWhenUnneeded= check takes place in service_enter_dead between the two
state mentioned above. We temporarily store the auto restart flag to query it
during the check. Because we don't return control to the main event loop, this
new service unit flag needn't be serialized.

This patch prevents the pathologic situation when the service with Restart=
won't restart automatically. As a side effect it also avoid restarting the
dependency unit with StopWhenUnneeded=yes.

Fixes: #7377

src/core/service.c

@@ -1493,9 +1493,13 @@ static bool service_shall_restart(Service *s) {
         }
 }
 
-static bool service_will_restart(Service *s) {
+static bool service_will_restart(Unit *u) {
+        Service *s = SERVICE(u);
+
         assert(s);
 
+        if (s->will_auto_restart)
+                return true;
         if (s->state == SERVICE_AUTO_RESTART)
                 return true;
         if (!UNIT(s)->job)
@@ -1521,13 +1525,17 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
         if (s->result != SERVICE_SUCCESS)
                 log_unit_warning(UNIT(s), "Failed with result '%s'.", service_result_to_string(s->result));
 
+        if (allow_restart && service_shall_restart(s))
+                s->will_auto_restart = true;
+
         /* Make sure service_release_resources() doesn't destroy our FD store, while we are changing through
          * SERVICE_FAILED/SERVICE_DEAD before entering into SERVICE_AUTO_RESTART. */
         s->n_keep_fd_store ++;
 
         service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : SERVICE_DEAD);
 
-        if (allow_restart && service_shall_restart(s)) {
+        if (s->will_auto_restart) {
+                s->will_auto_restart = false;
 
                 r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->restart_usec));
                 if (r < 0) {
@@ -1554,7 +1562,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
         s->exec_runtime = exec_runtime_unref(s->exec_runtime);
 
         if (s->exec_context.runtime_directory_preserve_mode == EXEC_PRESERVE_NO ||
-            (s->exec_context.runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART && !service_will_restart(s)))
+            (s->exec_context.runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART && !service_will_restart(UNIT(s))))
                 /* Also, remove the runtime directory */
                 exec_context_destroy_runtime_directory(&s->exec_context, UNIT(s)->manager->prefix[EXEC_DIRECTORY_RUNTIME]);
 
@@ -3772,6 +3780,8 @@ const UnitVTable service_vtable = {
         .active_state = service_active_state,
         .sub_state_to_string = service_sub_state_to_string,
 
+        .will_restart = service_will_restart,
+
         .check_gc = service_check_gc,
 
         .sigchld_event = service_sigchld_event,

src/core/service.h

@@ -164,6 +164,8 @@ struct Service {
         bool main_pid_alien:1;
         bool bus_name_good:1;
         bool forbid_restart:1;
+        /* Keep restart intention between UNIT_FAILED and UNIT_ACTIVATING */
+        bool will_auto_restart:1;
         bool start_timeout_defined:1;
 
         char *bus_name;

src/core/unit.c

@@ -2003,7 +2003,7 @@ static void unit_check_unneeded(Unit *u) {
                 void *v;
 
                 HASHMAP_FOREACH_KEY(v, other, u->dependencies[needed_dependencies[j]], i)
-                        if (unit_active_or_pending(other))
+                        if (unit_active_or_pending(other) || unit_will_restart(other))
                                 return;
         }
 
@@ -3794,6 +3794,15 @@ bool unit_active_or_pending(Unit *u) {
         return false;
 }
 
+bool unit_will_restart(Unit *u) {
+        assert(u);
+
+        if (!UNIT_VTABLE(u)->will_restart)
+                return false;
+
+        return UNIT_VTABLE(u)->will_restart(u);
+}
+
 int unit_kill(Unit *u, KillWho w, int signo, sd_bus_error *error) {
         assert(u);
         assert(w >= 0 && w < _KILL_WHO_MAX);

src/core/unit.h

@@ -484,6 +484,9 @@ struct UnitVTable {
          * unit is in. */
         const char* (*sub_state_to_string)(Unit *u);
 
+        /* Additionally to UnitActiveState determine whether unit is to be restarted. */
+        bool (*will_restart)(Unit *u);
+
         /* Return true when there is reason to keep this entry around
          * even nothing references it and it isn't active in any
          * way */
@@ -706,6 +709,7 @@ const char *unit_slice_name(Unit *u);
 bool unit_stop_pending(Unit *u) _pure_;
 bool unit_inactive_or_pending(Unit *u) _pure_;
 bool unit_active_or_pending(Unit *u);
+bool unit_will_restart(Unit *u);
 
 int unit_add_default_target_dependency(Unit *u, Unit *target);