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 51bba291e8..569a6871d6 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, @@ -138,6 +140,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, @@ -479,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); @@ -571,15 +575,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 && !SERVICE_FD_STORE_POPULATED(s)) + service_set_state(s, SERVICE_DEAD); + return 0; } @@ -2126,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) { @@ -3950,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; @@ -5612,7 +5623,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; } @@ -5865,7 +5876,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); }