core: warn about left-over processes in cgroup on unit start

Now that we don't kill control processes anymore, let's at least warn
about any processes left-over in the unit cgroup at the moment of
starting the unit.

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 4208d1d..78ef885 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1394,6 +1394,31 @@ int unit_watch_cgroup(Unit *u) {
         return 0;
 }
 
+int unit_pick_cgroup_path(Unit *u) {
+        _cleanup_free_ char *path = NULL;
+        int r;
+
+        assert(u);
+
+        if (u->cgroup_path)
+                return 0;
+
+        if (!UNIT_HAS_CGROUP_CONTEXT(u))
+                return -EINVAL;
+
+        path = unit_default_cgroup_path(u);
+        if (!path)
+                return log_oom();
+
+        r = unit_set_cgroup_path(u, path);
+        if (r == -EEXIST)
+                return log_unit_error_errno(u, r, "Control group %s exists already.", path);
+        if (r < 0)
+                return log_unit_error_errno(u, r, "Failed to set unit's control group path to %s: %m", path);
+
+        return 0;
+}
+
 static int unit_create_cgroup(
                 Unit *u,
                 CGroupMask target_mask,
@@ -1409,19 +1434,10 @@ static int unit_create_cgroup(
         if (!c)
                 return 0;
 
-        if (!u->cgroup_path) {
-                _cleanup_free_ char *path = NULL;
-
-                path = unit_default_cgroup_path(u);
-                if (!path)
-                        return log_oom();
-
-                r = unit_set_cgroup_path(u, path);
-                if (r == -EEXIST)
-                        return log_unit_error_errno(u, r, "Control group %s exists already.", path);
-                if (r < 0)
-                        return log_unit_error_errno(u, r, "Failed to set unit's control group path to %s: %m", path);
-        }
+        /* Figure out our cgroup path */
+        r = unit_pick_cgroup_path(u);
+        if (r < 0)
+                return r;
 
         /* First, create our own group */
         r = cg_create_everywhere(u->manager->cgroup_supported, target_mask, u->cgroup_path);

diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 00df3bb..0c5bb4a 100644 (file)
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -169,6 +169,7 @@ void unit_update_cgroup_members_masks(Unit *u);
 
 char *unit_default_cgroup_path(Unit *u);
 int unit_set_cgroup_path(Unit *u, const char *path);
+int unit_pick_cgroup_path(Unit *u);
 
 int unit_realize_cgroup(Unit *u);
 void unit_release_cgroup(Unit *u);

diff --git a/src/core/mount.c b/src/core/mount.c
index 1500670..b25bb9c 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -938,9 +938,6 @@ static void mount_enter_mounting(Mount *m) {
 
         assert(m);
 
-        m->control_command_id = MOUNT_EXEC_MOUNT;
-        m->control_command = m->exec_command + MOUNT_EXEC_MOUNT;
-
         r = unit_fail_if_symlink(UNIT(m), m->where);
         if (r < 0)
                 goto fail;
@@ -949,6 +946,11 @@ static void mount_enter_mounting(Mount *m) {
 
         unit_warn_if_dir_nonempty(UNIT(m), m->where);
 
+        unit_warn_leftover_processes(UNIT(m));
+
+        m->control_command_id = MOUNT_EXEC_MOUNT;
+        m->control_command = m->exec_command + MOUNT_EXEC_MOUNT;
+
         /* Create the source directory for bind-mounts if needed */
         p = get_mount_parameters_fragment(m);
         if (p && mount_is_bind(p))

diff --git a/src/core/service.c b/src/core/service.c
index efd8087..672eaa9 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1814,6 +1814,8 @@ static void service_enter_start(Service *s) {
         service_unwatch_control_pid(s);
         service_unwatch_main_pid(s);
 
+        unit_warn_leftover_processes(UNIT(s));
+
         if (s->type == SERVICE_FORKING) {
                 s->control_command_id = SERVICE_EXEC_START;
                 c = s->control_command = s->exec_command[SERVICE_EXEC_START];
@@ -1901,6 +1903,8 @@ static void service_enter_start_pre(Service *s) {
         s->control_command = s->exec_command[SERVICE_EXEC_START_PRE];
         if (s->control_command) {
 
+                unit_warn_leftover_processes(UNIT(s));
+
                 s->control_command_id = SERVICE_EXEC_START_PRE;
 
                 r = service_spawn(s,

diff --git a/src/core/socket.c b/src/core/socket.c
index 572a70a..7e3630a 100644 (file)
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2187,6 +2187,9 @@ static void socket_enter_start_pre(Socket *s) {
         assert(s);
 
         socket_unwatch_control_pid(s);
+
+        unit_warn_leftover_processes(UNIT(s));
+
         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 115e59c..849ccbd 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -734,6 +734,8 @@ static void swap_enter_activating(Swap *s) {
 
         assert(s);
 
+        unit_warn_leftover_processes(UNIT(s));
+
         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 2ddd95c..8569398 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5196,6 +5196,31 @@ int unit_prepare_exec(Unit *u) {
         return 0;
 }
 
+static void log_leftover(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 */
+                return;
+
+        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.",
+                         pid, strna(comm));
+}
+
+void unit_warn_leftover_processes(Unit *u) {
+        assert(u);
+
+        (void) unit_pick_cgroup_path(u);
+
+        if (!u->cgroup_path)
+                return;
+
+        (void) cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, 0, 0, NULL, log_leftover, u);
+}
+
 static const char* const collect_mode_table[_COLLECT_MODE_MAX] = {
         [COLLECT_INACTIVE] = "inactive",
         [COLLECT_INACTIVE_OR_FAILED] = "inactive-or-failed",

diff --git a/src/core/unit.h b/src/core/unit.h
index e33597c..cd7c08a 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -768,6 +768,8 @@ void unit_unlink_state_files(Unit *u);
 
 int unit_prepare_exec(Unit *u);
 
+void unit_warn_leftover_processes(Unit *u);
+
 /* Macros which append UNIT= or USER_UNIT= to the message */
 
 #define log_unit_full(unit, level, error, ...)                          \