RFC: clean up inactive/failed {service|scope}'s cgroups when the last process exits

[anitazha]

So you have a service/scope and one of its processes is stuck in D state.
The unit is sent a systemctl stop and the process is sent the SIGTERM
then SIGKILL. It doesn't respond and after TimeoutStopSec the unit is
marked as failed because it times out while processes are still around.
The unit's cgroup remains in the file system (for example,
/system.slice/test.service).

When the process comes out of D state it processes the signals and exits.
Pid1 gets the SIGCHLD and tries to figure out what unit this process belongs
to based on what cgroup it's in, but since the unit's cgroup path information
was released earlier, pid1 thinks the process belongs to the unit's parent
(from the previous example, pid1 thinks the process belongs to system.slice).
This parent cgroup probably has many subgroups and processes under it
and cannot be removed, leaving the initial /system.slice/test.service
there until something is started and stopped under it again or
system.slice is stopped.

D state is one example but this happens to any process that happens to
stick around after the final kill signal state.

The fix I'm proposing is to keep the unit's cgroup path information
around if pid1 fails to rmdir the cgroup. When the last process exits
and sends the cgroup empty notification, the service/scope can try to
prune the cgroup if the unit is marked as inactive/failed.

[anitazha]

I think this might help with #16350 but I only tested minimally with cgroups2

[poettering]

Hmm, I'd do this differently. Maybe introduce unit_maybe_release_cgroup(). which is a wrapper around unit_release_cgroup() and checks if the cgroup is actually empty first. If it's not it returns immediately, otherwise it calls the real function.

Calling that function should then be safe and robust from pretty much any suitable place.

[anitazha]

Updated with a wrapper. But I think it's only applicable in unit_prune_cgroup where my original check was; the other 2 uses don't use the wrapper because the cgroup should be released regardless of whether it is empty or not (they're unit_free and unit_set_cgroup_path).

[anitazha]

Updated according to comments

[keszybz]

LGTM.

[poettering]

lgtm, too