From 4c42543429b5c167a7900423f04f8294d1ec4650 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 May 2020 14:29:46 +0200 Subject: [PATCH] core: also log about left-over processes during unit stop Only log at LOG_INFO level, i.e. make this informational. During start let's leave it at LOG_WARNING though. Of course, it's ugly leaving processes around like that either in start or in stop, but at start its more dangerous than on stop, so be tougher there. --- src/core/mount.c | 4 +++- src/core/service.c | 12 +++++++----- src/core/socket.c | 4 +++- src/core/swap.c | 3 ++- src/core/unit.c | 31 +++++++++++++++++++++++++++---- src/core/unit.h | 4 +++- 6 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 48500b4932..10613d11ae 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -863,6 +863,8 @@ static void mount_enter_dead(Mount *m, MountResult f) { m->result = f; unit_log_result(UNIT(m), m->result == MOUNT_SUCCESS, mount_result_to_string(m->result)); + unit_warn_leftover_processes(UNIT(m), unit_log_leftover_process_stop); + mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD); m->exec_runtime = exec_runtime_unref(m->exec_runtime, true); @@ -1008,7 +1010,7 @@ static void mount_enter_mounting(Mount *m) { (void) mkdir_p_label(m->where, m->directory_mode); unit_warn_if_dir_nonempty(UNIT(m), m->where); - unit_warn_leftover_processes(UNIT(m)); + unit_warn_leftover_processes(UNIT(m), unit_log_leftover_process_start); m->control_command_id = MOUNT_EXEC_MOUNT; m->control_command = m->exec_command + MOUNT_EXEC_MOUNT; diff --git a/src/core/service.c b/src/core/service.c index 6e2a82a286..3e955edeff 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1755,6 +1755,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) unit_log_failure(UNIT(s), service_result_to_string(s->result)); end_state = SERVICE_FAILED; } + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); if (!allow_restart) log_unit_debug(UNIT(s), "Service restart not allowed."); @@ -2081,15 +2082,16 @@ static int service_adverse_to_leftover_processes(Service *s) { assert(s); /* KillMode=mixed and control group are used to indicate that all process should be killed off. - * SendSIGKILL is used for services that require a clean shutdown. These are typically database - * service where a SigKilled process would result in a lengthy recovery and who's shutdown or - * startup time is quite variable (so Timeout settings aren't of use). + * SendSIGKILL= is used for services that require a clean shutdown. These are typically database + * service where a SigKilled process would result in a lengthy recovery and who's shutdown or startup + * time is quite variable (so Timeout settings aren't of use). * * Here we take these two factors and refuse to start a service if there are existing processes * within a control group. Databases, while generally having some protection against multiple - * instances running, lets not stress the rigor of these. Also ExecStartPre parts of the service + * instances running, lets not stress the rigor of these. Also ExecStartPre= parts of the service * aren't as rigoriously written to protect aganst against multiple use. */ - if (unit_warn_leftover_processes(UNIT(s)) && + + if (unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start) > 0 && IN_SET(s->kill_context.kill_mode, KILL_MIXED, KILL_CONTROL_GROUP) && !s->kill_context.send_sigkill) return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(EBUSY), diff --git a/src/core/socket.c b/src/core/socket.c index 5e8f317bfc..218b4b245d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2035,6 +2035,8 @@ static void socket_enter_dead(Socket *s, SocketResult f) { else unit_log_failure(UNIT(s), socket_result_to_string(s->result)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); + socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD); s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); @@ -2237,7 +2239,7 @@ static void socket_enter_start_pre(Socket *s) { socket_unwatch_control_pid(s); - unit_warn_leftover_processes(UNIT(s)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start); s->control_command_id = SOCKET_EXEC_START_PRE; s->control_command = s->exec_command[SOCKET_EXEC_START_PRE]; diff --git a/src/core/swap.c b/src/core/swap.c index 0b42236aca..20179de2d2 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -701,6 +701,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) { s->result = f; unit_log_result(UNIT(s), s->result == SWAP_SUCCESS, swap_result_to_string(s->result)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD); s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); @@ -788,7 +789,7 @@ static void swap_enter_activating(Swap *s) { assert(s); - unit_warn_leftover_processes(UNIT(s)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start); s->control_command_id = SWAP_EXEC_ACTIVATE; s->control_command = s->exec_command + SWAP_EXEC_ACTIVATE; diff --git a/src/core/unit.c b/src/core/unit.c index c5eb72163b..d55cbea68a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5861,14 +5861,20 @@ int unit_prepare_exec(Unit *u) { return 0; } -static int log_leftover(pid_t pid, int sig, void *userdata) { +static bool ignore_leftover_process(const char *comm) { + return comm && comm[0] == '('; /* Most likely our own helper process (PAM?), ignore */ +} + +int unit_log_leftover_process_start(pid_t pid, int sig, void *userdata) { _cleanup_free_ char *comm = NULL; (void) get_process_comm(pid, &comm); - if (comm && comm[0] == '(') /* Most likely our own helper process (PAM?), ignore */ + if (ignore_leftover_process(comm)) return 0; + /* During start we print a warning */ + log_unit_warning(userdata, "Found left-over process " PID_FMT " (%s) in control group while starting unit. Ignoring.\n" "This usually indicates unclean termination of a previous run, or service implementation deficiencies.", @@ -5877,7 +5883,24 @@ static int log_leftover(pid_t pid, int sig, void *userdata) { return 1; } -int unit_warn_leftover_processes(Unit *u) { +int unit_log_leftover_process_stop(pid_t pid, int sig, void *userdata) { + _cleanup_free_ char *comm = NULL; + + (void) get_process_comm(pid, &comm); + + if (ignore_leftover_process(comm)) + return 0; + + /* During stop we only print an informational message */ + + log_unit_info(userdata, + "Unit process " PID_FMT " (%s) remains running after unit stopped.", + pid, strna(comm)); + + return 1; +} + +int unit_warn_leftover_processes(Unit *u, cg_kill_log_func_t log_func) { assert(u); (void) unit_pick_cgroup_path(u); @@ -5885,7 +5908,7 @@ int unit_warn_leftover_processes(Unit *u) { if (!u->cgroup_path) return 0; - return cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, 0, 0, NULL, log_leftover, u); + return cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, 0, 0, NULL, log_func, u); } bool unit_needs_console(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index bee68607fe..6659406c2d 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -846,7 +846,9 @@ void unit_unlink_state_files(Unit *u); int unit_prepare_exec(Unit *u); -int unit_warn_leftover_processes(Unit *u); +int unit_log_leftover_process_start(pid_t pid, int sig, void *userdata); +int unit_log_leftover_process_stop(pid_t pid, int sig, void *userdata); +int unit_warn_leftover_processes(Unit *u, cg_kill_log_func_t log_func); bool unit_needs_console(Unit *u);