From e98b2fbbe9b48669f04e0e01eadf668be388fef4 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 24 Nov 2017 20:27:01 +0100
Subject: [PATCH] core: generalize the cgroup empty check on GC

Let's move the cgroup empty check for all unit types into the generic
unit_check_gc() call, out of the per-unit-type _check_gc() type. This
not only allows us to share some code, but also hooks up mount and
socket units with this kind of check, for free, as it was missing there
previously.
---
 src/core/scope.c   | 14 --------------
 src/core/service.c |  9 +++++----
 src/core/unit.c    | 12 ++++++++++++
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/core/scope.c b/src/core/scope.c
index e0e25673cb..10454d56b0 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -460,18 +460,6 @@ static int scope_deserialize_item(Unit *u, const char *key, const char *value, F
         return 0;
 }
 
-static bool scope_check_gc(Unit *u) {
-        assert(u);
-
-        /* Never clean up scopes that still have a process around,
-         * even if the scope is formally dead. */
-
-        if (!u->cgroup_path)
-                return false;
-
-        return cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path) <= 0;
-}
-
 static void scope_notify_cgroup_empty_event(Unit *u) {
         Scope *s = SCOPE(u);
         assert(u);
@@ -639,8 +627,6 @@ const UnitVTable scope_vtable = {
         .active_state = scope_active_state,
         .sub_state_to_string = scope_sub_state_to_string,
 
-        .check_gc = scope_check_gc,
-
         .sigchld_event = scope_sigchld_event,
 
         .reset_failed = scope_reset_failed,
diff --git a/src/core/service.c b/src/core/service.c
index 9a50153fb5..efd808784d 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2716,10 +2716,11 @@ static bool service_check_gc(Unit *u) {
 
         assert(s);
 
-        /* Never clean up services that still have a process around,
-         * even if the service is formally dead. */
-        if (cgroup_good(s) > 0 ||
-            main_pid_good(s) > 0 ||
+        /* Never clean up services that still have a process around, even if the service is formally dead. Note that
+         * unit_check_gc() already checked our cgroup for us, we just check our two additional PIDs, too, in case they
+         * have moved outside of the cgroup. */
+
+        if (main_pid_good(s) > 0 ||
             control_pid_good(s) > 0)
                 return true;
 
diff --git a/src/core/unit.c b/src/core/unit.c
index a5feb11ddb..2ddd95c3ac 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -334,6 +334,7 @@ int unit_set_description(Unit *u, const char *description) {
 
 bool unit_check_gc(Unit *u) {
         UnitActiveState state;
+        int r;
 
         assert(u);
 
@@ -381,6 +382,17 @@ bool unit_check_gc(Unit *u) {
                 assert_not_reached("Unknown garbage collection mode");
         }
 
+        if (u->cgroup_path) {
+                /* If the unit has a cgroup, then check whether there's anything in it. If so, we should stay
+                 * around. Units with active processes should never be collected. */
+
+                r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path);
+                if (r < 0)
+                        log_unit_debug_errno(u, r, "Failed to determine whether cgroup %s is empty: %m", u->cgroup_path);
+                if (r <= 0)
+                        return true;
+        }
+
         if (UNIT_VTABLE(u)->check_gc)
                 if (UNIT_VTABLE(u)->check_gc(u))
                         return true;