From 1fa9473b07e16ee863c045183251c6613cef375d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 22 Oct 2025 20:50:17 +0200 Subject: [PATCH 01/21] core/job: mark job_type_lookup_merge() and _is_redundant() as const They don't take pointers, hence are eligible for stronger guarantees. --- src/core/job.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/job.h b/src/core/job.h index d8be0b652e..028281eb5f 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -162,7 +162,7 @@ int job_coldplug(Job *j); JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); void job_dependency_free(JobDependency *l); -JobType job_type_lookup_merge(JobType a, JobType b) _pure_; +JobType job_type_lookup_merge(JobType a, JobType b) _const_; _pure_ static inline bool job_type_is_mergeable(JobType a, JobType b) { return job_type_lookup_merge(a, b) >= 0; @@ -181,7 +181,7 @@ _pure_ static inline bool job_type_is_superset(JobType a, JobType b) { return a == job_type_lookup_merge(a, b); } -bool job_type_is_redundant(JobType a, UnitActiveState b) _pure_; +bool job_type_is_redundant(JobType a, UnitActiveState b) _const_; /* Collapses a state-dependent job type into a simpler type by observing * the state of the unit which it is going to be applied to. */ From b1947bf6e316644b0c2098e9ebb338b85c78bc4b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 22 Oct 2025 20:53:15 +0200 Subject: [PATCH 02/21] core/job: drop pure qualifier for static inline functions The impl is directly visible to the compiler so it can apply all sorts of optimizations wherever it sees fit. And with the previous commit they are actually "const". --- src/core/job.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/job.h b/src/core/job.h index 028281eb5f..4c2f78815b 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -164,15 +164,15 @@ void job_dependency_free(JobDependency *l); JobType job_type_lookup_merge(JobType a, JobType b) _const_; -_pure_ static inline bool job_type_is_mergeable(JobType a, JobType b) { +static inline bool job_type_is_mergeable(JobType a, JobType b) { return job_type_lookup_merge(a, b) >= 0; } -_pure_ static inline bool job_type_is_conflicting(JobType a, JobType b) { +static inline bool job_type_is_conflicting(JobType a, JobType b) { return a != JOB_NOP && b != JOB_NOP && !job_type_is_mergeable(a, b); } -_pure_ static inline bool job_type_is_superset(JobType a, JobType b) { +static inline bool job_type_is_superset(JobType a, JobType b) { /* Checks whether operation a is a "superset" of b in its actions */ if (b == JOB_NOP) return true; From 969c8050d8e4b77125a063a71ded3aff6c734ee7 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 22 Oct 2025 20:16:55 +0200 Subject: [PATCH 03/21] core/job: use UNIT_IS_* helpers --- src/core/job.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/core/job.c b/src/core/job.c index 42bca3a253..5e53512c9d 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -436,13 +436,11 @@ bool job_type_is_redundant(JobType a, UnitActiveState b) { switch (a) { case JOB_START: - return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING, UNIT_REFRESHING); + case JOB_VERIFY_ACTIVE: + return UNIT_IS_ACTIVE_OR_RELOADING(b); case JOB_STOP: - return IN_SET(b, UNIT_INACTIVE, UNIT_FAILED); - - case JOB_VERIFY_ACTIVE: - return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING, UNIT_REFRESHING); + return UNIT_IS_INACTIVE_OR_FAILED(b); case JOB_RELOAD: /* Reload jobs are never considered redundant/duplicate. Refer to jobs_may_late_merge() for From af3a197515e596eb21f1287d6ff46252c278b449 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:43:27 +0200 Subject: [PATCH 04/21] core/service: no need for STATUS_TEXT_MAX to reside in header --- src/core/service.c | 2 ++ src/core/service.h | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index f45d0c4801..91434f1382 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -56,6 +56,8 @@ #include "unit-name.h" #include "utf8.h" +#define STATUS_TEXT_MAX (16U*1024U) + #define service_spawn(...) service_spawn_internal(__func__, __VA_ARGS__) static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { diff --git a/src/core/service.h b/src/core/service.h index b69f3008de..8965dc827b 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -298,7 +298,5 @@ ServiceTimeoutFailureMode service_timeout_failure_mode_from_string(const char *s DEFINE_CAST(SERVICE, Service); -#define STATUS_TEXT_MAX (16U*1024U) - /* Only exported for unit tests */ int service_deserialize_exec_command(Unit *u, const char *key, const char *value); From aebe553db389ff859237a7c44332f401922c99ac Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 18:37:41 +0200 Subject: [PATCH 05/21] core/service: do not set reload_begin_usec when refreshing confexts reload_begin_usec is a barrier for determining whether RELOADING=1 came after the main process got signaled, unrelated to refreshing. --- src/core/service.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 91434f1382..70512aa239 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2879,8 +2879,6 @@ static void service_enter_reload_mounting(Service *s) { assert(s); - usec_t ts = now(CLOCK_MONOTONIC); - r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); @@ -2889,8 +2887,6 @@ static void service_enter_reload_mounting(Service *s) { return; } - s->reload_begin_usec = ts; - service_enter_refresh_extensions(s); } From 2e43eee666de246500c73bead594bc9b236b7752 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 20:16:05 +0200 Subject: [PATCH 06/21] core/service: restore spuriously changed comment Not sure why dfdeb0b1cbb05a213f0965eedfe0e7ef06cd39d3 touched it at all. --- src/core/service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/service.c b/src/core/service.c index 70512aa239..c1f6d3445a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4835,7 +4835,7 @@ static void service_notify_message( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; /* Propagate a reload explicitly for plain RELOADING=1 (semantically equivalent to - * service_enter_reload_mounting() call in below) */ + * service_enter_reload_by_notify() call in below) */ r = manager_propagate_reload(UNIT(s)->manager, UNIT(s), JOB_FAIL, &error); if (r < 0) log_unit_warning(UNIT(s), "Failed to schedule propagation of reload, ignoring: %s", From 16a0cbe915a99edc4592417e932527c1558b44a3 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 19:41:13 +0200 Subject: [PATCH 07/21] core/service: merge service_enter_reload_mounting() into _refresh_extensions() --- src/core/service.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index c1f6d3445a..48462b5d18 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2809,13 +2809,19 @@ static void service_enter_refresh_extensions(Service *s) { /* If we don't have extensions to reload, immediately go to the signal step */ if (!service_should_reload_extensions(s)) - return (void) service_enter_reload_signal_exec(s); + return service_enter_reload_signal_exec(s); service_unwatch_control_pid(s); s->reload_result = SERVICE_SUCCESS; s->control_command = NULL; s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; + r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + goto fail; + } + /* Given we are running from PID1, avoid doing potentially heavy I/O operations like opening images * directly, and instead fork a worker process. */ r = unit_fork_helper_process(UNIT(s), "(sd-refresh-extensions)", /* into_cgroup= */ false, &worker); @@ -2874,22 +2880,6 @@ fail: service_enter_running(s, SERVICE_SUCCESS); } -static void service_enter_reload_mounting(Service *s) { - int r; - - assert(s); - - r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); - if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); - return; - } - - service_enter_refresh_extensions(s); -} - static void service_run_next_control(Service *s) { usec_t timeout; int r; @@ -3106,7 +3096,7 @@ static int service_reload(Unit *u) { assert(IN_SET(s->state, SERVICE_RUNNING, SERVICE_EXITED)); - service_enter_reload_mounting(s); + service_enter_refresh_extensions(s); return 1; } From 1773d2a2075d5c75ea427870ec8867e16f2dad2a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 20:14:08 +0200 Subject: [PATCH 08/21] core: drop redundant pidref_done() calls {service,socket}_unwatch_control_pid() -> unit_unwatch_pidref_done() is unconditionally called everywhere. --- src/core/service.c | 8 -------- src/core/socket.c | 10 ---------- 2 files changed, 18 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 48462b5d18..6b72d48ccd 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2207,7 +2207,6 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { s->control_command = s->exec_command[SERVICE_EXEC_STOP_POST]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_STOP_POST; - pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2320,7 +2319,6 @@ static void service_enter_stop(Service *s, ServiceResult f) { s->control_command = s->exec_command[SERVICE_EXEC_STOP]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_STOP; - pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2406,7 +2404,6 @@ static void service_enter_start_post(Service *s) { s->control_command = s->exec_command[SERVICE_EXEC_START_POST]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_START_POST; - pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2540,7 +2537,6 @@ static void service_enter_start(Service *s) { case SERVICE_FORKING: /* For forking services we wait until the start process exited. */ - pidref_done(&s->control_pid); s->control_pid = TAKE_PIDREF(pidref); return service_set_state(s, SERVICE_START); @@ -2614,7 +2610,6 @@ static void service_enter_condition(Service *s) { goto fail; s->control_command_id = SERVICE_EXEC_CONDITION; - pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2735,7 +2730,6 @@ static void service_enter_reload_signal_exec(Service *s) { s->control_command = s->exec_command[SERVICE_EXEC_RELOAD]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_RELOAD; - pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2898,8 +2892,6 @@ static void service_run_next_control(Service *s) { else timeout = s->timeout_stop_usec; - pidref_done(&s->control_pid); - r = service_spawn(s, s->control_command, service_exec_flags(s->control_command_id, /* cred_flag = */ 0), diff --git a/src/core/socket.c b/src/core/socket.c index d57e2302de..cc339d631d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2148,8 +2148,6 @@ static void socket_enter_stop_post(Socket *s, SocketResult f) { s->control_command = s->exec_command[SOCKET_EXEC_STOP_POST]; if (s->control_command) { - pidref_done(&s->control_pid); - r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop-post' task: %m"); @@ -2226,8 +2224,6 @@ static void socket_enter_stop_pre(Socket *s, SocketResult f) { s->control_command = s->exec_command[SOCKET_EXEC_STOP_PRE]; if (s->control_command) { - pidref_done(&s->control_pid); - r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop-pre' task: %m"); @@ -2289,8 +2285,6 @@ static void socket_enter_start_post(Socket *s) { s->control_command = s->exec_command[SOCKET_EXEC_START_POST]; if (s->control_command) { - pidref_done(&s->control_pid); - r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-post' task: %m"); @@ -2363,8 +2357,6 @@ static void socket_enter_start_pre(Socket *s) { s->control_command = s->exec_command[SOCKET_EXEC_START_PRE]; if (s->control_command) { - pidref_done(&s->control_pid); - r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-pre' task: %m"); @@ -2612,8 +2604,6 @@ static void socket_run_next(Socket *s) { s->control_command = s->control_command->command_next; - pidref_done(&s->control_pid); - r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn next task: %m"); From 1b272d778a5e8a0ba30f422da4b2939912ef47f9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 20:27:42 +0200 Subject: [PATCH 09/21] core/service: introduce SERVICE_STATE_WITH_WATCHDOG --- src/core/service.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 6b72d48ccd..df89534a45 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -151,6 +151,14 @@ static bool SERVICE_STATE_WITH_CONTROL_PROCESS(ServiceState state) { SERVICE_CLEANING); } +static bool SERVICE_STATE_WITH_WATCHDOG(ServiceState state) { + return IN_SET(state, + SERVICE_START_POST, + SERVICE_RUNNING, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, + SERVICE_MOUNTING); +} + static void service_init(Unit *u) { Service *s = SERVICE(u); @@ -1319,7 +1327,7 @@ static void service_set_state(Service *s, ServiceState state) { if (state != SERVICE_START) s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source); - if (!IN_SET(state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, SERVICE_MOUNTING)) + if (!SERVICE_STATE_WITH_WATCHDOG(state)) service_stop_watchdog(s); if (state != SERVICE_MOUNTING) /* Just in case */ @@ -1431,7 +1439,7 @@ static int service_coldplug(Unit *u) { SERVICE_DEAD_RESOURCES_PINNED)) (void) unit_setup_exec_runtime(u); - if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, SERVICE_MOUNTING) && + if (SERVICE_STATE_WITH_WATCHDOG(s->deserialized_state) && freezer_state_objective(u->freezer_state) == FREEZER_RUNNING) service_start_watchdog(s); From 1b5a9a7d569a246c46537ba4bb2b5edc6463e784 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:26:48 +0200 Subject: [PATCH 10/21] core/service: avoid duplicate unit_add_to_dbus_queue() call If we're changing state anyways, service_set_state() -> unit_notify() will take care of dbus signaling for us. --- src/core/service.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index df89534a45..49c4b52af1 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4806,8 +4806,6 @@ static void service_notify_message( if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS)) service_enter_stop_by_notify(s); - notify_dbus = true; - } else if (strv_contains(tags, "READY=1")) { s->notify_state = NOTIFY_READY; @@ -4842,8 +4840,6 @@ static void service_notify_message( if (s->state == SERVICE_RELOAD_NOTIFY) service_enter_running(s, SERVICE_SUCCESS); - notify_dbus = true; - } else if (strv_contains(tags, "RELOADING=1")) { s->notify_state = NOTIFY_RELOADING; @@ -4861,8 +4857,6 @@ static void service_notify_message( if (s->state == SERVICE_RUNNING) service_enter_reload_by_notify(s); - - notify_dbus = true; } /* Interpret STATUS= */ From 38ea58a907a84573d6064b72613e4ed0124162a4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:47:54 +0200 Subject: [PATCH 11/21] core/service: remove effectively unused NOTIFY_UNKNOWN state We usually use _INVALID enum value as placeholder. While at it, reset notify_state in service_enter_dead() for consistency. --- src/core/service.c | 7 ++++--- src/core/service.h | 1 - 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 49c4b52af1..a8c6883294 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -185,6 +185,7 @@ static void service_init(Unit *u) { EXEC_KEYRING_PRIVATE : EXEC_KEYRING_INHERIT; s->notify_access_override = _NOTIFY_ACCESS_INVALID; + s->notify_state = _NOTIFY_STATE_INVALID; s->watchdog_original_usec = USEC_INFINITY; @@ -2177,8 +2178,9 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) /* The next restart might not be a manual stop, hence reset the flag indicating manual stops */ s->forbid_restart = false; - /* Reset NotifyAccess override */ + /* Reset notify states */ s->notify_access_override = _NOTIFY_ACCESS_INVALID; + s->notify_state = _NOTIFY_STATE_INVALID; /* We want fresh tmpdirs and ephemeral snapshots in case the service is started again immediately. */ s->exec_runtime = exec_runtime_destroy(s->exec_runtime); @@ -2983,7 +2985,7 @@ static int service_start(Unit *u) { s->status_varlink_error = mfree(s->status_varlink_error); s->notify_access_override = _NOTIFY_ACCESS_INVALID; - s->notify_state = NOTIFY_UNKNOWN; + s->notify_state = _NOTIFY_STATE_INVALID; s->watchdog_original_usec = s->watchdog_usec; s->watchdog_override_enable = false; @@ -5745,7 +5747,6 @@ static const char* const service_exec_ex_command_table[_SERVICE_EXEC_COMMAND_MAX DEFINE_STRING_TABLE_LOOKUP(service_exec_ex_command, ServiceExecCommand); static const char* const notify_state_table[_NOTIFY_STATE_MAX] = { - [NOTIFY_UNKNOWN] = "unknown", [NOTIFY_READY] = "ready", [NOTIFY_RELOADING] = "reloading", [NOTIFY_STOPPING] = "stopping", diff --git a/src/core/service.h b/src/core/service.h index 8965dc827b..e27b382e80 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -53,7 +53,6 @@ typedef enum ServiceExecCommand { } ServiceExecCommand; typedef enum NotifyState { - NOTIFY_UNKNOWN, NOTIFY_READY, NOTIFY_RELOADING, NOTIFY_STOPPING, From 3d888e2dadae56a3d4423ea2252a2f8da3d3b6b2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:56:33 +0200 Subject: [PATCH 12/21] core/service: add missing serialization for notify_state This really should be persisted across daemon-reload since it might contain deferred state transitions. --- src/core/service.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/service.c b/src/core/service.c index a8c6883294..644c9e4155 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3298,6 +3298,8 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { if (s->notify_access_override >= 0) (void) serialize_item(f, "notify-access-override", notify_access_to_string(s->notify_access_override)); + if (s->notify_state >= 0) + (void) serialize_item(f, "notify-state", notify_state_to_string(s->notify_state)); r = serialize_item_escaped(f, "status-text", s->status_text); if (r < 0) @@ -3608,6 +3610,16 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, log_unit_debug(u, "Failed to parse notify-access-override value: %s", value); else s->notify_access_override = notify_access; + + } else if (streq(key, "notify-state")) { + NotifyState notify_state; + + notify_state = notify_state_from_string(value); + if (notify_state < 0) + log_unit_debug(u, "Failed to parse notify-state value: %s", value); + else + s->notify_state = notify_state; + } else if (streq(key, "n-restarts")) { r = safe_atou(value, &s->n_restarts); if (r < 0) From ed834f11cbb5ffa2d861afd6b74c2cef574872a2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:32:59 +0200 Subject: [PATCH 13/21] core/service: split out service_notify_message_process_state() No functional change, preparation for later changes. --- src/core/service.c | 119 ++++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 56 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 644c9e4155..3722cc5e66 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4749,68 +4749,17 @@ finish: return 1; } -static void service_notify_message( - Unit *u, - PidRef *pidref, - const struct ucred *ucred, - char * const *tags, - FDSet *fds) { - - Service *s = ASSERT_PTR(SERVICE(u)); +static void service_notify_message_process_state(Service *s, char * const *tags) { + usec_t monotonic_usec = USEC_INFINITY; int r; - assert(pidref_is_set(pidref)); - assert(ucred); + assert(s); - if (!service_notify_message_authorized(s, pidref)) - return; - - if (DEBUG_LOGGING) { - _cleanup_free_ char *cc = strv_join(tags, ", "); - log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", pidref->pid, empty_to_na(cc)); - } - - usec_t monotonic_usec = USEC_INFINITY; - bool notify_dbus = false; - const char *e; - - /* Interpret MAINPID= (+ MAINPIDFDID=) / MAINPIDFD=1 */ - _cleanup_(pidref_done) PidRef new_main_pid = PIDREF_NULL; - - 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, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, - SERVICE_STOP, SERVICE_STOP_SIGTERM) && - (!s->main_pid_known || !pidref_equal(&new_main_pid, &s->main_pid))) { - - r = service_is_suitable_main_pid(s, &new_main_pid, LOG_WARNING); - if (r == 0) { - /* The new main PID is a bit suspicious, which is OK if the sender is privileged. */ - - if (ucred->uid == 0) { - log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, but we'll accept it as the request to change it came from a privileged process.", new_main_pid.pid); - r = 1; - } else - log_unit_warning(u, "New main PID "PID_FMT" does not belong to service, refusing.", new_main_pid.pid); - } - if (r > 0) { - (void) service_set_main_pidref(s, TAKE_PIDREF(new_main_pid), /* start_timestamp = */ NULL); - - r = unit_watch_pidref(UNIT(s), &s->main_pid, /* exclusive= */ false); - if (r < 0) - log_unit_warning_errno(UNIT(s), r, "Failed to watch new main PID "PID_FMT" for service: %m", s->main_pid.pid); - - notify_dbus = true; - } - } - - /* Parse MONOTONIC_USEC= */ - e = strv_find_startswith(tags, "MONOTONIC_USEC="); + const char *e = strv_find_startswith(tags, "MONOTONIC_USEC="); if (e) { r = safe_atou64(e, &monotonic_usec); if (r < 0) - log_unit_warning_errno(u, r, "Failed to parse MONOTONIC_USEC= field in notification message, ignoring: %s", e); + log_unit_warning_errno(UNIT(s), r, "Failed to parse MONOTONIC_USEC= field in notification message, ignoring: %s", e); } /* Interpret READY=/STOPPING=/RELOADING=. STOPPING= wins over the others, and READY= over RELOADING= */ @@ -4872,6 +4821,64 @@ static void service_notify_message( if (s->state == SERVICE_RUNNING) service_enter_reload_by_notify(s); } +} + +static void service_notify_message( + Unit *u, + PidRef *pidref, + const struct ucred *ucred, + char * const *tags, + FDSet *fds) { + + Service *s = ASSERT_PTR(SERVICE(u)); + int r; + + assert(pidref_is_set(pidref)); + assert(ucred); + + if (!service_notify_message_authorized(s, pidref)) + return; + + if (DEBUG_LOGGING) { + _cleanup_free_ char *cc = strv_join(tags, ", "); + log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", pidref->pid, empty_to_na(cc)); + } + + bool notify_dbus = false; + const char *e; + + /* Interpret MAINPID= (+ MAINPIDFDID=) / MAINPIDFD=1 */ + _cleanup_(pidref_done) PidRef new_main_pid = PIDREF_NULL; + + 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, + SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, + SERVICE_STOP, SERVICE_STOP_SIGTERM) && + (!s->main_pid_known || !pidref_equal(&new_main_pid, &s->main_pid))) { + + r = service_is_suitable_main_pid(s, &new_main_pid, LOG_WARNING); + if (r == 0) { + /* The new main PID is a bit suspicious, which is OK if the sender is privileged. */ + + if (ucred->uid == 0) { + log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, but we'll accept it as the request to change it came from a privileged process.", new_main_pid.pid); + r = 1; + } else + log_unit_warning(u, "New main PID "PID_FMT" does not belong to service, refusing.", new_main_pid.pid); + } + if (r > 0) { + (void) service_set_main_pidref(s, TAKE_PIDREF(new_main_pid), /* start_timestamp = */ NULL); + + r = unit_watch_pidref(UNIT(s), &s->main_pid, /* exclusive= */ false); + if (r < 0) + log_unit_warning_errno(UNIT(s), r, "Failed to watch new main PID "PID_FMT" for service: %m", s->main_pid.pid); + + notify_dbus = true; + } + } + + service_notify_message_process_state(s, tags); /* Interpret STATUS= */ e = strv_find_startswith(tags, "STATUS="); From 98734ac74d16862302c490e2a5bbc60539b7e6a4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 21:35:52 +0200 Subject: [PATCH 14/21] core/service: forbid reverting STOPPING=1 We don't permit state transition from STOPPING back to RUNNING, hence refrain from resetting notify_state too. --- src/core/service.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/core/service.c b/src/core/service.c index 3722cc5e66..bfd6022e0e 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4769,7 +4769,14 @@ static void service_notify_message_process_state(Service *s, char * const *tags) if (IN_SET(s->state, SERVICE_RUNNING, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS)) service_enter_stop_by_notify(s); - } else if (strv_contains(tags, "READY=1")) { + return; + } + + /* Disallow resurrecting a dying service */ + if (s->notify_state == NOTIFY_STOPPING) + return; + + if (strv_contains(tags, "READY=1")) { s->notify_state = NOTIFY_READY; From 584e89f26ed52efdfdf930e4ad9aba6adb779214 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 22:33:03 +0200 Subject: [PATCH 15/21] core/service: consolidate where to initialize reload_result --- src/core/service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index bfd6022e0e..af2c2b16bd 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2723,7 +2723,6 @@ static void service_enter_reload_signal_exec(Service *s) { assert(s); service_unwatch_control_pid(s); - s->reload_result = SERVICE_SUCCESS; usec_t ts = now(CLOCK_MONOTONIC); @@ -2816,7 +2815,6 @@ static void service_enter_refresh_extensions(Service *s) { return service_enter_reload_signal_exec(s); service_unwatch_control_pid(s); - s->reload_result = SERVICE_SUCCESS; s->control_command = NULL; s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; @@ -3098,6 +3096,8 @@ static int service_reload(Unit *u) { assert(IN_SET(s->state, SERVICE_RUNNING, SERVICE_EXITED)); + s->reload_result = SERVICE_SUCCESS; + service_enter_refresh_extensions(s); return 1; From 5fb8387c671011dd8dec9c897d2707fcb4d74298 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 22:34:42 +0200 Subject: [PATCH 16/21] core/service: reset all reload-related states once a cycle completes Fixes #37515 --- src/core/service.c | 61 ++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index af2c2b16bd..bc519514a1 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -128,6 +128,8 @@ static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void static int service_dispatch_exec_io(sd_event_source *source, int fd, uint32_t events, void *userdata); static void service_enter_signal(Service *s, ServiceState state, ServiceResult f); + +static void service_reload_finish(Service *s, ServiceResult f); static void service_enter_reload_by_notify(Service *s); static bool SERVICE_STATE_WITH_MAIN_PROCESS(ServiceState state) { @@ -2703,9 +2705,7 @@ static void service_enter_reload_by_notify(Service *s) { r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); - return; + return service_reload_finish(s, SERVICE_FAILURE_RESOURCES); } service_set_state(s, SERVICE_RELOAD_NOTIFY); @@ -2759,10 +2759,8 @@ static void service_enter_reload_signal_exec(Service *s) { } service_set_state(s, SERVICE_RELOAD_SIGNAL); - } else { - service_enter_running(s, SERVICE_SUCCESS); - return; - } + } else + return service_reload_finish(s, SERVICE_SUCCESS); /* Store the timestamp when we started reloading: when reloading via SIGHUP we won't leave the reload * state until we received both RELOADING=1 and READY=1 with MONOTONIC_USEC= set to a value above @@ -2772,8 +2770,7 @@ static void service_enter_reload_signal_exec(Service *s) { return; fail: - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); + service_reload_finish(s, SERVICE_FAILURE_RESOURCES); } static bool service_should_reload_extensions(Service *s) { @@ -2878,8 +2875,7 @@ static void service_enter_refresh_extensions(Service *s) { return; fail: - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); + service_reload_finish(s, SERVICE_FAILURE_RESOURCES); } static void service_run_next_control(Service *s) { @@ -2912,10 +2908,9 @@ static void service_run_next_control(Service *s) { service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); else if (s->state == SERVICE_STOP_POST) service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); - else if (s->state == SERVICE_RELOAD) { - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); - } else + else if (s->state == SERVICE_RELOAD) + service_reload_finish(s, SERVICE_FAILURE_RESOURCES); + else service_enter_stop(s, SERVICE_FAILURE_RESOURCES); } } @@ -2967,8 +2962,6 @@ static int service_start(Unit *u) { if (r < 0) return r; - s->result = SERVICE_SUCCESS; - s->reload_result = SERVICE_SUCCESS; s->main_pid_known = false; s->main_pid_alien = false; s->forbid_restart = false; @@ -2977,6 +2970,10 @@ static int service_start(Unit *u) { if (s->state != SERVICE_AUTO_RESTART_QUEUED) s->n_restarts = 0; + s->result = SERVICE_SUCCESS; + s->reload_result = SERVICE_SUCCESS; + s->reload_begin_usec = USEC_INFINITY; + s->status_text = mfree(s->status_text); s->status_errno = 0; s->status_bus_error = mfree(s->status_bus_error); @@ -3028,6 +3025,20 @@ static void service_live_mount_finish(Service *s, ServiceResult f, const char *e s->mount_request = sd_bus_message_unref(s->mount_request); } +static void service_reload_finish(Service *s, ServiceResult f) { + assert(s); + + s->reload_result = f; + s->reload_begin_usec = USEC_INFINITY; + + /* If notify state is still in dangling NOTIFY_RELOADING, reset it so service_enter_running() + * won't get confused (see #37515) */ + if (s->notify_state == NOTIFY_RELOADING) + s->notify_state = _NOTIFY_STATE_INVALID; + + service_enter_running(s, SERVICE_SUCCESS); +} + static int service_stop(Unit *u) { Service *s = ASSERT_PTR(SERVICE(u)); @@ -4330,16 +4341,15 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { if (service_load_pid_file(s, true) < 0) service_search_main_pid(s); - s->reload_result = f; - /* If the last notification we received from the service process indicates * we are still reloading, then don't leave reloading state just yet, just * transition into SERVICE_RELOAD_NOTIFY, to wait for the READY=1 coming, * too. */ - if (s->notify_state == NOTIFY_RELOADING) + if (s->notify_state == NOTIFY_RELOADING) { + s->reload_result = f; service_set_state(s, SERVICE_RELOAD_NOTIFY); - else - service_enter_running(s, SERVICE_SUCCESS); + } else + service_reload_finish(s, f); break; case SERVICE_REFRESH_EXTENSIONS: @@ -4448,8 +4458,7 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us case SERVICE_REFRESH_EXTENSIONS: log_unit_warning(UNIT(s), "Reload operation timed out. Killing reload process."); service_kill_control_process(s); - s->reload_result = SERVICE_FAILURE_TIMEOUT; - service_enter_running(s, SERVICE_SUCCESS); + service_reload_finish(s, SERVICE_FAILURE_TIMEOUT); break; case SERVICE_MOUNTING: @@ -4787,7 +4796,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) monotonic_usec != USEC_INFINITY && monotonic_usec >= s->reload_begin_usec) /* Valid Type=notify-reload protocol? Then we're all good. */ - service_enter_running(s, SERVICE_SUCCESS); + service_reload_finish(s, SERVICE_SUCCESS); else if (s->state == SERVICE_RUNNING) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -4808,7 +4817,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) /* Sending READY=1 while we are reloading informs us that the reloading is complete. */ if (s->state == SERVICE_RELOAD_NOTIFY) - service_enter_running(s, SERVICE_SUCCESS); + service_reload_finish(s, SERVICE_SUCCESS); } else if (strv_contains(tags, "RELOADING=1")) { From 1b4cf02ada5036c8dcde10807a97f13d6bc63a38 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 20 Oct 2025 01:42:57 +0200 Subject: [PATCH 17/21] core/service: fix missing error handling for refresh-extensions control process --- src/core/service.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index bc519514a1..8b2158c677 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4353,8 +4353,11 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { break; case SERVICE_REFRESH_EXTENSIONS: - /* Remounting extensions asynchronously done, proceed to signal */ - service_enter_reload_signal_exec(s); + if (f == SERVICE_SUCCESS) + /* Remounting extensions asynchronously done, proceed to signal */ + service_enter_reload_signal_exec(s); + else + service_reload_finish(s, f); break; case SERVICE_MOUNTING: From 48632305c76a2d6c1ae531c65c23274cb2969e5b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 20 Oct 2025 01:35:08 +0200 Subject: [PATCH 18/21] man/org.freedesktop.systemd1: fix typo (ExecStop -> -Post) --- man/org.freedesktop.systemd1.xml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index cb56d664c4..127ff76677 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -3539,8 +3539,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { - - @@ -4841,7 +4839,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { last watchdog ping received from the service, or 0 if none was ever received. ExecStartPre, ExecStart, ExecStartPost, - ExecReload, ExecStop, and ExecStop are arrays + ExecReload, ExecStop, and ExecStopPost are arrays of structures where each struct contains: the binary path to execute; an array with all arguments to pass to the executed command, starting with argument 0; a boolean whether it should be considered a failure if the process exits uncleanly; two pairs of @@ -5814,8 +5812,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { - - From b3c6709fde819be6cca7c075d8169cfd77865fa2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 20 Oct 2025 01:03:12 +0200 Subject: [PATCH 19/21] macro: add 21st case for IN_SET --- src/fundamental/macro-fundamental.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index ba2551cd9e..8fc23c5e6a 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -350,11 +350,12 @@ assert_cc(sizeof(long long) == sizeof(intmax_t)); #define CASE_F_18(X, ...) case X: CASE_F_17( __VA_ARGS__) #define CASE_F_19(X, ...) case X: CASE_F_18( __VA_ARGS__) #define CASE_F_20(X, ...) case X: CASE_F_19( __VA_ARGS__) +#define CASE_F_21(X, ...) case X: CASE_F_20( __VA_ARGS__) -#define GET_CASE_F(_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_11,_12,_13,_14,_15,_16,_17,_18,_19,_20,NAME,...) NAME +#define GET_CASE_F(_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_11,_12,_13,_14,_15,_16,_17,_18,_19,_20,_21,NAME,...) NAME #define FOR_EACH_MAKE_CASE(...) \ - GET_CASE_F(__VA_ARGS__,CASE_F_20,CASE_F_19,CASE_F_18,CASE_F_17,CASE_F_16,CASE_F_15,CASE_F_14,CASE_F_13,CASE_F_12,CASE_F_11, \ - CASE_F_10,CASE_F_9,CASE_F_8,CASE_F_7,CASE_F_6,CASE_F_5,CASE_F_4,CASE_F_3,CASE_F_2,CASE_F_1) \ + GET_CASE_F(__VA_ARGS__,CASE_F_21,CASE_F_20,CASE_F_19,CASE_F_18,CASE_F_17,CASE_F_16,CASE_F_15,CASE_F_14,CASE_F_13,CASE_F_12, \ + CASE_F_11,CASE_F_10,CASE_F_9,CASE_F_8,CASE_F_7,CASE_F_6,CASE_F_5,CASE_F_4,CASE_F_3,CASE_F_2,CASE_F_1) \ (__VA_ARGS__) #define IN_SET(x, first, ...) \ @@ -363,7 +364,7 @@ assert_cc(sizeof(long long) == sizeof(intmax_t)); /* If the build breaks in the line below, you need to extend the case macros. We use typeof(+x) \ * here to widen the type of x if it is a bit-field as this would otherwise be illegal. */ \ static const typeof(+x) __assert_in_set[] _unused_ = { first, __VA_ARGS__ }; \ - assert_cc(ELEMENTSOF(__assert_in_set) <= 20); \ + assert_cc(ELEMENTSOF(__assert_in_set) <= 21); \ switch (x) { \ FOR_EACH_MAKE_CASE(first, __VA_ARGS__) \ _found = true; \ From b03e1b09af7cf305c762db94c779f78cd85bca80 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 19 Oct 2025 23:23:17 +0200 Subject: [PATCH 20/21] core/service: rework ExecReload= + Type=notify-reload interaction, add ExecReloadPost= When Type=notify-reload got introduced, it wasn't intended to be mutually exclusive with ExecReload=. However, currently ExecReload= is immediately forked off after the service main process is signaled, leaving states in between essentially undefined. Given so broken it is I doubt any sane user is using this setup, hence I took a stab to rework everything: 1. Extensions are refreshed (unchanged) 2. ExecReload= is forked off without signaling the process 3a. If RELOADING=1 is sent during the ExecReload= invocation, we'd refrain from signaling the process again, instead just transition to SERVICE_RELOAD_NOTIFY directly and wait for READY=1 3b. If not, signal the process after ExecReload= finishes (from now on the same as Type=notify-reload w/o ExecReload=) 4. To accomodate the use case of performing post-reload tasks, ExecReloadPost= is introduced which executes after READY=1 The new model greatly simplifies things, as no control processes will be around in SERVICE_RELOAD_SIGNAL and SERVICE_RELOAD_NOTIFY states. See also: https://github.com/systemd/systemd/issues/37515#issuecomment-2891229652 --- man/org.freedesktop.systemd1.xml | 24 ++- man/systemd.service.xml | 49 +++--- src/basic/unit-def.c | 3 +- src/basic/unit-def.h | 3 +- src/core/dbus-service.c | 2 + src/core/load-fragment-gperf.gperf.in | 3 +- src/core/service.c | 225 ++++++++++++++++---------- src/core/service.h | 2 + src/shared/bus-unit-util.c | 2 + src/systemctl/systemctl-show.c | 2 + 10 files changed, 205 insertions(+), 110 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 127ff76677..f1f433bbac 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -2856,6 +2856,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") readonly a(sasasttttuii) ExecReloadEx = [...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") + readonly a(sasbttttuii) ExecReloadPost = [...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") + readonly a(sasasttttuii) ExecReloadPostEx = [...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") readonly a(sasbttttuii) ExecStop = [...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") readonly a(sasasttttuii) ExecStopEx = [...]; @@ -3537,6 +3541,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -4209,6 +4215,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + + @@ -4839,10 +4849,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { last watchdog ping received from the service, or 0 if none was ever received. ExecStartPre, ExecStart, ExecStartPost, - ExecReload, ExecStop, and ExecStopPost are arrays - of structures where each struct contains: the binary path to execute; an array with all arguments to - pass to the executed command, starting with argument 0; a boolean whether it should be considered a - failure if the process exits uncleanly; two pairs of + ExecReload, ExecReloadPost, ExecStop, and + ExecStopPost are arrays of structures where each struct contains: the binary path + to execute; an array with all arguments to pass to the executed command, starting with argument 0; + a boolean whether it should be considered a failure if the process exits uncleanly; two pairs of CLOCK_REALTIME/CLOCK_MONOTONIC microsecond timestamps when the process began and finished running the last time, or 0 if it never ran or never finished running; the PID of the process, or 0 if it has not run yet; the exit code and status of the last run. This @@ -12540,8 +12550,10 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ LogsDirectoryAccounting, and KillSubgroup() were added in version 258. UserNamespacePath, - OOMKills, and - ManagedOOMKills were added in 259. + OOMKills, + ManagedOOMKills, + ExecReloadPost, and + ExecReloadPostEx were added in version 259. Socket Unit Objects diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 8c00a093f8..2ddeeafcec 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -475,8 +475,8 @@ ExecReload= - Commands to execute to trigger a configuration reload in the service. This argument - takes multiple command lines, following the same scheme as described for + Commands to execute to trigger a configuration reload in the service. This setting + may take multiple command lines, following the same scheme as described for ExecStart= above. Use of this setting is optional. Specifier and environment variable substitution is supported here following the same scheme as for ExecStart=. @@ -486,13 +486,12 @@ ExecReload=kill -HUP $MAINPID - Note however that reloading a daemon by enqueuing a signal (as with the example line above) is - usually not a good choice, because this is an asynchronous operation and hence not suitable when - ordering reloads of multiple services against each other. It is thus strongly recommended to either - use Type= in place of - ExecReload=, or to set ExecReload= to a command that not only - triggers a configuration reload of the daemon, but also synchronously waits for it to complete. For - example, Note however that reloading a daemon by enqueuing a signal without completion notification + (as is the case with the example line above) is usually not a good choice, because this is an + asynchronous operation and hence not suitable when ordering reloads of multiple services against + each other. It is thus strongly recommended to either use Type=notify-reload, + or to set ExecReload= to a command that not only triggers a configuration reload + of the daemon, but also synchronously waits for it to complete. For example, dbus-broker1 uses the following: @@ -500,6 +499,22 @@ /org/freedesktop/DBus org.freedesktop.DBus \ ReloadConfig + + This setting can be combined with Type=notify-reload, in which case + the service main process is signaled after all specified command lines finish execution. Specially, + if RELOADING=1 notification is received before ExecReload= + completes, the signaling is skipped and the service manager immediately starts listening for + READY=1. + + + + + ExecReloadPost= + + Commands to execute after a successful reload operation. Syntax for this setting + is exactly the same as ExecReload=. + + @@ -1072,18 +1087,14 @@ RootDirectoryStartOnly= - Takes a boolean argument. If true, the root - directory, as configured with the + Takes a boolean argument. If true, the root directory, as configured with the RootDirectory= option (see systemd.exec5 - for more information), is only applied to the process started - with ExecStart=, and not to the various - other ExecStartPre=, - ExecStartPost=, - ExecReload=, ExecStop=, - and ExecStopPost= commands. If false, the - setting is applied to all configured commands the same way. - Defaults to false. + for more information), is only applied to the process started with ExecStart=, + and not to the various other ExecStartPre=, ExecStartPost=, + ExecReload=, ExecReloadPost=, ExecStop=, + and ExecStopPost= commands. If false, the setting is applied to all + configured commands the same way. Defaults to false. diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index a2b2f9d6f0..9179897f3c 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -224,10 +224,11 @@ static const char* const service_state_table[_SERVICE_STATE_MAX] = { [SERVICE_START_POST] = "start-post", [SERVICE_RUNNING] = "running", [SERVICE_EXITED] = "exited", + [SERVICE_REFRESH_EXTENSIONS] = "refresh-extensions", [SERVICE_RELOAD] = "reload", [SERVICE_RELOAD_SIGNAL] = "reload-signal", [SERVICE_RELOAD_NOTIFY] = "reload-notify", - [SERVICE_REFRESH_EXTENSIONS] = "refresh-extensions", + [SERVICE_RELOAD_POST] = "reload-post", [SERVICE_STOP] = "stop", [SERVICE_STOP_WATCHDOG] = "stop-watchdog", [SERVICE_STOP_SIGTERM] = "stop-sigterm", diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h index f42198e08e..37539ef018 100644 --- a/src/basic/unit-def.h +++ b/src/basic/unit-def.h @@ -131,10 +131,11 @@ typedef enum ServiceState { SERVICE_START_POST, SERVICE_RUNNING, SERVICE_EXITED, /* Nothing is running anymore, but RemainAfterExit is true hence this is OK */ + SERVICE_REFRESH_EXTENSIONS, /* Refreshing extensions for a reload request */ SERVICE_RELOAD, /* Reloading via ExecReload= */ SERVICE_RELOAD_SIGNAL, /* Reloading via SIGHUP requested */ SERVICE_RELOAD_NOTIFY, /* Waiting for READY=1 after RELOADING=1 notify */ - SERVICE_REFRESH_EXTENSIONS, /* Refreshing extensions for a reload request */ + SERVICE_RELOAD_POST, SERVICE_MOUNTING, /* Performing a live mount into the namespace of the service */ SERVICE_STOP, /* No STOP_PRE state, instead just register multiple STOP executables */ SERVICE_STOP_WATCHDOG, diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 2ff6272bd4..aa6a587c16 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -383,6 +383,8 @@ const sd_bus_vtable bus_service_vtable[] = { BUS_EXEC_EX_COMMAND_LIST_VTABLE("ExecStartPostEx", offsetof(Service, exec_command[SERVICE_EXEC_START_POST]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_COMMAND_LIST_VTABLE("ExecReload", offsetof(Service, exec_command[SERVICE_EXEC_RELOAD]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_EX_COMMAND_LIST_VTABLE("ExecReloadEx", offsetof(Service, exec_command[SERVICE_EXEC_RELOAD]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + BUS_EXEC_COMMAND_LIST_VTABLE("ExecReloadPost", offsetof(Service, exec_command[SERVICE_EXEC_RELOAD_POST]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + BUS_EXEC_EX_COMMAND_LIST_VTABLE("ExecReloadPostEx", offsetof(Service, exec_command[SERVICE_EXEC_RELOAD_POST]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_COMMAND_LIST_VTABLE("ExecStop", offsetof(Service, exec_command[SERVICE_EXEC_STOP]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_EX_COMMAND_LIST_VTABLE("ExecStopEx", offsetof(Service, exec_command[SERVICE_EXEC_STOP]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_COMMAND_LIST_VTABLE("ExecStopPost", offsetof(Service, exec_command[SERVICE_EXEC_STOP_POST]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index 5b83c95e4c..95ef508105 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -430,8 +430,9 @@ Service.PIDFile, config_parse_pid_file, Service.ExecCondition, config_parse_exec, SERVICE_EXEC_CONDITION, offsetof(Service, exec_command) Service.ExecStartPre, config_parse_exec, SERVICE_EXEC_START_PRE, offsetof(Service, exec_command) Service.ExecStart, config_parse_exec, SERVICE_EXEC_START, offsetof(Service, exec_command) -Service.ExecReload, config_parse_exec, SERVICE_EXEC_RELOAD, offsetof(Service, exec_command) Service.ExecStartPost, config_parse_exec, SERVICE_EXEC_START_POST, offsetof(Service, exec_command) +Service.ExecReload, config_parse_exec, SERVICE_EXEC_RELOAD, offsetof(Service, exec_command) +Service.ExecReloadPost, config_parse_exec, SERVICE_EXEC_RELOAD_POST, offsetof(Service, exec_command) Service.ExecStop, config_parse_exec, SERVICE_EXEC_STOP, offsetof(Service, exec_command) Service.ExecStopPost, config_parse_exec, SERVICE_EXEC_STOP_POST, offsetof(Service, exec_command) Service.RestartSec, config_parse_sec, 0, offsetof(Service, restart_usec) diff --git a/src/core/service.c b/src/core/service.c index 8b2158c677..5c1e6189f5 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -68,10 +68,11 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_START_POST] = UNIT_ACTIVATING, [SERVICE_RUNNING] = UNIT_ACTIVE, [SERVICE_EXITED] = UNIT_ACTIVE, + [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, [SERVICE_RELOAD] = UNIT_RELOADING, [SERVICE_RELOAD_SIGNAL] = UNIT_RELOADING, [SERVICE_RELOAD_NOTIFY] = UNIT_RELOADING, - [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, + [SERVICE_RELOAD_POST] = UNIT_RELOADING, [SERVICE_MOUNTING] = UNIT_REFRESHING, [SERVICE_STOP] = UNIT_DEACTIVATING, [SERVICE_STOP_WATCHDOG] = UNIT_DEACTIVATING, @@ -100,10 +101,11 @@ static const UnitActiveState state_translation_table_idle[_SERVICE_STATE_MAX] = [SERVICE_START_POST] = UNIT_ACTIVE, [SERVICE_RUNNING] = UNIT_ACTIVE, [SERVICE_EXITED] = UNIT_ACTIVE, + [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, [SERVICE_RELOAD] = UNIT_RELOADING, [SERVICE_RELOAD_SIGNAL] = UNIT_RELOADING, [SERVICE_RELOAD_NOTIFY] = UNIT_RELOADING, - [SERVICE_REFRESH_EXTENSIONS] = UNIT_REFRESHING, + [SERVICE_RELOAD_POST] = UNIT_RELOADING, [SERVICE_MOUNTING] = UNIT_REFRESHING, [SERVICE_STOP] = UNIT_DEACTIVATING, [SERVICE_STOP_WATCHDOG] = UNIT_DEACTIVATING, @@ -136,7 +138,7 @@ static bool SERVICE_STATE_WITH_MAIN_PROCESS(ServiceState state) { return IN_SET(state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL); @@ -146,7 +148,7 @@ static bool SERVICE_STATE_WITH_CONTROL_PROCESS(ServiceState state) { return IN_SET(state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_POST, SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, @@ -157,7 +159,7 @@ static bool SERVICE_STATE_WITH_WATCHDOG(ServiceState state) { return IN_SET(state, SERVICE_START_POST, SERVICE_RUNNING, - SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_MOUNTING); } @@ -1302,7 +1304,7 @@ static void service_set_state(Service *s, ServiceState state) { if (!IN_SET(state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, @@ -1372,10 +1374,11 @@ static usec_t service_coldplug_timeout(Service *s) { case SERVICE_START_PRE: case SERVICE_START: case SERVICE_START_POST: + case SERVICE_REFRESH_EXTENSIONS: case SERVICE_RELOAD: case SERVICE_RELOAD_SIGNAL: case SERVICE_RELOAD_NOTIFY: - case SERVICE_REFRESH_EXTENSIONS: + case SERVICE_RELOAD_POST: case SERVICE_MOUNTING: return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_start_usec); @@ -2716,25 +2719,87 @@ static void service_enter_reload_by_notify(Service *s) { log_unit_warning(UNIT(s), "Failed to schedule propagation of reload, ignoring: %s", bus_error_message(&error, r)); } -static void service_enter_reload_signal_exec(Service *s) { - bool killed = false; +static void service_enter_reload_post(Service *s) { int r; assert(s); service_unwatch_control_pid(s); - usec_t ts = now(CLOCK_MONOTONIC); + s->control_command = s->exec_command[SERVICE_EXEC_RELOAD_POST]; + if (s->control_command) { + s->control_command_id = SERVICE_EXEC_RELOAD_POST; + + r = service_spawn(s, + s->control_command, + service_exec_flags(s->control_command_id, /* cred_flag = */ 0), + s->timeout_start_usec, + &s->control_pid); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'reload-post' task: %m"); + return service_reload_finish(s, SERVICE_FAILURE_RESOURCES); + } + + service_set_state(s, SERVICE_RELOAD_POST); + } else + service_reload_finish(s, SERVICE_SUCCESS); +} + +static void service_enter_reload_signal(Service *s) { + int r; + + assert(s); + + if (s->type != SERVICE_NOTIFY_RELOAD) + return service_enter_reload_post(s); + + if (s->state == SERVICE_RELOAD) { + /* We executed ExecReload=, and the service has already notified us the result? + * Directly transition to next state. */ + if (s->notify_state == NOTIFY_RELOADING) + return service_set_state(s, SERVICE_RELOAD_NOTIFY); + if (s->notify_state == NOTIFY_RELOAD_READY) + return service_enter_reload_post(s); + } + + if (pidref_is_set(&s->main_pid)) { + r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + goto fail; + } - if (s->type == SERVICE_NOTIFY_RELOAD && pidref_is_set(&s->main_pid)) { r = pidref_kill_and_sigcont(&s->main_pid, s->reload_signal); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to send reload signal: %m"); goto fail; } - killed = true; - } + service_set_state(s, SERVICE_RELOAD_SIGNAL); + } else + service_enter_reload_post(s); + + return; + +fail: + service_reload_finish(s, SERVICE_FAILURE_RESOURCES); +} + +static void service_enter_reload(Service *s) { + int r; + + assert(s); + + service_unwatch_control_pid(s); + + if (IN_SET(s->notify_state, NOTIFY_RELOADING, NOTIFY_RELOAD_READY)) + s->notify_state = _NOTIFY_STATE_INVALID; + + /* Store the timestamp when we started reloading: when reloading via SIGHUP we won't leave the reload + * state until we received both RELOADING=1 and READY=1 with MONOTONIC_USEC= set to a value above + * this. Thus we know for sure the reload cycle was executed *after* we requested it, and is not one + * that was already in progress before. */ + s->reload_begin_usec = now(CLOCK_MONOTONIC); s->control_command = s->exec_command[SERVICE_EXEC_RELOAD]; if (s->control_command) { @@ -2747,30 +2812,12 @@ static void service_enter_reload_signal_exec(Service *s) { &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'reload' task: %m"); - goto fail; + return service_reload_finish(s, SERVICE_FAILURE_RESOURCES); } service_set_state(s, SERVICE_RELOAD); - } else if (killed) { - r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); - if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); - goto fail; - } - - service_set_state(s, SERVICE_RELOAD_SIGNAL); } else - return service_reload_finish(s, SERVICE_SUCCESS); - - /* Store the timestamp when we started reloading: when reloading via SIGHUP we won't leave the reload - * state until we received both RELOADING=1 and READY=1 with MONOTONIC_USEC= set to a value above - * this. Thus we know for sure the reload cycle was executed *after* we requested it, and is not one - * that was already in progress before. */ - s->reload_begin_usec = ts; - return; - -fail: - service_reload_finish(s, SERVICE_FAILURE_RESOURCES); + service_enter_reload_signal(s); } static bool service_should_reload_extensions(Service *s) { @@ -2807,9 +2854,9 @@ static void service_enter_refresh_extensions(Service *s) { assert(s); - /* If we don't have extensions to reload, immediately go to the signal step */ + /* If we don't have extensions to refresh, immediately transition to reload state */ if (!service_should_reload_extensions(s)) - return service_enter_reload_signal_exec(s); + return service_enter_reload(s); service_unwatch_control_pid(s); s->control_command = NULL; @@ -2891,7 +2938,11 @@ 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_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) + if (IN_SET(s->state, + SERVICE_CONDITION, + SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, + SERVICE_RUNNING, + SERVICE_RELOAD, SERVICE_RELOAD_POST)) timeout = s->timeout_start_usec; else timeout = s->timeout_stop_usec; @@ -2908,7 +2959,7 @@ static void service_run_next_control(Service *s) { service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); else if (s->state == SERVICE_STOP_POST) service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); - else if (s->state == SERVICE_RELOAD) + else if (IN_SET(s->state, SERVICE_RELOAD, SERVICE_RELOAD_POST)) service_reload_finish(s, SERVICE_FAILURE_RESOURCES); else service_enter_stop(s, SERVICE_FAILURE_RESOURCES); @@ -3076,6 +3127,7 @@ static int service_stop(Unit *u) { case SERVICE_RELOAD: case SERVICE_RELOAD_SIGNAL: case SERVICE_RELOAD_NOTIFY: + case SERVICE_RELOAD_POST: case SERVICE_STOP_WATCHDOG: /* If there's already something running we go directly into kill mode. */ service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_SUCCESS); @@ -4128,10 +4180,11 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { switch (s->state) { case SERVICE_START_POST: + case SERVICE_REFRESH_EXTENSIONS: case SERVICE_RELOAD: case SERVICE_RELOAD_SIGNAL: case SERVICE_RELOAD_NOTIFY: - case SERVICE_REFRESH_EXTENSIONS: + case SERVICE_RELOAD_POST: case SERVICE_MOUNTING: /* If neither main nor control processes are running then the current * state can never exit cleanly, hence immediately terminate the @@ -4247,7 +4300,8 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { success, code, status); - if (!IN_SET(s->state, SERVICE_RELOAD, SERVICE_MOUNTING) && s->result == SERVICE_SUCCESS) + if (!IN_SET(s->state, SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_POST, SERVICE_MOUNTING) && + s->result == SERVICE_SUCCESS) s->result = f; if (s->control_command && @@ -4334,30 +4388,28 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { service_enter_running(s, SERVICE_SUCCESS); break; - case SERVICE_RELOAD: - case SERVICE_RELOAD_SIGNAL: - case SERVICE_RELOAD_NOTIFY: + case SERVICE_REFRESH_EXTENSIONS: if (f == SERVICE_SUCCESS) - if (service_load_pid_file(s, true) < 0) - service_search_main_pid(s); - - /* If the last notification we received from the service process indicates - * we are still reloading, then don't leave reloading state just yet, just - * transition into SERVICE_RELOAD_NOTIFY, to wait for the READY=1 coming, - * too. */ - if (s->notify_state == NOTIFY_RELOADING) { - s->reload_result = f; - service_set_state(s, SERVICE_RELOAD_NOTIFY); - } else + /* Remounting extensions asynchronously done, proceed to reload */ + service_enter_reload(s); + else service_reload_finish(s, f); break; - case SERVICE_REFRESH_EXTENSIONS: - if (f == SERVICE_SUCCESS) - /* Remounting extensions asynchronously done, proceed to signal */ - service_enter_reload_signal_exec(s); - else + case SERVICE_RELOAD: + if (f != SERVICE_SUCCESS) { service_reload_finish(s, f); + break; + } + + if (service_load_pid_file(s, true) < 0) + service_search_main_pid(s); + + service_enter_reload_signal(s); + break; + + case SERVICE_RELOAD_POST: + service_reload_finish(s, f); break; case SERVICE_MOUNTING: @@ -4455,10 +4507,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_REFRESH_EXTENSIONS: case SERVICE_RELOAD: case SERVICE_RELOAD_SIGNAL: case SERVICE_RELOAD_NOTIFY: - case SERVICE_REFRESH_EXTENSIONS: + case SERVICE_RELOAD_POST: log_unit_warning(UNIT(s), "Reload operation timed out. Killing reload process."); service_kill_control_process(s); service_reload_finish(s, SERVICE_FAILURE_TIMEOUT); @@ -4790,7 +4843,10 @@ static void service_notify_message_process_state(Service *s, char * const *tags) if (strv_contains(tags, "READY=1")) { - s->notify_state = NOTIFY_READY; + if (s->notify_state == NOTIFY_RELOADING) + s->notify_state = NOTIFY_RELOAD_READY; + else + s->notify_state = NOTIFY_READY; /* Combined RELOADING=1 and READY=1? Then this is indication that the service started and * immediately finished reloading. */ @@ -4799,7 +4855,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) monotonic_usec != USEC_INFINITY && monotonic_usec >= s->reload_begin_usec) /* Valid Type=notify-reload protocol? Then we're all good. */ - service_reload_finish(s, SERVICE_SUCCESS); + service_enter_reload_post(s); else if (s->state == SERVICE_RUNNING) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -4820,7 +4876,7 @@ static void service_notify_message_process_state(Service *s, char * const *tags) /* Sending READY=1 while we are reloading informs us that the reloading is complete. */ if (s->state == SERVICE_RELOAD_NOTIFY) - service_reload_finish(s, SERVICE_SUCCESS); + service_enter_reload_post(s); } else if (strv_contains(tags, "RELOADING=1")) { @@ -4872,7 +4928,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, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_RELOAD_POST, SERVICE_STOP, SERVICE_STOP_SIGTERM) && (!s->main_pid_known || !pidref_equal(&new_main_pid, &s->main_pid))) { @@ -5140,10 +5196,11 @@ static bool pick_up_pid_from_bus_name(Service *s) { SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, - SERVICE_REFRESH_EXTENSIONS, + SERVICE_RELOAD_POST, SERVICE_MOUNTING); } @@ -5325,10 +5382,11 @@ static bool service_needs_console(Unit *u) { SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, + SERVICE_REFRESH_EXTENSIONS, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, - SERVICE_REFRESH_EXTENSIONS, + SERVICE_RELOAD_POST, SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, @@ -5761,33 +5819,36 @@ static const char* const service_exit_type_table[_SERVICE_EXIT_TYPE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(service_exit_type, ServiceExitType); static const char* const service_exec_command_table[_SERVICE_EXEC_COMMAND_MAX] = { - [SERVICE_EXEC_CONDITION] = "ExecCondition", - [SERVICE_EXEC_START_PRE] = "ExecStartPre", - [SERVICE_EXEC_START] = "ExecStart", - [SERVICE_EXEC_START_POST] = "ExecStartPost", - [SERVICE_EXEC_RELOAD] = "ExecReload", - [SERVICE_EXEC_STOP] = "ExecStop", - [SERVICE_EXEC_STOP_POST] = "ExecStopPost", + [SERVICE_EXEC_CONDITION] = "ExecCondition", + [SERVICE_EXEC_START_PRE] = "ExecStartPre", + [SERVICE_EXEC_START] = "ExecStart", + [SERVICE_EXEC_START_POST] = "ExecStartPost", + [SERVICE_EXEC_RELOAD] = "ExecReload", + [SERVICE_EXEC_RELOAD_POST] = "ExecReloadPost", + [SERVICE_EXEC_STOP] = "ExecStop", + [SERVICE_EXEC_STOP_POST] = "ExecStopPost", }; DEFINE_STRING_TABLE_LOOKUP(service_exec_command, ServiceExecCommand); static const char* const service_exec_ex_command_table[_SERVICE_EXEC_COMMAND_MAX] = { - [SERVICE_EXEC_CONDITION] = "ExecConditionEx", - [SERVICE_EXEC_START_PRE] = "ExecStartPreEx", - [SERVICE_EXEC_START] = "ExecStartEx", - [SERVICE_EXEC_START_POST] = "ExecStartPostEx", - [SERVICE_EXEC_RELOAD] = "ExecReloadEx", - [SERVICE_EXEC_STOP] = "ExecStopEx", - [SERVICE_EXEC_STOP_POST] = "ExecStopPostEx", + [SERVICE_EXEC_CONDITION] = "ExecConditionEx", + [SERVICE_EXEC_START_PRE] = "ExecStartPreEx", + [SERVICE_EXEC_START] = "ExecStartEx", + [SERVICE_EXEC_START_POST] = "ExecStartPostEx", + [SERVICE_EXEC_RELOAD] = "ExecReloadEx", + [SERVICE_EXEC_RELOAD_POST] = "ExecReloadPostEx", + [SERVICE_EXEC_STOP] = "ExecStopEx", + [SERVICE_EXEC_STOP_POST] = "ExecStopPostEx", }; DEFINE_STRING_TABLE_LOOKUP(service_exec_ex_command, ServiceExecCommand); static const char* const notify_state_table[_NOTIFY_STATE_MAX] = { - [NOTIFY_READY] = "ready", - [NOTIFY_RELOADING] = "reloading", - [NOTIFY_STOPPING] = "stopping", + [NOTIFY_READY] = "ready", + [NOTIFY_RELOADING] = "reloading", + [NOTIFY_RELOAD_READY] = "reload-ready", + [NOTIFY_STOPPING] = "stopping", }; DEFINE_STRING_TABLE_LOOKUP(notify_state, NotifyState); diff --git a/src/core/service.h b/src/core/service.h index e27b382e80..06fafb5482 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -46,6 +46,7 @@ typedef enum ServiceExecCommand { SERVICE_EXEC_START, SERVICE_EXEC_START_POST, SERVICE_EXEC_RELOAD, + SERVICE_EXEC_RELOAD_POST, SERVICE_EXEC_STOP, SERVICE_EXEC_STOP_POST, _SERVICE_EXEC_COMMAND_MAX, @@ -55,6 +56,7 @@ typedef enum ServiceExecCommand { typedef enum NotifyState { NOTIFY_READY, NOTIFY_RELOADING, + NOTIFY_RELOAD_READY, NOTIFY_STOPPING, _NOTIFY_STATE_MAX, _NOTIFY_STATE_INVALID = -EINVAL, diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 30a029716d..3d49cf5415 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2649,6 +2649,8 @@ static const BusProperty service_properties[] = { { "ExecStartPostEx", bus_append_exec_command }, /* compat */ { "ExecReload", bus_append_exec_command }, { "ExecReloadEx", bus_append_exec_command }, /* compat */ + { "ExecReloadPost", bus_append_exec_command }, + { "ExecReloadPostEx", bus_append_exec_command }, /* compat */ { "ExecStop", bus_append_exec_command }, { "ExecStopEx", bus_append_exec_command }, /* compat */ { "ExecStopPost", bus_append_exec_command }, diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 79e1908a23..f1c1ab0eff 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -2284,6 +2284,8 @@ static int show_one( { "ExecStartPostEx", "a(sasasttttuii)", map_exec, 0 }, { "ExecReload", "a(sasbttttuii)", map_exec, 0 }, { "ExecReloadEx", "a(sasasttttuii)", map_exec, 0 }, + { "ExecReloadPost", "a(sasbttttuii)", map_exec, 0 }, + { "ExecReloadPostEx", "a(sasasttttuii)", map_exec, 0 }, { "ExecStopPre", "a(sasbttttuii)", map_exec, 0 }, { "ExecStop", "a(sasbttttuii)", map_exec, 0 }, { "ExecStopEx", "a(sasasttttuii)", map_exec, 0 }, From ca8658120e1c9993bc05aa08dac2c74e618c2118 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 22 Oct 2025 01:34:04 +0200 Subject: [PATCH 21/21] TEST-80-NOTIFYACCESS: add test case for #37515 --- .../reload-timeout.service | 4 ++ .../reload-timeout.sh | 40 +++++++++++++++++++ test/units/TEST-80-NOTIFYACCESS.sh | 27 +++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.service create mode 100755 test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.sh diff --git a/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.service b/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.service new file mode 100644 index 0000000000..95a501e37a --- /dev/null +++ b/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.service @@ -0,0 +1,4 @@ +[Service] +Type=notify-reload +TimeoutStartSec=40 +ExecStart=/usr/lib/systemd/tests/testdata/TEST-80-NOTIFYACCESS.units/reload-timeout.sh diff --git a/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.sh b/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.sh new file mode 100755 index 0000000000..266c3cfef2 --- /dev/null +++ b/test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/reload-timeout.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +# shellcheck disable=SC2016 +set -eux +set -o pipefail + +COUNTER=0 + +sync_in() { + read -r x < /tmp/syncfifo2 + test "$x" = "$1" +} + +wait_for_signal() { + sleep infinity & + wait "$!" || : +} + +sighup_handler() { + echo "hup$(( ++COUNTER ))" > /tmp/syncfifo1 +} + +trap sighup_handler SIGHUP + +export SYSTEMD_LOG_LEVEL=debug + +systemd-notify --ready + +wait_for_signal +systemd-notify --reloading + +wait_for_signal +systemd-notify --reloading +sync_in ready +systemd-notify --ready + +wait_for_signal +systemd-notify --reloading --ready + +exec sleep infinity diff --git a/test/units/TEST-80-NOTIFYACCESS.sh b/test/units/TEST-80-NOTIFYACCESS.sh index a8ecd9fa70..22a5a3af80 100755 --- a/test/units/TEST-80-NOTIFYACCESS.sh +++ b/test/units/TEST-80-NOTIFYACCESS.sh @@ -62,6 +62,33 @@ assert_eq "$(systemctl show notify.service -p NotifyAccess --value)" "none" systemctl stop notify.service assert_eq "$(systemctl show notify.service -p NotifyAccess --value)" "all" +# Timeout of READY=1 for Type=notify-reload services (issue #37515) + +systemctl start reload-timeout.service + +systemctl reload --no-block reload-timeout.service +timeout 10 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "reload-signal" ]]; do sleep .5; done' +sync_in hup1 +timeout 10 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "reload-notify" ]]; do sleep .5; done' +timeout 80 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "running" ]]; do sleep 5; done' +assert_eq "$(systemctl show reload-timeout.service -P ReloadResult)" "timeout" + +systemctl reload --no-block reload-timeout.service +timeout 10 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "reload-signal" ]]; do sleep .5; done' +assert_eq "$(systemctl show reload-timeout.service -P ReloadResult)" "success" +sync_in hup2 +timeout 10 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "reload-notify" ]]; do sleep .5; done' +sync_out ready +timeout 40 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "running" ]]; do sleep 1; done' +assert_eq "$(systemctl show reload-timeout.service -P ReloadResult)" "success" + +systemctl reload --no-block reload-timeout.service +sync_in hup3 +timeout 40 bash -c 'until [[ $(systemctl show reload-timeout.service -P SubState) == "running" ]]; do sleep 1; done' +assert_eq "$(systemctl show reload-timeout.service -P ReloadResult)" "success" + +systemctl stop reload-timeout.service + rm /tmp/syncfifo1 /tmp/syncfifo2 # Explicitly test busctl's BUSERROR= reporting and systemctl status should show it