manager: prevent cleanup of triggering units before we start the handler

This fixes the following case:
OnFailure= would be spawned correctly, but OnSuccess= would be
spawned without the MONITOR_* metadata, because we'd "collect" the unit
that started successfully. So let's block cleanup while we have a job
running for the handler. The job cannot last infinitely, so at some point
we'll be able to collect both.

1 files changed, 19 insertions, 4 deletions

diff --git a/src/core/unit.c b/src/core/unit.c
index c9f88c81ef..bd11049c77 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -375,6 +375,20 @@ int unit_set_description(Unit *u, const char *description) {
         return 0;
 }
 
+static bool unit_success_failure_handler_has_jobs(Unit *unit) {
+        Unit *other;
+
+        UNIT_FOREACH_DEPENDENCY(other, unit, UNIT_ATOM_ON_SUCCESS)
+                if (other->job || other->nop_job)
+                        return true;
+
+        UNIT_FOREACH_DEPENDENCY(other, unit, UNIT_ATOM_ON_FAILURE)
+                if (other->job || other->nop_job)
+                        return true;
+
+        return false;
+}
+
 bool unit_may_gc(Unit *u) {
         UnitActiveState state;
         int r;
@@ -389,10 +403,7 @@ bool unit_may_gc(Unit *u) {
          * in unit_gc_sweep(), but using markers to properly collect dependency loops.
          */
 
-        if (u->job)
-                return false;
-
-        if (u->nop_job)
+        if (u->job || u->nop_job)
                 return false;
 
         state = unit_active_state(u);
@@ -427,6 +438,10 @@ bool unit_may_gc(Unit *u) {
                 assert_not_reached();
         }
 
+        /* Check if any OnFailure= or on Success= jobs may be pending */
+        if (unit_success_failure_handler_has_jobs(u))
+                return false;
+
         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. */