From 501ece433b701d65c5dd484a22d3026bf872b877 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Apr 2026 12:08:56 +0100 Subject: [PATCH 1/3] service: transition unit from SERVICE_DEAD_RESOURCES_PINNED to SERVICE_DEAD when fd store is emptied Follow-up for b9c1883a9cd9b5126fe648f3e198143dc19a222d --- 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 51bba291e8..b511b422ac 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -138,6 +138,8 @@ static void service_enter_reload_by_notify(Service *s); static bool service_can_reload_extensions(Service *s, bool warn); +static void service_set_state(Service *s, ServiceState state); + static bool SERVICE_STATE_WITH_MAIN_PROCESS(ServiceState state) { return IN_SET(state, SERVICE_START, SERVICE_START_POST, @@ -571,15 +573,20 @@ static void service_done(Unit *u) { static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *userdata) { ServiceFDStore *fs = ASSERT_PTR(userdata); + Service *s = fs->service; assert(e); /* If we get either EPOLLHUP or EPOLLERR, it's time to remove this entry from the fd store */ - log_unit_debug(UNIT(fs->service), + log_unit_debug(UNIT(s), "Received %s on stored fd %d (%s), closing.", revents & EPOLLERR ? "EPOLLERR" : "EPOLLHUP", fs->fd, strna(fs->fdname)); service_fd_store_unlink(fs); + + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + service_set_state(s, SERVICE_DEAD); + return 0; } From 5a20987bef4455dc66ddc370e938bccc36b712eb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Apr 2026 12:10:39 +0100 Subject: [PATCH 2/3] service: add macro to check if FD store is empty Follow-up for b9c1883a9cd9b5126fe648f3e198143dc19a222d --- src/core/service.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index b511b422ac..f3e5a5f85b 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -60,6 +60,8 @@ #define service_spawn(...) service_spawn_internal(__func__, __VA_ARGS__) +#define SERVICE_FD_STORE_POPULATED(s) (!!(s)->fd_store) + static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_DEAD] = UNIT_INACTIVE, [SERVICE_CONDITION] = UNIT_ACTIVATING, @@ -481,12 +483,12 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(ServiceFDStore*, service_fd_store_unlink); static void service_release_fd_store(Service *s) { assert(s); - if (!s->fd_store) + if (!SERVICE_FD_STORE_POPULATED(s)) return; log_unit_debug(UNIT(s), "Releasing all stored fds."); - while (s->fd_store) + while (SERVICE_FD_STORE_POPULATED(s)) service_fd_store_unlink(s->fd_store); assert(s->n_fd_store == 0); @@ -584,7 +586,7 @@ static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *us fs->fd, strna(fs->fdname)); service_fd_store_unlink(fs); - if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !SERVICE_FD_STORE_POPULATED(s)) service_set_state(s, SERVICE_DEAD); return 0; @@ -2133,7 +2135,7 @@ static bool service_will_restart(Unit *u) { static ServiceState service_determine_dead_state(Service *s) { assert(s); - return s->fd_store && s->fd_store_preserve_mode == EXEC_PRESERVE_YES ? SERVICE_DEAD_RESOURCES_PINNED : SERVICE_DEAD; + return SERVICE_FD_STORE_POPULATED(s) && s->fd_store_preserve_mode == EXEC_PRESERVE_YES ? SERVICE_DEAD_RESOURCES_PINNED : SERVICE_DEAD; } static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) { @@ -5619,7 +5621,7 @@ static int service_clean(Unit *u, ExecCleanMask mask) { /* If we are done, leave quickly */ if (strv_isempty(l)) { - if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !SERVICE_FD_STORE_POPULATED(s)) service_set_state(s, SERVICE_DEAD); return 0; } @@ -5872,7 +5874,7 @@ static void service_release_resources(Unit *u) { if (s->fd_store_preserve_mode != EXEC_PRESERVE_YES) service_release_fd_store(s); - if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !SERVICE_FD_STORE_POPULATED(s)) service_set_state(s, SERVICE_DEAD); } From 55327a1fd859e8c168786059242306f1a74d4153 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Apr 2026 12:11:01 +0100 Subject: [PATCH 3/3] core: do not GC units that have FDs stored If a unit has FileDescriptorStorePreserve=yes we'll keep its FDs around in case it starts again. But if there are no reverse dependencies referencing it, we'll also GC it and lose all the FDs, which defeats the point of the setting (which is opt-in). Do not GC units that have FDs stored to avoid this. Follow-up for b9c1883a9cd9b5126fe648f3e198143dc19a222d --- man/systemd.service.xml | 7 ++++--- man/systemd.unit.xml | 5 +++++ src/core/service.c | 6 ++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 549f36af1d..d4a1978523 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -1250,9 +1250,10 @@ RestartMaxDelaySec=160s file descriptor store is automatically released when the service is stopped; if restart (the default) it is kept around as long as the unit is neither inactive nor failed, or a job is queued for the service, or the service is expected to be restarted. If - yes the file descriptor store is kept around until the unit is removed from - memory (i.e. is not referenced anymore and inactive). The latter is useful to keep entries in the - file descriptor store pinned until the service manager exits. + yes the file descriptor store is kept around and garbage collection of the unit + is disabled. The latter is useful to keep entries in the file descriptor store pinned until the unit + is removed, the service manager exits, or the file descriptors get EPOLLHUP or + EPOLLERR. Use systemctl clean --what=fdstore … to release the file descriptor store explicitly. diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 8bbff2f210..47c30d869f 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1082,6 +1082,11 @@ resources, …) are flushed out immediately after the unit completed, except for what is stored in the logging subsystem. Defaults to . + Since v261, if FileDescriptorStorePreserve= is set to , + and the unit has file descriptors stored, garbage collection will be disabled until the unit is + removed, the service manager exits, or the file descriptors get EPOLLHUP or + EPOLLERR. + diff --git a/src/core/service.c b/src/core/service.c index f3e5a5f85b..569a6871d6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3959,8 +3959,10 @@ static bool service_may_gc(Unit *u) { return false; /* Only allow collection of actually dead services, i.e. not those that are in the transitionary - * SERVICE_DEAD_BEFORE_AUTO_RESTART/SERVICE_FAILED_BEFORE_AUTO_RESTART states. */ - if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_RESOURCES_PINNED)) + * SERVICE_DEAD_BEFORE_AUTO_RESTART/SERVICE_FAILED_BEFORE_AUTO_RESTART states, and not those + * that still have resources pinned (fd store with FileDescriptorStorePreserve=yes) in case they are + * started again later despite not having any reverse dependency. */ + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)) return false; return true;