From f3a269a9354261d55754963d568112b6ab94ba81 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 14:03:55 +0100 Subject: [PATCH] manager: process exec_fd (i.e. Type=exec) events before SIGCHLD events We want to make sure we don't confuse the case "process started successfully but then failed quickly" from the case "process failed to start". Hence we need to make sure we take notice of Type=exec before we bother with SIGCHLD. Hence move EVENT_PRIORITY_EXEC_FD to the front. In fact, let's move it even further up than SIGCHLD, i.e. before sd_notify() handling, so that we don't end up processing service state change notifications before we even considered that the service is properly started. This also gives the cgroup OOM handling and the exec_fd handling different priorities, to improve robustness of the system, we should act quickly on OOM, and it doesn't matter if a service started succcessfully if we have to act on OOM anyway. This is based on Andrew Onyshchuk work here: See: #30799 Fixes: #28304 --- src/core/manager.h | 10 +++++----- src/core/service.c | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/manager.h b/src/core/manager.h index cfcf1929f5..4193c10e4b 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -653,11 +653,11 @@ enum { EVENT_PRIORITY_CGROUP_AGENT = SD_EVENT_PRIORITY_NORMAL-9, EVENT_PRIORITY_CGROUP_INOTIFY = SD_EVENT_PRIORITY_NORMAL-9, EVENT_PRIORITY_CGROUP_OOM = SD_EVENT_PRIORITY_NORMAL-8, - EVENT_PRIORITY_NOTIFY = SD_EVENT_PRIORITY_NORMAL-8, - EVENT_PRIORITY_SIGCHLD = SD_EVENT_PRIORITY_NORMAL-7, - EVENT_PRIORITY_SIGNALS = SD_EVENT_PRIORITY_NORMAL-6, - EVENT_PRIORITY_CGROUP_EMPTY = SD_EVENT_PRIORITY_NORMAL-5, - EVENT_PRIORITY_EXEC_FD = SD_EVENT_PRIORITY_NORMAL-3, + EVENT_PRIORITY_EXEC_FD = SD_EVENT_PRIORITY_NORMAL-6, + EVENT_PRIORITY_NOTIFY = SD_EVENT_PRIORITY_NORMAL-5, + EVENT_PRIORITY_SIGCHLD = SD_EVENT_PRIORITY_NORMAL-4, + EVENT_PRIORITY_SIGNALS = SD_EVENT_PRIORITY_NORMAL-3, + EVENT_PRIORITY_CGROUP_EMPTY = SD_EVENT_PRIORITY_NORMAL-2, EVENT_PRIORITY_TIME_CHANGE = SD_EVENT_PRIORITY_NORMAL-1, EVENT_PRIORITY_TIME_ZONE = SD_EVENT_PRIORITY_NORMAL-1, EVENT_PRIORITY_IPC = SD_EVENT_PRIORITY_NORMAL, diff --git a/src/core/service.c b/src/core/service.c index 79859a2a1e..aacfe0d57e 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1512,7 +1512,8 @@ static int service_allocate_exec_fd_event_source( if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to allocate exec_fd event source: %m"); - /* This is a bit lower priority than SIGCHLD, as that carries a lot more interesting failure information */ + /* This is a bit higher priority than SIGCHLD, to make sure we don't confuse the case "failed to + * start" from the case "succeeded to start, but failed immediately after". */ r = sd_event_source_set_priority(source, EVENT_PRIORITY_EXEC_FD); if (r < 0) -- 2.25.1