From 32d02c924487c8ed3bc76cb5582ed5607a50345b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Jun 2024 12:08:39 +0200 Subject: [PATCH 1/2] core/unit: add one assertion for u->manager (cherry picked from commit 8b17371b6185c9829bb21a813aadb2225ccfc4de) Resolves: RHEL-55734 --- src/core/unit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/unit.c b/src/core/unit.c index a5556ba462..c668c45ee9 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6103,6 +6103,7 @@ int unit_test_trigger_loaded(Unit *u) { void unit_destroy_runtime_data(Unit *u, const ExecContext *context) { assert(u); + assert(u->manager); assert(context); /* EXEC_PRESERVE_RESTART is handled via unit_release_resources()! */ From a9ac0af16b00054863ae2eb9c37c5ffc44af20d4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Jun 2024 12:03:50 +0200 Subject: [PATCH 2/2] core/service: destroy runtime data when Type=oneshot services exit Currently, we have a bunch of Type=oneshot + RemainAfterExit=yes services that make use of credentials. When those exits, the cred mounts remain established, which is pointless and quite annoying. Let's instead destroy the runtime data on SERVICE_EXITED, if no process will be spawned for the unit again. (cherry picked from commit c26948c6dae1d2ca13499b36f193b13a0760834c) Resolves: RHEL-55734 --- src/core/service.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 6e81460ad0..60cc902745 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1206,13 +1206,12 @@ static void service_search_main_pid(Service *s) { } static void service_set_state(Service *s, ServiceState state) { + Unit *u = UNIT(ASSERT_PTR(s)); ServiceState old_state; const UnitActiveState *table; - assert(s); - if (s->state != state) - bus_unit_send_pending_change_signal(UNIT(s), false); + bus_unit_send_pending_change_signal(u, false); table = s->type == SERVICE_IDLE ? state_translation_table_idle : state_translation_table; @@ -1246,8 +1245,8 @@ static void service_set_state(Service *s, ServiceState state) { SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART, SERVICE_AUTO_RESTART, SERVICE_AUTO_RESTART_QUEUED, SERVICE_DEAD_RESOURCES_PINNED)) { - unit_unwatch_all_pids(UNIT(s)); - unit_dequeue_rewatch_pids(UNIT(s)); + unit_unwatch_all_pids(u); + unit_dequeue_rewatch_pids(u); } if (state != SERVICE_START) @@ -1256,15 +1255,31 @@ static void service_set_state(Service *s, ServiceState state) { if (!IN_SET(state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY)) service_stop_watchdog(s); - /* For the inactive states unit_notify() will trim the cgroup, - * but for exit we have to do that ourselves... */ - if (state == SERVICE_EXITED && !MANAGER_IS_RELOADING(UNIT(s)->manager)) - unit_prune_cgroup(UNIT(s)); + if (state == SERVICE_EXITED && !MANAGER_IS_RELOADING(u->manager)) { + /* For the inactive states unit_notify() will trim the cgroup. But for exit we have to + * do that ourselves... */ + unit_prune_cgroup(u); + + /* If none of ExecReload= and ExecStop*= is used, we can safely destroy runtime data + * as soon as the service enters SERVICE_EXITED. This saves us from keeping the credential mount + * for the whole duration of the oneshot service while no processes are actually running, + * among other things. */ + + bool start_only = true; + for (ServiceExecCommand c = SERVICE_EXEC_RELOAD; c < _SERVICE_EXEC_COMMAND_MAX; c++) + if (s->exec_command[c]) { + start_only = false; + break; + } + + if (start_only) + unit_destroy_runtime_data(u, &s->exec_context); + } if (old_state != state) - log_unit_debug(UNIT(s), "Changed %s -> %s", service_state_to_string(old_state), service_state_to_string(state)); + log_unit_debug(u, "Changed %s -> %s", service_state_to_string(old_state), service_state_to_string(state)); - unit_notify(UNIT(s), table[old_state], table[state], s->reload_result == SERVICE_SUCCESS); + unit_notify(u, table[old_state], table[state], s->reload_result == SERVICE_SUCCESS); } static usec_t service_coldplug_timeout(Service *s) {