Merge pull request #2564 from poettering/fix-2467

Fix for #2467

11 files changed, 90 insertions, 94 deletions

diff --git a/src/core/busname.c b/src/core/busname.c
index 4b0cad8db4..a322252b96 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -945,7 +945,6 @@ static void busname_reset_failed(Unit *u) {
 
 static void busname_trigger_notify(Unit *u, Unit *other) {
         BusName *n = BUSNAME(u);
-        Service *s;
 
         assert(n);
         assert(other);
@@ -953,19 +952,22 @@ static void busname_trigger_notify(Unit *u, Unit *other) {
         if (!IN_SET(n->state, BUSNAME_RUNNING, BUSNAME_LISTENING))
                 return;
 
-        if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE)
+        if (other->start_limit_hit) {
+                busname_enter_dead(n, BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT);
                 return;
+        }
 
-        s = SERVICE(other);
+        if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE)
+                return;
 
-        if (s->state == SERVICE_FAILED && s->result == SERVICE_FAILURE_START_LIMIT)
-                busname_enter_dead(n, BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT);
-        else if (IN_SET(s->state,
-                        SERVICE_DEAD, SERVICE_FAILED,
-                        SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
-                        SERVICE_STOP_POST, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
-                        SERVICE_AUTO_RESTART))
+        if (IN_SET(SERVICE(other)->state,
+                   SERVICE_DEAD, SERVICE_FAILED,
+                   SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
+                   SERVICE_AUTO_RESTART))
                 busname_enter_listening(n);
+
+        if (SERVICE(other)->state == SERVICE_RUNNING)
+                busname_set_state(n, BUSNAME_RUNNING);
 }
 
 static int busname_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) {
@@ -1006,7 +1008,7 @@ static const char* const busname_result_table[_BUSNAME_RESULT_MAX] = {
         [BUSNAME_FAILURE_EXIT_CODE] = "exit-code",
         [BUSNAME_FAILURE_SIGNAL] = "signal",
         [BUSNAME_FAILURE_CORE_DUMP] = "core-dump",
-        [BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT] = "service-failed-permanent",
+        [BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(busname_result, BusNameResult);
diff --git a/src/core/busname.h b/src/core/busname.h
index 46f7b6f097..bd37083695 100644
--- a/src/core/busname.h
+++ b/src/core/busname.h
@@ -33,7 +33,7 @@ typedef enum BusNameResult {
         BUSNAME_FAILURE_EXIT_CODE,
         BUSNAME_FAILURE_SIGNAL,
         BUSNAME_FAILURE_CORE_DUMP,
-        BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT,
+        BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT,
         _BUSNAME_RESULT_MAX,
         _BUSNAME_RESULT_INVALID = -1
 } BusNameResult;
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 16f50238a1..f36ce3f9fe 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -52,10 +52,11 @@ const sd_bus_vtable bus_service_vtable[] = {
         SD_BUS_PROPERTY("RuntimeMaxUSec", "t", bus_property_get_usec, offsetof(Service, runtime_max_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("WatchdogUSec", "t", bus_property_get_usec, offsetof(Service, watchdog_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, watchdog_timestamp), 0),
-        SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Service, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
+        /* The following four are obsolete, and thus marked hidden here. They moved into the Unit interface */
+        SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN),
+        SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN),
+        SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN),
+        SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN),
         SD_BUS_PROPERTY("FailureAction", "s", property_get_failure_action, offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 7b0ae03228..a3da7455e6 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -704,6 +704,10 @@ const sd_bus_vtable bus_unit_vtable[] = {
         SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("NetClass", "u", NULL, offsetof(Unit, cgroup_netclass_id), 0),
+        SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
 
         SD_BUS_METHOD("Start", "s", "o", method_start, SD_BUS_VTABLE_UNPRIVILEGED),
         SD_BUS_METHOD("Stop", "s", "o", method_stop, SD_BUS_VTABLE_UNPRIVILEGED),
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 2507db1932..c2a47c7b11 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -164,6 +164,10 @@ Unit.IgnoreOnSnapshot,           config_parse_warn_compat,           DISABLED_LE
 Unit.JobTimeoutSec,              config_parse_sec,                   0,                             offsetof(Unit, job_timeout)
 Unit.JobTimeoutAction,           config_parse_failure_action,        0,                             offsetof(Unit, job_timeout_action)
 Unit.JobTimeoutRebootArgument,   config_parse_string,                0,                             offsetof(Unit, job_timeout_reboot_arg)
+Unit.StartLimitInterval,         config_parse_sec,                   0,                             offsetof(Unit, start_limit.interval)
+Unit.StartLimitBurst,            config_parse_unsigned,              0,                             offsetof(Unit, start_limit.burst)
+Unit.StartLimitAction,           config_parse_failure_action,        0,                             offsetof(Unit, start_limit_action)
+Unit.RebootArgument,             config_parse_string,                0,                             offsetof(Unit, reboot_arg)
 Unit.ConditionPathExists,        config_parse_unit_condition_path,   CONDITION_PATH_EXISTS,         offsetof(Unit, conditions)
 Unit.ConditionPathExistsGlob,    config_parse_unit_condition_path,   CONDITION_PATH_EXISTS_GLOB,    offsetof(Unit, conditions)
 Unit.ConditionPathIsDirectory,   config_parse_unit_condition_path,   CONDITION_PATH_IS_DIRECTORY,   offsetof(Unit, conditions)
@@ -216,10 +220,10 @@ Service.TimeoutStartSec,         config_parse_service_timeout,       0,
 Service.TimeoutStopSec,          config_parse_service_timeout,       0,                             offsetof(Service, timeout_stop_usec)
 Service.RuntimeMaxSec,           config_parse_sec,                   0,                             offsetof(Service, runtime_max_usec)
 Service.WatchdogSec,             config_parse_sec,                   0,                             offsetof(Service, watchdog_usec)
-Service.StartLimitInterval,      config_parse_sec,                   0,                             offsetof(Service, start_limit.interval)
-Service.StartLimitBurst,         config_parse_unsigned,              0,                             offsetof(Service, start_limit.burst)
-Service.StartLimitAction,        config_parse_failure_action,        0,                             offsetof(Service, start_limit_action)
-Service.RebootArgument,          config_parse_string,                0,                             offsetof(Service, reboot_arg)
+Service.StartLimitInterval,      config_parse_sec,                   0,                             offsetof(Unit, start_limit.interval)
+Service.StartLimitBurst,         config_parse_unsigned,              0,                             offsetof(Unit, start_limit.burst)
+Service.StartLimitAction,        config_parse_failure_action,        0,                             offsetof(Unit, start_limit_action)
+Service.RebootArgument,          config_parse_string,                0,                             offsetof(Unit, reboot_arg)
 Service.FailureAction,           config_parse_failure_action,        0,                             offsetof(Service, failure_action)
 Service.Type,                    config_parse_service_type,          0,                             offsetof(Service, type)
 Service.Restart,                 config_parse_service_restart,       0,                             offsetof(Service, restart)
diff --git a/src/core/service.c b/src/core/service.c
index 02ce1a566a..3bb3cc8b18 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -119,8 +119,6 @@ static void service_init(Unit *u) {
         s->stdin_fd = s->stdout_fd = s->stderr_fd = -1;
         s->guess_main_pid = true;
 
-        RATELIMIT_INIT(s->start_limit, u->manager->default_start_limit_interval, u->manager->default_start_limit_burst);
-
         s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID;
 }
 
@@ -302,7 +300,6 @@ static void service_done(Unit *u) {
 
         s->pid_file = mfree(s->pid_file);
         s->status_text = mfree(s->status_text);
-        s->reboot_arg = mfree(s->reboot_arg);
 
         s->exec_runtime = exec_runtime_unref(s->exec_runtime);
         exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX);
@@ -1422,7 +1419,7 @@ 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));
-                failure_action(UNIT(s)->manager, s->failure_action, s->reboot_arg);
+                failure_action(UNIT(s)->manager, s->failure_action, UNIT(s)->reboot_arg);
         }
 
         if (allow_restart && service_shall_restart(s)) {
@@ -1989,20 +1986,8 @@ fail:
         service_enter_stop(s, SERVICE_FAILURE_RESOURCES);
 }
 
-static int service_start_limit_test(Service *s) {
-        assert(s);
-
-        if (ratelimit_test(&s->start_limit))
-                return 0;
-
-        log_unit_warning(UNIT(s), "Start request repeated too quickly.");
-
-        return failure_action(UNIT(s)->manager, s->start_limit_action, s->reboot_arg);
-}
-
 static int service_start(Unit *u) {
         Service *s = SERVICE(u);
-        int r;
 
         assert(s);
 
@@ -2029,13 +2014,6 @@ static int service_start(Unit *u) {
 
         assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED));
 
-        /* Make sure we don't enter a busy loop of some kind. */
-        r = service_start_limit_test(s);
-        if (r < 0) {
-                service_enter_dead(s, SERVICE_FAILURE_START_LIMIT, false);
-                return r;
-        }
-
         s->result = SERVICE_SUCCESS;
         s->reload_result = SERVICE_SUCCESS;
         s->main_pid_known = false;
@@ -3248,8 +3226,6 @@ static void service_reset_failed(Unit *u) {
 
         s->result = SERVICE_SUCCESS;
         s->reload_result = SERVICE_SUCCESS;
-
-        RATELIMIT_RESET(s->start_limit);
 }
 
 static int service_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) {
@@ -3317,7 +3293,6 @@ static const char* const service_result_table[_SERVICE_RESULT_MAX] = {
         [SERVICE_FAILURE_SIGNAL] = "signal",
         [SERVICE_FAILURE_CORE_DUMP] = "core-dump",
         [SERVICE_FAILURE_WATCHDOG] = "watchdog",
-        [SERVICE_FAILURE_START_LIMIT] = "start-limit"
 };
 
 DEFINE_STRING_TABLE_LOOKUP(service_result, ServiceResult);
diff --git a/src/core/service.h b/src/core/service.h
index 24408940d4..55fcec48c7 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -88,7 +88,6 @@ typedef enum ServiceResult {
         SERVICE_FAILURE_SIGNAL,
         SERVICE_FAILURE_CORE_DUMP,
         SERVICE_FAILURE_WATCHDOG,
-        SERVICE_FAILURE_START_LIMIT,
         _SERVICE_RESULT_MAX,
         _SERVICE_RESULT_INVALID = -1
 } ServiceResult;
@@ -178,10 +177,7 @@ struct Service {
         char *status_text;
         int status_errno;
 
-        RateLimit start_limit;
-        FailureAction start_limit_action;
         FailureAction failure_action;
-        char *reboot_arg;
 
         UnitRef accept_socket;
 
diff --git a/src/core/socket.c b/src/core/socket.c
index f0d65577e3..1f24f289e0 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2702,23 +2702,6 @@ static void socket_reset_failed(Unit *u) {
         s->result = SOCKET_SUCCESS;
 }
 
-static void socket_notify_service_dead(Socket *s, bool failed_permanent) {
-        assert(s);
-
-        /* The service is dead. Dang!
-         *
-         * This is strictly for one-instance-for-all-connections
-         * services. */
-
-        if (s->state == SOCKET_RUNNING) {
-                log_unit_debug(UNIT(s), "Got notified about service death (failed permanently: %s)", yes_no(failed_permanent));
-                if (failed_permanent)
-                        socket_enter_stop_pre(s, SOCKET_FAILURE_SERVICE_FAILED_PERMANENT);
-                else
-                        socket_enter_listening(s);
-        }
-}
-
 void socket_connection_unref(Socket *s) {
         assert(s);
 
@@ -2735,34 +2718,30 @@ void socket_connection_unref(Socket *s) {
 
 static void socket_trigger_notify(Unit *u, Unit *other) {
         Socket *s = SOCKET(u);
-        Service *se;
 
         assert(u);
         assert(other);
 
         /* Don't propagate state changes from the service if we are
            already down or accepting connections */
-        if ((s->state != SOCKET_RUNNING &&
-            s->state != SOCKET_LISTENING) ||
-            s->accept)
+        if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING) || s->accept)
                 return;
 
-        if (other->load_state != UNIT_LOADED ||
-            other->type != UNIT_SERVICE)
+        if (other->start_limit_hit) {
+                socket_enter_stop_pre(s, SOCKET_FAILURE_SERVICE_START_LIMIT_HIT);
                 return;
+        }
 
-        se = SERVICE(other);
-
-        if (se->state == SERVICE_FAILED)
-                socket_notify_service_dead(s, se->result == SERVICE_FAILURE_START_LIMIT);
+        if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE)
+                return;
 
-        if (se->state == SERVICE_DEAD ||
-            se->state == SERVICE_FINAL_SIGTERM ||
-            se->state == SERVICE_FINAL_SIGKILL ||
-            se->state == SERVICE_AUTO_RESTART)
-                socket_notify_service_dead(s, false);
+        if (IN_SET(SERVICE(other)->state,
+                   SERVICE_DEAD, SERVICE_FAILED,
+                   SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
+                   SERVICE_AUTO_RESTART))
+               socket_enter_listening(s);
 
-        if (se->state == SERVICE_RUNNING)
+        if (SERVICE(other)->state == SERVICE_RUNNING)
                 socket_set_state(s, SOCKET_RUNNING);
 }
 
@@ -2818,7 +2797,7 @@ static const char* const socket_result_table[_SOCKET_RESULT_MAX] = {
         [SOCKET_FAILURE_EXIT_CODE] = "exit-code",
         [SOCKET_FAILURE_SIGNAL] = "signal",
         [SOCKET_FAILURE_CORE_DUMP] = "core-dump",
-        [SOCKET_FAILURE_SERVICE_FAILED_PERMANENT] = "service-failed-permanent"
+        [SOCKET_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit"
 };
 
 DEFINE_STRING_TABLE_LOOKUP(socket_result, SocketResult);
diff --git a/src/core/socket.h b/src/core/socket.h
index 08033287a6..ca3f815a47 100644
--- a/src/core/socket.h
+++ b/src/core/socket.h
@@ -54,7 +54,7 @@ typedef enum SocketResult {
         SOCKET_FAILURE_EXIT_CODE,
         SOCKET_FAILURE_SIGNAL,
         SOCKET_FAILURE_CORE_DUMP,
-        SOCKET_FAILURE_SERVICE_FAILED_PERMANENT,
+        SOCKET_FAILURE_SERVICE_START_LIMIT_HIT,
         _SOCKET_RESULT_MAX,
         _SOCKET_RESULT_INVALID = -1
 } SocketResult;
diff --git a/src/core/unit.c b/src/core/unit.c
index be4dac8fea..9df951badb 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -101,6 +101,7 @@ Unit *unit_new(Manager *m, size_t size) {
         u->cgroup_inotify_wd = -1;
         u->job_timeout = USEC_INFINITY;
 
+        RATELIMIT_INIT(u->start_limit, m->default_start_limit_interval, m->default_start_limit_burst);
         RATELIMIT_INIT(u->auto_stop_ratelimit, 10 * USEC_PER_SEC, 16);
 
         return u;
@@ -559,6 +560,8 @@ void unit_free(Unit *u) {
         condition_free_list(u->conditions);
         condition_free_list(u->asserts);
 
+        free(u->reboot_arg);
+
         unit_ref_unset(&u->slice);
 
         while (u->refs)
@@ -1446,23 +1449,36 @@ void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) {
         unit_status_print_starting_stopping(u, t);
 }
 
+static int unit_start_limit_test(Unit *u) {
+        assert(u);
+
+        if (ratelimit_test(&u->start_limit)) {
+                u->start_limit_hit = false;
+                return 0;
+        }
+
+        log_unit_warning(u, "Start request repeated too quickly.");
+        u->start_limit_hit = true;
+
+        return failure_action(u->manager, u->start_limit_action, u->reboot_arg);
+}
+
 /* Errors:
- *         -EBADR:     This unit type does not support starting.
- *         -EALREADY:  Unit is already started.
- *         -EAGAIN:    An operation is already in progress. Retry later.
- *         -ECANCELED: Too many requests for now.
- *         -EPROTO:    Assert failed
+ *         -EBADR:      This unit type does not support starting.
+ *         -EALREADY:   Unit is already started.
+ *         -EAGAIN:     An operation is already in progress. Retry later.
+ *         -ECANCELED:  Too many requests for now.
+ *         -EPROTO:     Assert failed
+ *         -EINVAL:     Unit not loaded
+ *         -EOPNOTSUPP: Unit type not supported
  */
 int unit_start(Unit *u) {
         UnitActiveState state;
         Unit *following;
+        int r;
 
         assert(u);
 
-        /* Units that aren't loaded cannot be started */
-        if (u->load_state != UNIT_LOADED)
-                return -EINVAL;
-
         /* If this is already started, then this will succeed. Note
          * that this will even succeed if this unit is not startable
          * by the user. This is relied on to detect when we need to
@@ -1471,6 +1487,15 @@ int unit_start(Unit *u) {
         if (UNIT_IS_ACTIVE_OR_RELOADING(state))
                 return -EALREADY;
 
+        /* Make sure we don't enter a busy loop of some kind. */
+        r = unit_start_limit_test(u);
+        if (r < 0)
+                return r;
+
+        /* Units that aren't loaded cannot be started */
+        if (u->load_state != UNIT_LOADED)
+                return -EINVAL;
+
         /* If the conditions failed, don't do anything at all. If we
          * already are activating this call might still be useful to
          * speed up activation in case there is some hold-off time,
@@ -2988,6 +3013,9 @@ void unit_reset_failed(Unit *u) {
 
         if (UNIT_VTABLE(u)->reset_failed)
                 UNIT_VTABLE(u)->reset_failed(u);
+
+        RATELIMIT_RESET(u->start_limit);
+        u->start_limit_hit = false;
 }
 
 Unit *unit_following(Unit *u) {
diff --git a/src/core/unit.h b/src/core/unit.h
index 8712e03133..a87ef74fb3 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -167,6 +167,11 @@ struct Unit {
         /* Error code when we didn't manage to load the unit (negative) */
         int load_error;
 
+        /* Put a ratelimit on unit starting */
+        RateLimit start_limit;
+        FailureAction start_limit_action;
+        char *reboot_arg;
+
         /* Make sure we never enter endless loops with the check unneeded logic, or the BindsTo= logic */
         RateLimit auto_stop_ratelimit;
 
@@ -230,6 +235,8 @@ struct Unit {
         bool cgroup_members_mask_valid:1;
         bool cgroup_subtree_mask_valid:1;
 
+        bool start_limit_hit:1;
+
         /* Did we already invoke unit_coldplug() for this unit? */
         bool coldplugged:1;
 };