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

diff --git a/src/core/service.c b/src/core/service.c
index e7a829a564f79da24fb68b73d06b61d1425ee0d8..97f129b8f02b51f58e22ee9fabfa987829df437f 100644 (file)
--- a/src/core/service.c
+++ b/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,

diff --git a/src/core/service.h b/src/core/service.h
index dfe27e894fa2a4c75a15dd272cd66ac9f3437ba9..5c5b24e3efdb6cbd4257e4b9f95981805ba32afa 100644 (file)
--- a/src/core/service.h
+++ b/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;

diff --git a/src/core/unit.c b/src/core/unit.c
index 1bbe5fdded1f41396f5bca0eb569cbaffa984dad..ed0cf026785cd2aeab81bc8e89291fe52d270b1e 100644 (file)
--- a/src/core/unit.c
+++ b/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);

diff --git a/src/core/unit.h b/src/core/unit.h
index 92bb4779a9735a5c24ed7a9720bb182d78036900..fdd82315bac5adc37b5af0eee2fb70275d7c681a 100644 (file)
--- a/src/core/unit.h
+++ b/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);