diff --git a/src/core/service.c b/src/core/service.c index bf6e91016c..5c097c139d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -70,6 +70,8 @@ #define SERVICE_FD_STORE_POPULATED(s) (!!(s)->fd_store) +#define SERVICE_BUS_NAME_GRACE_USEC (2 * USEC_PER_SEC) + static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_DEAD] = UNIT_INACTIVE, [SERVICE_CONDITION] = UNIT_ACTIVATING, @@ -142,6 +144,7 @@ static int service_dispatch_inotify_io(sd_event_source *source, int fd, uint32_t static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void *userdata); static int service_dispatch_exec_io(sd_event_source *source, int fd, uint32_t events, void *userdata); +static int service_dispatch_bus_name_grace(sd_event_source *source, usec_t usec, void *userdata); static void service_enter_signal(Service *s, ServiceState state, ServiceResult f); @@ -215,6 +218,7 @@ static void service_init(Unit *u) { s->oom_policy = _OOM_POLICY_INVALID; s->reload_begin_usec = USEC_INFINITY; + s->revalidate_runtime_begin_usec = USEC_INFINITY; s->reload_signal = SIGHUP; s->fd_store_preserve_mode = EXEC_PRESERVE_RESTART; @@ -620,6 +624,7 @@ static void service_done(Unit *u) { service_stop_watchdog(s); s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); + s->bus_name_grace_event_source = sd_event_source_disable_unref(s->bus_name_grace_event_source); s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source); s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot); @@ -1670,6 +1675,9 @@ static void service_set_state(Service *s, ServiceState state) { SERVICE_CLEANING)) s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); + if (state != SERVICE_RUNNING_REVALIDATING) + s->bus_name_grace_event_source = sd_event_source_disable_unref(s->bus_name_grace_event_source); + if (!SERVICE_STATE_WITH_MAIN_PROCESS(state)) { service_unwatch_main_pid(s); s->main_command = NULL; @@ -1783,6 +1791,14 @@ static int service_coldplug(Unit *u) { if (r < 0) return r; + if (s->deserialized_state == SERVICE_RUNNING_REVALIDATING) { + r = unit_arm_timer(UNIT(s), &s->bus_name_grace_event_source, /* relative= */ true, + SERVICE_BUS_NAME_GRACE_USEC, + service_dispatch_bus_name_grace); + if (r < 0) + return r; + } + if (pidref_is_set(&s->main_pid) && pidref_is_unwaited(&s->main_pid) > 0 && SERVICE_STATE_WITH_MAIN_PROCESS(s->deserialized_state)) { @@ -2741,14 +2757,11 @@ static void service_enter_stop(Service *s, ServiceResult f) { service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_SUCCESS); } -static bool service_good(Service *s) { +static bool service_good_except_bus_name(Service *s) { int main_pid_ok; assert(s); - if (s->type == SERVICE_DBUS && !s->bus_name_good) - return false; - main_pid_ok = main_pid_good(s); if (main_pid_ok > 0) /* It's alive */ return true; @@ -2762,6 +2775,26 @@ static bool service_good(Service *s) { return cgroup_good(s) != 0; } +static bool service_good(Service *s) { + assert(s); + + if (s->type == SERVICE_DBUS && !s->bus_name_good) + return false; + + return service_good_except_bus_name(s); +} + +static void service_enter_exited_or_stop(Service *s) { + assert(s); + + s->revalidate_runtime_begin_usec = USEC_INFINITY; + + if (s->remain_after_exit) + service_set_state(s, SERVICE_EXITED); + else + service_enter_stop(s, SERVICE_SUCCESS); +} + static void service_enter_running(Service *s, ServiceResult f) { int r; @@ -2784,7 +2817,17 @@ static void service_enter_running(Service *s, ServiceResult f) { else { service_set_state(s, SERVICE_RUNNING); - r = service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); + /* We're resuming. Recompute the budget from the current config (which was measured + * from when we entered revalidation). */ + usec_t remaining = USEC_INFINITY; + if (s->revalidate_runtime_begin_usec != USEC_INFINITY) + remaining = usec_sub_unsigned(service_running_timeout(s), s->revalidate_runtime_begin_usec); + s->revalidate_runtime_begin_usec = USEC_INFINITY; + + if (remaining != USEC_INFINITY) + r = service_arm_timer(s, /* relative= */ true, remaining); + else + r = service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); service_enter_running(s, SERVICE_FAILURE_RESOURCES); @@ -2792,10 +2835,33 @@ static void service_enter_running(Service *s, ServiceResult f) { } } - } else if (s->remain_after_exit) - service_set_state(s, SERVICE_EXITED); - else - service_enter_stop(s, SERVICE_SUCCESS); + } else if (s->type == SERVICE_DBUS && !s->bus_name_good && service_good_except_bus_name(s)) { + + /* If we're already revalidating, just keep waiting on the timer we armed on the way in, so a + * flapping broker that reconnects repeatedly can't keep pushing the deadline out. */ + if (s->state == SERVICE_RUNNING_REVALIDATING) + return; + + log_unit_warning(UNIT(s), + "D-Bus name %s vanished (possibly from an unsupported broker restart), giving it %s to return.", + s->bus_name, + FORMAT_TIMESPAN(SERVICE_BUS_NAME_GRACE_USEC, USEC_PER_SEC)); + + r = unit_arm_timer(UNIT(s), &s->bus_name_grace_event_source, /* relative= */ true, + SERVICE_BUS_NAME_GRACE_USEC, service_dispatch_bus_name_grace); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to arm D-Bus name grace timer: %m"); + service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + return; + } + + if (s->state == SERVICE_RUNNING) + s->revalidate_runtime_begin_usec = now(CLOCK_MONOTONIC); + + service_set_state(s, SERVICE_RUNNING_REVALIDATING); + + } else + service_enter_exited_or_stop(s); } static void service_enter_start_post(Service *s) { @@ -3561,6 +3627,7 @@ static int service_start(Unit *u) { s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; s->reload_begin_usec = USEC_INFINITY; + s->revalidate_runtime_begin_usec = USEC_INFINITY; s->status_text = mfree(s->status_text); s->status_errno = 0; @@ -3926,6 +3993,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_usec(f, "watchdog-override-usec", s->watchdog_override_usec); (void) serialize_usec(f, "reload-begin-usec", s->reload_begin_usec); + (void) serialize_usec(f, "revalidate-runtime-begin-usec", s->revalidate_runtime_begin_usec); if (s->refreshed_mask > 0) { _cleanup_strv_free_ char **l = NULL; @@ -4361,6 +4429,8 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, } else if (streq(key, "reload-begin-usec")) (void) deserialize_usec(value, &s->reload_begin_usec); + else if (streq(key, "revalidate-runtime-begin-usec")) + (void) deserialize_usec(value, &s->revalidate_runtime_begin_usec); else if (streq(key, "refreshed-mask")) { r = service_refresh_on_reload_from_string_many(value, &s->refreshed_mask); if (r < 0) @@ -5446,7 +5516,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) if (strv_contains(tags, "STOPPING=1")) { s->notify_state = NOTIFY_STOPPING; - if (IN_SET(s->state, SERVICE_RUNNING, + if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY)) service_enter_stop_by_notify(s); @@ -5483,7 +5553,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) /* Valid Type=notify-reload protocol? Then we're all good. */ service_enter_reload_post(s); - else if (s->state == SERVICE_RUNNING) { + else if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING)) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; /* Propagate a reload explicitly for plain RELOADING=1 (semantically equivalent to @@ -5524,6 +5594,8 @@ static void service_notify_message_process_state(Service *s, char * const *tags) * don't need reload propagation nor do we want to restart the timeout. */ service_set_state(s, SERVICE_RELOAD_NOTIFY); + /* The reload will still be preserved by service_enter_running() replaying it once the bus + * name returns. */ if (s->state == SERVICE_RUNNING) service_enter_reload_by_notify(s); } @@ -5558,7 +5630,7 @@ static void service_notify_message( r = service_notify_message_parse_new_pid(u, tags, fds, &new_main_pid); if (r > 0 && - IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_REFRESH_EXTENSIONS, SERVICE_REFRESH_CREDENTIALS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_STOP, SERVICE_STOP_SIGTERM) && @@ -5702,7 +5774,7 @@ static void service_notify_message( } /* Interpret RESTART_RESET=1 */ - if (strv_contains(tags, "RESTART_RESET=1") && IN_SET(s->state, SERVICE_RUNNING, SERVICE_STOP)) { + if (strv_contains(tags, "RESTART_RESET=1") && IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING, SERVICE_STOP)) { log_unit_struct(u, LOG_NOTICE, LOG_UNIT_MESSAGE(u, "Got RESTART_RESET=1, resetting restart counter from %u.", s->n_restarts), LOG_ITEM("N_RESTARTS=0"), @@ -5877,7 +5949,19 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s return 1; } -static void service_bus_name_owner_change(Unit *u, const char *new_owner) { +static int service_dispatch_bus_name_grace(sd_event_source *source, usec_t usec, void *userdata) { + Service *s = ASSERT_PTR(SERVICE(userdata)); + + assert(source == s->bus_name_grace_event_source); + assert(s->state == SERVICE_RUNNING_REVALIDATING); + + log_unit_warning(UNIT(s), "D-Bus name %s still not owned after grace period, giving up.", s->bus_name); + service_enter_exited_or_stop(s); + + return 0; +} + +static void service_bus_name_owner_change(Unit *u, const char *new_owner, bool from_signal) { Service *s = ASSERT_PTR(SERVICE(u)); int r; @@ -5889,10 +5973,12 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) { s->bus_name_good = new_owner; if (s->type == SERVICE_DBUS) { - /* service_enter_running() will figure out what to do */ - if (s->state == SERVICE_RUNNING) - service_enter_running(s, SERVICE_SUCCESS); - else if (s->state == SERVICE_START && new_owner) + if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RUNNING_REVALIDATING)) { + if (!new_owner && from_signal) + service_enter_exited_or_stop(s); + else + service_enter_running(s, SERVICE_SUCCESS); + } else if (s->state == SERVICE_START && new_owner) service_enter_start_post(s); } else if (new_owner && pick_up_pid_from_bus_name(s)) { diff --git a/src/core/service.h b/src/core/service.h index c028248bba..c6002eb1ba 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -163,6 +163,7 @@ typedef struct Service { bool timeout_abort_set; usec_t runtime_max_usec; usec_t runtime_rand_extra_usec; + usec_t revalidate_runtime_begin_usec; ServiceTimeoutFailureMode timeout_start_failure_mode; ServiceTimeoutFailureMode timeout_stop_failure_mode; @@ -226,6 +227,7 @@ typedef struct Service { bool exec_fd_hot:1; char *bus_name; + sd_event_source *bus_name_grace_event_source; char *status_text; char *status_bus_error; diff --git a/src/core/unit.c b/src/core/unit.c index 1d5fe8859e..31d3740d0f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3593,7 +3593,7 @@ static int signal_name_owner_changed_install_handler(sd_bus_message *message, vo u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot); if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, NULL); + UNIT_VTABLE(u)->bus_name_owner_change(u, NULL, /* from_signal= */ true); return 0; } @@ -3612,7 +3612,7 @@ static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd } if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, empty_to_null(new_owner)); + UNIT_VTABLE(u)->bus_name_owner_change(u, empty_to_null(new_owner), /* from_signal= */ true); return 0; } @@ -3645,8 +3645,11 @@ static int get_name_owner_handler(sd_bus_message *message, void *userdata, sd_bu assert(!isempty(new_owner)); } + /* Given we're in a GetNameOwner() refresh, which we issue right after (re)connecting to the broker, + * a NULL owner here may simply mean the owning client is still getting its bearings, so treat it as + * non-definitive and worth waiting a bit for. */ if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, new_owner); + UNIT_VTABLE(u)->bus_name_owner_change(u, new_owner, /* from_signal= */ false); return 0; } diff --git a/src/core/unit.h b/src/core/unit.h index f96ae27936..c07588ac1e 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -670,7 +670,7 @@ typedef struct UnitVTable { void (*notify_pidref)(Unit *u, PidRef *parent_pidref, PidRef *child_pidref); /* Called whenever a name this Unit registered for comes or goes away. */ - void (*bus_name_owner_change)(Unit *u, const char *new_owner); + void (*bus_name_owner_change)(Unit *u, const char *new_owner, bool from_signal); /* Called for each property that is being set */ int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *reterr_error);