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.

diff --git a/src/core/scope.c b/src/core/scope.c
index e0e25673cb3803ccbbcb0a626b80b7fb77cd5f32..10454d56b04651e7fe45c71b3774ccae849388ff 100644 (file)
--- 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 9a50153fb50989ece18b94b59de26106afe7b40d..efd808784debafc896d1acb7415a098725a2bd8a 100644 (file)
--- 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 a5feb11ddb4d56e3bbc456be6c0b84cccfd43e55..2ddd95c3ac64a7e3ab2fe36fc309e5730147cce3 100644 (file)
--- 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;