core/cgroup: Properly handle aborting a pending freeze operation

We must thaw the cgroup even if cgroup.events/frozen=0 if a freeze
operation is in flight as it means the cgroup is already partially
frozen.

Fixes #37590.
Fixes #38337.

(cherry picked from commit 85d00912c0fa08c80785d18a6818e7d92d40743e)
(cherry picked from commit 4bf37df99c905d21cf0febde59190d885ad201dc)

src/core/cgroup.c

@@ -5123,7 +5123,8 @@ static int unit_cgroup_freezer_kernel_state(Unit *u, FreezerState *ret) {
 
 int unit_cgroup_freezer_action(Unit *u, FreezerAction action) {
         _cleanup_free_ char *path = NULL;
-        FreezerState target, current, next;
+        FreezerState current, next, objective;
+        bool action_in_progress = false;
         int r;
 
         assert(u);
@@ -5133,25 +5134,32 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) {
         if (!cg_freezer_supported())
                 return 0;
 
-        unit_next_freezer_state(u, action, &next, &target);
+        unit_next_freezer_state(u, action, &next, &objective);
 
         CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_realized) {
+        if (!crt || !crt->cgroup_path)
                 /* No realized cgroup = nothing to freeze */
-                u->freezer_state = freezer_state_finish(next);
+                goto finish;
-                return 0;
-        }
 
         r = unit_cgroup_freezer_kernel_state(u, &current);
         if (r < 0)
                 return r;
 
-        if (current == target)
+        if (current == objective) {
-                next = freezer_state_finish(next);
+                if (objective == FREEZER_FROZEN)
-        else if (IN_SET(next, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT, FREEZER_RUNNING)) {
+                        goto finish;
-                /* We're transitioning into a finished state, which implies that the cgroup's
+
-                 * current state already matches the target and thus we'd return 0. But, reality
+                /* Skip thaw only if no freeze operation was in flight */
-                 * shows otherwise. This indicates that our freezer_state tracking has diverged
+                if (IN_SET(u->freezer_state, FREEZER_RUNNING, FREEZER_THAWING))
+                        goto finish;
+        } else
+                action_in_progress = true;
+
+        if (next == freezer_state_finish(next)) {
+                /* We're directly transitioning into a finished state, which in theory means that
+                 * the cgroup's current state already matches the objective and thus we'd return 0.
+                 * But, reality shows otherwise (such case would have been handled by current == objective
+                 * branch above). This indicates that our freezer_state tracking has diverged
                  * from the real state of the cgroup, which can happen if someone meddles with the
                  * cgroup from underneath us. This really shouldn't happen during normal operation,
                  * though. So, let's warn about it and fix up the state to be valid */
@@ -5165,22 +5173,25 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action) {
                         next = FREEZER_FREEZING_BY_PARENT;
                 else if (next == FREEZER_RUNNING)
                         next = FREEZER_THAWING;
+                else
+                        assert_not_reached();
         }
 
         r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, crt->cgroup_path, "cgroup.freeze", &path);
         if (r < 0)
                 return r;
 
-        log_unit_debug(u, "Unit freezer state was %s, now %s.",
+        r = write_string_file(path, one_zero(objective == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER);
-                       freezer_state_to_string(u->freezer_state),
-                       freezer_state_to_string(next));
-
-        r = write_string_file(path, one_zero(target == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER);
         if (r < 0)
                 return r;
 
-        u->freezer_state = next;
+finish:
-        return target != current;
+        if (action_in_progress)
+                unit_set_freezer_state(u, next);
+        else
+                unit_set_freezer_state(u, freezer_state_finish(next));
+
+        return action_in_progress;
 }
 
 int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name) {

src/core/unit.c

@@ -6188,23 +6188,23 @@ bool unit_can_isolate_refuse_manual(Unit *u) {
         return unit_can_isolate(u) && !u->refuse_manual_start;
 }
 
-void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_target) {
+void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_objective) {
-        Unit *slice;
+        FreezerState curr, parent, next, objective;
-        FreezerState curr, parent, next, tgt;
 
         assert(u);
         assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE,
                               FREEZER_THAW, FREEZER_PARENT_THAW));
         assert(ret);
-        assert(ret_target);
+        assert(ret_objective);
 
         /* This function determines the correct freezer state transitions for a unit
-         * given the action being requested. It returns the next state, and also the "target",
+         * given the action being requested. It returns the next state, and also the "objective",
          * which is either FREEZER_FROZEN or FREEZER_RUNNING, depending on what actual state we
          * ultimately want to achieve. */
 
         curr = u->freezer_state;
-         slice = UNIT_GET_SLICE(u);
+
+        Unit *slice = UNIT_GET_SLICE(u);
         if (slice)
                 parent = slice->freezer_state;
         else
@@ -6251,13 +6251,13 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, F
                         next = FREEZER_THAWING;
         }
 
-        tgt = freezer_state_finish(next);
+        objective = freezer_state_finish(next);
-        if (tgt == FREEZER_FROZEN_BY_PARENT)
+        if (objective == FREEZER_FROZEN_BY_PARENT)
-                tgt = FREEZER_FROZEN;
+                objective = FREEZER_FROZEN;
-        assert(IN_SET(tgt, FREEZER_RUNNING, FREEZER_FROZEN));
+        assert(IN_SET(objective, FREEZER_RUNNING, FREEZER_FROZEN));
 
         *ret = next;
-        *ret_target = tgt;
+        *ret_objective = objective;
 }
 
 bool unit_can_freeze(Unit *u) {
@@ -6272,6 +6272,22 @@ bool unit_can_freeze(Unit *u) {
         return UNIT_VTABLE(u)->freezer_action;
 }
 
+void unit_set_freezer_state(Unit *u, FreezerState state) {
+        assert(u);
+        assert(state >= 0);
+        assert(state < _FREEZER_STATE_MAX);
+
+        if (u->freezer_state == state)
+                return;
+
+        log_unit_debug(u, "Freezer state changed %s -> %s",
+                       freezer_state_to_string(u->freezer_state), freezer_state_to_string(state));
+
+        u->freezer_state = state;
+
+        unit_add_to_dbus_queue(u);
+}
+
 void unit_frozen(Unit *u) {
         assert(u);
 

src/core/unit.h

@@ -1040,7 +1040,8 @@ bool unit_can_isolate_refuse_manual(Unit *u);
 
 bool unit_can_freeze(Unit *u);
 int unit_freezer_action(Unit *u, FreezerAction action);
-void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_tgt);
+void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_objective);
+void unit_set_freezer_state(Unit *u, FreezerState state);
 void unit_frozen(Unit *u);
 void unit_thawed(Unit *u);