core: rework unit timeout handling, and add new setting RuntimeMaxSec=

This clean-ups timeout handling in PID 1. Specifically, instead of storing 0 in internal timeout variables as
indication for a disabled timeout, use USEC_INFINITY which is in-line with how we do this in the rest of our code
(following the logic that 0 means "no", and USEC_INFINITY means "never").

This also replace all usec_t additions with invocations to usec_add(), so that USEC_INFINITY is properly propagated,
and sd-event considers it has indication for turning off the event source.

This also alters the deserialization of the units to restart timeouts from the time they were originally started from.
Before this patch timeouts would be restarted beginning with the time of the deserialization, which could lead to
artificially prolonged timeouts if a daemon reload took place.

Finally, a new RuntimeMaxSec= setting is introduced for service units, that specifies a maximum runtime after which a
specific service is forcibly terminated. This is useful to put time limits on time-intensive processing jobs.

This also simplifies the various xyz_spawn() calls of the various types in that explicit distruction of the timers is
removed, as that is done anyway by the state change handlers, and a state change is always done when the xyz_spawn()
calls fail.

Fixes: #2249

1 files changed, 108 insertions, 104 deletions

diff --git a/src/core/service.c b/src/core/service.c
index 355de3e15d..edda02b8f3 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -112,6 +112,7 @@ static void service_init(Unit *u) {
         s->timeout_start_usec = u->manager->default_timeout_start_usec;
         s->timeout_stop_usec = u->manager->default_timeout_stop_usec;
         s->restart_usec = u->manager->default_restart_usec;
+        s->runtime_max_usec = USEC_INFINITY;
         s->type = _SERVICE_TYPE_INVALID;
         s->socket_fd = -1;
         s->bus_endpoint_fd = -1;
@@ -216,7 +217,7 @@ static void service_start_watchdog(Service *s) {
                 return;
 
         if (s->watchdog_event_source) {
-                r = sd_event_source_set_time(s->watchdog_event_source, s->watchdog_timestamp.monotonic + s->watchdog_usec);
+                r = sd_event_source_set_time(s->watchdog_event_source, usec_add(s->watchdog_timestamp.monotonic, s->watchdog_usec));
                 if (r < 0) {
                         log_unit_warning_errno(UNIT(s), r, "Failed to reset watchdog timer: %m");
                         return;
@@ -228,7 +229,7 @@ static void service_start_watchdog(Service *s) {
                                 UNIT(s)->manager->event,
                                 &s->watchdog_event_source,
                                 CLOCK_MONOTONIC,
-                                s->watchdog_timestamp.monotonic + s->watchdog_usec, 0,
+                                usec_add(s->watchdog_timestamp.monotonic, s->watchdog_usec), 0,
                                 service_dispatch_watchdog, s);
                 if (r < 0) {
                         log_unit_warning_errno(UNIT(s), r, "Failed to add watchdog timer: %m");
@@ -433,18 +434,21 @@ static int service_arm_timer(Service *s, usec_t usec) {
         assert(s);
 
         if (s->timer_event_source) {
-                r = sd_event_source_set_time(s->timer_event_source, now(CLOCK_MONOTONIC) + usec);
+                r = sd_event_source_set_time(s->timer_event_source, usec);
                 if (r < 0)
                         return r;
 
                 return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT);
         }
 
+        if (usec == USEC_INFINITY)
+                return 0;
+
         r = sd_event_add_time(
                         UNIT(s)->manager->event,
                         &s->timer_event_source,
                         CLOCK_MONOTONIC,
-                        now(CLOCK_MONOTONIC) + usec, 0,
+                        usec, 0,
                         service_dispatch_timer, s);
         if (r < 0)
                 return r;
@@ -509,6 +513,9 @@ static int service_verify(Service *s) {
         if (!s->usb_function_descriptors && s->usb_function_strings)
                 log_unit_warning(UNIT(s), "Service has USBFunctionStrings= setting, but no USBFunctionDescriptors=. Ignoring.");
 
+        if (s->runtime_max_usec != USEC_INFINITY && s->type == SERVICE_ONESHOT)
+                log_unit_warning(UNIT(s), "MaxRuntimeSec= has no effect in combination with Type=oneshot. Ignoring.");
+
         return 0;
 }
 
@@ -624,7 +631,7 @@ static int service_add_extras(Service *s) {
 
         /* Oneshot services have disabled start timeout by default */
         if (s->type == SERVICE_ONESHOT && !s->start_timeout_defined)
-                s->timeout_start_usec = 0;
+                s->timeout_start_usec = USEC_INFINITY;
 
         service_fix_output(s);
 
@@ -882,6 +889,7 @@ static void service_set_state(Service *s, ServiceState state) {
 
         if (!IN_SET(state,
                     SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
+                    SERVICE_RUNNING,
                     SERVICE_RELOAD,
                     SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST,
                     SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
@@ -954,6 +962,37 @@ static void service_set_state(Service *s, ServiceState state) {
         unit_notify(UNIT(s), table[old_state], table[state], s->reload_result == SERVICE_SUCCESS);
 }
 
+static usec_t service_coldplug_timeout(Service *s) {
+        assert(s);
+
+        switch (s->deserialized_state) {
+
+        case SERVICE_START_PRE:
+        case SERVICE_START:
+        case SERVICE_START_POST:
+        case SERVICE_RELOAD:
+                return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_start_usec);
+
+        case SERVICE_RUNNING:
+                return usec_add(UNIT(s)->active_enter_timestamp.monotonic, s->runtime_max_usec);
+
+        case SERVICE_STOP:
+        case SERVICE_STOP_SIGABRT:
+        case SERVICE_STOP_SIGTERM:
+        case SERVICE_STOP_SIGKILL:
+        case SERVICE_STOP_POST:
+        case SERVICE_FINAL_SIGTERM:
+        case SERVICE_FINAL_SIGKILL:
+                return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_stop_usec);
+
+        case SERVICE_AUTO_RESTART:
+                return usec_add(UNIT(s)->inactive_enter_timestamp.monotonic, s->restart_usec);
+
+        default:
+                return USEC_INFINITY;
+        }
+}
+
 static int service_coldplug(Unit *u) {
         Service *s = SERVICE(u);
         int r;
@@ -964,31 +1003,9 @@ static int service_coldplug(Unit *u) {
         if (s->deserialized_state == s->state)
                 return 0;
 
-        if (IN_SET(s->deserialized_state,
-                   SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
-                   SERVICE_RELOAD,
-                   SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST,
-                   SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {
-
-                usec_t k;
-
-                k = IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec;
-
-                /* For the start/stop timeouts 0 means off */
-                if (k > 0) {
-                        r = service_arm_timer(s, k);
-                        if (r < 0)
-                                return r;
-                }
-        }
-
-        if (s->deserialized_state == SERVICE_AUTO_RESTART) {
-
-                /* The restart timeouts 0 means immediately */
-                r = service_arm_timer(s, s->restart_usec);
-                if (r < 0)
-                        return r;
-        }
+        r = service_arm_timer(s, service_coldplug_timeout(s));
+        if (r < 0)
+                return r;
 
         if (s->main_pid > 0 &&
             pid_is_unwaited(s->main_pid) &&
@@ -1175,7 +1192,7 @@ static int service_spawn(
 
         r = unit_setup_exec_runtime(UNIT(s));
         if (r < 0)
-                goto fail;
+                return r;
 
         if (pass_fds ||
             s->exec_context.std_input == EXEC_INPUT_SOCKET ||
@@ -1184,55 +1201,42 @@ static int service_spawn(
 
                 r = service_collect_fds(s, &fds, &fd_names);
                 if (r < 0)
-                        goto fail;
+                        return r;
 
                 n_fds = r;
         }
 
-        if (timeout > 0) {
-                r = service_arm_timer(s, timeout);
-                if (r < 0)
-                        goto fail;
-        } else
-                s->timer_event_source = sd_event_source_unref(s->timer_event_source);
+        r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), timeout));
+        if (r < 0)
+                return r;
 
         r = unit_full_printf_strv(UNIT(s), c->argv, &argv);
         if (r < 0)
-                goto fail;
+                return r;
 
         our_env = new0(char*, 6);
-        if (!our_env) {
-                r = -ENOMEM;
-                goto fail;
-        }
+        if (!our_env)
+                return -ENOMEM;
 
         if (is_control ? s->notify_access == NOTIFY_ALL : s->notify_access != NOTIFY_NONE)
-                if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0)
+                        return -ENOMEM;
 
         if (s->main_pid > 0)
-                if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0)
+                        return -ENOMEM;
 
         if (UNIT(s)->manager->running_as != MANAGER_SYSTEM)
-                if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid()) < 0) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid()) < 0)
+                        return -ENOMEM;
 
         if (s->socket_fd >= 0) {
                 union sockaddr_union sa;
                 socklen_t salen = sizeof(sa);
 
                 r = getpeername(s->socket_fd, &sa.sa, &salen);
-                if (r < 0) {
-                        r = -errno;
-                        goto fail;
-                }
+                if (r < 0)
+                        return -errno;
 
                 if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
                         _cleanup_free_ char *addr = NULL;
@@ -1241,34 +1245,26 @@ static int service_spawn(
 
                         r = sockaddr_pretty(&sa.sa, salen, true, false, &addr);
                         if (r < 0)
-                                goto fail;
+                                return r;
 
                         t = strappend("REMOTE_ADDR=", addr);
-                        if (!t) {
-                                r = -ENOMEM;
-                                goto fail;
-                        }
+                        if (!t)
+                                return -ENOMEM;
                         our_env[n_env++] = t;
 
                         port = sockaddr_port(&sa.sa);
-                        if (port < 0) {
-                                r = port;
-                                goto fail;
-                        }
+                        if (port < 0)
+                                return port;
 
-                        if (asprintf(&t, "REMOTE_PORT=%u", port) < 0) {
-                                r = -ENOMEM;
-                                goto fail;
-                        }
+                        if (asprintf(&t, "REMOTE_PORT=%u", port) < 0)
+                                return -ENOMEM;
                         our_env[n_env++] = t;
                 }
         }
 
         final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, NULL);
-        if (!final_env) {
-                r = -ENOMEM;
-                goto fail;
-        }
+        if (!final_env)
+                return -ENOMEM;
 
         if (is_control && UNIT(s)->cgroup_path) {
                 path = strjoina(UNIT(s)->cgroup_path, "/control");
@@ -1280,7 +1276,7 @@ static int service_spawn(
                 r = bus_kernel_create_endpoint(UNIT(s)->manager->running_as == MANAGER_SYSTEM ? "system" : "user",
                                                UNIT(s)->id, &bus_endpoint_path);
                 if (r < 0)
-                        goto fail;
+                        return r;
 
                 /* Pass the fd to the exec_params so that the child process can upload the policy.
                  * Keep a reference to the fd in the service, so the endpoint is kept alive as long
@@ -1314,22 +1310,16 @@ static int service_spawn(
                        s->exec_runtime,
                        &pid);
         if (r < 0)
-                goto fail;
+                return r;
 
         r = unit_watch_pid(UNIT(s), pid);
         if (r < 0)
                 /* FIXME: we need to do something here */
-                goto fail;
+                return r;
 
         *_pid = pid;
 
         return 0;
-
-fail:
-        if (timeout)
-                s->timer_event_source = sd_event_source_unref(s->timer_event_source);
-
-        return r;
 }
 
 static int main_pid_good(Service *s) {
@@ -1437,7 +1427,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
 
         if (allow_restart && service_shall_restart(s)) {
 
-                r = service_arm_timer(s, s->restart_usec);
+                r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->restart_usec));
                 if (r < 0)
                         goto fail;
 
@@ -1545,11 +1535,9 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f
                 goto fail;
 
         if (r > 0) {
-                if (s->timeout_stop_usec > 0) {
-                        r = service_arm_timer(s, s->timeout_stop_usec);
-                        if (r < 0)
-                                goto fail;
-                }
+                r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec));
+                if (r < 0)
+                        goto fail;
 
                 service_set_state(s, state);
         } else if (IN_SET(state, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM) && s->kill_context.send_sigkill)
@@ -1577,8 +1565,7 @@ static void service_enter_stop_by_notify(Service *s) {
 
         unit_watch_all_pids(UNIT(s));
 
-        if (s->timeout_stop_usec > 0)
-                service_arm_timer(s, s->timeout_stop_usec);
+        service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec));
 
         /* The service told us it's stopping, so it's as if we SIGTERM'd it. */
         service_set_state(s, SERVICE_STOP_SIGTERM);
@@ -1656,8 +1643,10 @@ static void service_enter_running(Service *s, ServiceResult f) {
                         service_enter_reload_by_notify(s);
                 else if (s->notify_state == NOTIFY_STOPPING)
                         service_enter_stop_by_notify(s);
-                else
+                else {
                         service_set_state(s, SERVICE_RUNNING);
+                        service_arm_timer(s, usec_add(UNIT(s)->active_enter_timestamp.monotonic, s->runtime_max_usec));
+                }
 
         } else if (s->remain_after_exit)
                 service_set_state(s, SERVICE_EXITED);
@@ -1711,6 +1700,7 @@ static void service_kill_control_processes(Service *s) {
 
 static void service_enter_start(Service *s) {
         ExecCommand *c;
+        usec_t timeout;
         pid_t pid;
         int r;
 
@@ -1742,9 +1732,16 @@ static void service_enter_start(Service *s) {
                 return;
         }
 
+        if (IN_SET(s->type, SERVICE_SIMPLE, SERVICE_IDLE))
+                /* For simple + idle this is the main process. We don't apply any timeout here, but
+                 * service_enter_running() will later apply the .runtime_max_usec timeout. */
+                timeout = USEC_INFINITY;
+        else
+                timeout = s->timeout_start_usec;
+
         r = service_spawn(s,
                           c,
-                          IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_NOTIFY, SERVICE_ONESHOT) ? s->timeout_start_usec : 0,
+                          timeout,
                           true,
                           true,
                           true,
@@ -1754,7 +1751,7 @@ static void service_enter_start(Service *s) {
         if (r < 0)
                 goto fail;
 
-        if (s->type == SERVICE_SIMPLE || s->type == SERVICE_IDLE) {
+        if (IN_SET(s->type, SERVICE_SIMPLE, SERVICE_IDLE)) {
                 /* For simple services we immediately start
                  * the START_POST binaries. */
 
@@ -1769,9 +1766,7 @@ static void service_enter_start(Service *s) {
                 s->control_pid = pid;
                 service_set_state(s, SERVICE_START);
 
-        } else if (s->type == SERVICE_ONESHOT ||
-                   s->type == SERVICE_DBUS ||
-                   s->type == SERVICE_NOTIFY) {
+        } else if (IN_SET(s->type, SERVICE_ONESHOT, SERVICE_DBUS, SERVICE_NOTIFY)) {
 
                 /* For oneshot services we wait until the start
                  * process exited, too, but it is our main process. */
@@ -1840,7 +1835,7 @@ static void service_enter_restart(Service *s) {
                 /* Don't restart things if we are going down anyway */
                 log_unit_info(UNIT(s), "Stop job pending for unit, delaying automatic restart.");
 
-                r = service_arm_timer(s, s->restart_usec);
+                r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->restart_usec));
                 if (r < 0)
                         goto fail;
 
@@ -1870,9 +1865,7 @@ fail:
 static void service_enter_reload_by_notify(Service *s) {
         assert(s);
 
-        if (s->timeout_start_usec > 0)
-                service_arm_timer(s, s->timeout_start_usec);
-
+        service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_start_usec));
         service_set_state(s, SERVICE_RELOAD);
 }
 
@@ -1913,6 +1906,7 @@ fail:
 }
 
 static void service_run_next_control(Service *s) {
+        usec_t timeout;
         int r;
 
         assert(s);
@@ -1924,9 +1918,14 @@ static void service_run_next_control(Service *s) {
         s->control_command = s->control_command->command_next;
         service_unwatch_control_pid(s);
 
+        if (IN_SET(s->state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD))
+                timeout = s->timeout_start_usec;
+        else
+                timeout = s->timeout_stop_usec;
+
         r = service_spawn(s,
                           s->control_command,
-                          IN_SET(s->state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec,
+                          timeout,
                           false,
                           !s->permissions_start_only,
                           !s->root_directory_start_only,
@@ -2882,6 +2881,11 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us
                 service_enter_stop(s, SERVICE_FAILURE_TIMEOUT);
                 break;
 
+        case SERVICE_RUNNING:
+                log_unit_warning(UNIT(s), "Service reached runtime time limit. Stopping.");
+                service_enter_stop(s, SERVICE_FAILURE_TIMEOUT);
+                break;
+
         case SERVICE_RELOAD:
                 log_unit_warning(UNIT(s), "Reload operation timed out. Stopping.");
                 service_unwatch_control_pid(s);