From 1e8f0beee4272ddc8b25dfa9af8e54bafc4c061a Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Sat, 26 Oct 2024 17:38:06 +0200
Subject: [PATCH 1/7] core/service: use log_unit_* where appropriate

---
 src/core/service.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 1ac158f1ea..03b9601be1 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -4893,7 +4893,7 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s
         e = sd_bus_message_get_error(reply);
         if (e) {
                 r = sd_bus_error_get_errno(e);
-                log_warning_errno(r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r));
+                log_unit_warning_errno(UNIT(s), r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r));
                 return 1;
         }
 
@@ -4905,7 +4905,7 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s
 
         r = pidref_set_pid(&pidref, pid);
         if (r < 0) {
-                log_debug_errno(r, "GetConnectionUnixProcessID() returned invalid PID: %m");
+                log_unit_debug_errno(UNIT(s), r, "GetConnectionUnixProcessID() returned invalid PID: %m");
                 return 1;
         }
 
@@ -4952,7 +4952,7 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
                                 "s",
                                 s->bus_name);
                 if (r < 0)
-                        log_debug_errno(r, "Failed to request owner PID of service name, ignoring: %m");
+                        log_unit_debug_errno(u, r, "Failed to request owner PID of service name, ignoring: %m");
         }
 }
 

From 2c0ce41cb82cb1300ceb85c57d897745491a6820 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 19:27:19 +0200
Subject: [PATCH 2/7] core: drop effectively unused UNIT_ATOM_PROPAGATE_RESTART

Restart jobs are always run as stop jobs initially, and later gets
converted to start jobs by job engine. Hence UNIT_ATOM_PROPAGATE_STOP
should and does cover the restart case, as currently all dep types
with _RESTART also carries _STOP. Drop UNIT_ATOM_PROPAGATE_RESTART.
---
 src/core/transaction.c          |  2 +-
 src/core/unit-dependency-atom.c | 12 ++---------
 src/core/unit-dependency-atom.h | 38 ++++++++++++++++-----------------
 3 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/src/core/transaction.c b/src/core/transaction.c
index 9d48768d87..1ab59b823e 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -1087,7 +1087,7 @@ int transaction_add_job_and_dependencies(
         if (IN_SET(type, JOB_RESTART, JOB_STOP) || (type == JOB_START && FLAGS_SET(flags, TRANSACTION_PROPAGATE_START_AS_RESTART))) {
                 bool is_stop = type == JOB_STOP;
 
-                UNIT_FOREACH_DEPENDENCY(dep, ret->unit, is_stop ? UNIT_ATOM_PROPAGATE_STOP : UNIT_ATOM_PROPAGATE_RESTART) {
+                UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP) {
                         /* We propagate RESTART only as TRY_RESTART, in order not to start dependencies that
                          * are not around. */
                         JobType nt;
diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c
index 35b279b5c7..aba58612f7 100644
--- a/src/core/unit-dependency-atom.c
+++ b/src/core/unit-dependency-atom.c
@@ -33,8 +33,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
                                         UNIT_ATOM_ADD_STOP_WHEN_UNNEEDED_QUEUE |
                                         UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE,
 
-        [UNIT_PART_OF]                = UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE,
-
         [UNIT_UPHOLDS]                = UNIT_ATOM_PULL_IN_START_IGNORED |
                                         UNIT_ATOM_RETROACTIVE_START_REPLACE |
                                         UNIT_ATOM_ADD_START_WHEN_UPHELD_QUEUE |
@@ -42,13 +40,11 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
                                         UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE,
 
         [UNIT_REQUIRED_BY]            = UNIT_ATOM_PROPAGATE_STOP |
-                                        UNIT_ATOM_PROPAGATE_RESTART |
                                         UNIT_ATOM_PROPAGATE_START_FAILURE |
                                         UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED |
                                         UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES,
 
         [UNIT_REQUISITE_OF]           = UNIT_ATOM_PROPAGATE_STOP |
-                                        UNIT_ATOM_PROPAGATE_RESTART |
                                         UNIT_ATOM_PROPAGATE_START_FAILURE |
                                         UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE |
                                         UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED |
@@ -59,7 +55,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
 
         [UNIT_BOUND_BY]               = UNIT_ATOM_RETROACTIVE_STOP_ON_STOP |
                                         UNIT_ATOM_PROPAGATE_STOP |
-                                        UNIT_ATOM_PROPAGATE_RESTART |
                                         UNIT_ATOM_PROPAGATE_START_FAILURE |
                                         UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED |
                                         UNIT_ATOM_ADD_CANNOT_BE_ACTIVE_WITHOUT_QUEUE |
@@ -69,9 +64,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
                                         UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES |
                                         UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED,
 
-        [UNIT_CONSISTS_OF]            = UNIT_ATOM_PROPAGATE_STOP |
-                                        UNIT_ATOM_PROPAGATE_RESTART,
-
         [UNIT_CONFLICTS]              = UNIT_ATOM_PULL_IN_STOP |
                                         UNIT_ATOM_RETROACTIVE_STOP_ON_START,
 
@@ -89,6 +81,8 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
         [UNIT_ON_SUCCESS_OF]          = UNIT_ATOM_ON_SUCCESS_OF,
         [UNIT_BEFORE]                 = UNIT_ATOM_BEFORE,
         [UNIT_AFTER]                  = UNIT_ATOM_AFTER,
+        [UNIT_PART_OF]                = UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE,
+        [UNIT_CONSISTS_OF]            = UNIT_ATOM_PROPAGATE_STOP,
         [UNIT_TRIGGERS]               = UNIT_ATOM_TRIGGERS,
         [UNIT_TRIGGERED_BY]           = UNIT_ATOM_TRIGGERED_BY,
         [UNIT_PROPAGATES_RELOAD_TO]   = UNIT_ATOM_PROPAGATES_RELOAD_TO,
@@ -160,7 +154,6 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) {
                 return UNIT_UPHOLDS;
 
         case UNIT_ATOM_PROPAGATE_STOP |
-                UNIT_ATOM_PROPAGATE_RESTART |
                 UNIT_ATOM_PROPAGATE_START_FAILURE |
                 UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE |
                 UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED |
@@ -170,7 +163,6 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) {
 
         case UNIT_ATOM_RETROACTIVE_STOP_ON_STOP |
                 UNIT_ATOM_PROPAGATE_STOP |
-                UNIT_ATOM_PROPAGATE_RESTART |
                 UNIT_ATOM_PROPAGATE_START_FAILURE |
                 UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED |
                 UNIT_ATOM_ADD_CANNOT_BE_ACTIVE_WITHOUT_QUEUE |
diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h
index 96f00ca39a..89a3eae1bf 100644
--- a/src/core/unit-dependency-atom.h
+++ b/src/core/unit-dependency-atom.h
@@ -56,35 +56,33 @@ typedef enum UnitDependencyAtom {
         /* If our start job succeeded but the unit is inactive then (think: oneshot units), propagate this as
          * failure to the other unit. */
         UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE = UINT64_C(1) << 17,
-        /* When putting together a transaction, propagate JOB_STOP from our unit to the other. */
+        /* When putting together a transaction, propagate JOB_STOP/JOB_RESTART from our unit to the other. */
         UNIT_ATOM_PROPAGATE_STOP                      = UINT64_C(1) << 18,
         /* Like UNIT_ATOM_PROPAGATE_STOP, but enqueues a restart job if there's already a start job (avoids
          * job type conflict). */
         UNIT_ATOM_PROPAGATE_STOP_GRACEFUL             = UINT64_C(1) << 19,
-        /* When putting together a transaction, propagate JOB_RESTART from our unit to the other. */
-        UNIT_ATOM_PROPAGATE_RESTART                   = UINT64_C(1) << 20,
 
         /* Add the other unit to the default target dependency queue */
-        UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 21,
+        UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 20,
         /* Recheck default target deps on other units (which are target units) */
-        UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES         = UINT64_C(1) << 22,
+        UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES         = UINT64_C(1) << 21,
 
         /* The remaining atoms map 1:1 to the equally named high-level deps */
-        UNIT_ATOM_ON_FAILURE                          = UINT64_C(1) << 23,
-        UNIT_ATOM_ON_SUCCESS                          = UINT64_C(1) << 24,
-        UNIT_ATOM_ON_FAILURE_OF                       = UINT64_C(1) << 25,
-        UNIT_ATOM_ON_SUCCESS_OF                       = UINT64_C(1) << 26,
-        UNIT_ATOM_BEFORE                              = UINT64_C(1) << 27,
-        UNIT_ATOM_AFTER                               = UINT64_C(1) << 28,
-        UNIT_ATOM_TRIGGERS                            = UINT64_C(1) << 29,
-        UNIT_ATOM_TRIGGERED_BY                        = UINT64_C(1) << 30,
-        UNIT_ATOM_PROPAGATES_RELOAD_TO                = UINT64_C(1) << 31,
-        UNIT_ATOM_JOINS_NAMESPACE_OF                  = UINT64_C(1) << 32,
-        UNIT_ATOM_REFERENCES                          = UINT64_C(1) << 33,
-        UNIT_ATOM_REFERENCED_BY                       = UINT64_C(1) << 34,
-        UNIT_ATOM_IN_SLICE                            = UINT64_C(1) << 35,
-        UNIT_ATOM_SLICE_OF                            = UINT64_C(1) << 36,
-        _UNIT_DEPENDENCY_ATOM_MAX                     = (UINT64_C(1) << 37) - 1,
+        UNIT_ATOM_ON_FAILURE                          = UINT64_C(1) << 22,
+        UNIT_ATOM_ON_SUCCESS                          = UINT64_C(1) << 23,
+        UNIT_ATOM_ON_FAILURE_OF                       = UINT64_C(1) << 24,
+        UNIT_ATOM_ON_SUCCESS_OF                       = UINT64_C(1) << 25,
+        UNIT_ATOM_BEFORE                              = UINT64_C(1) << 26,
+        UNIT_ATOM_AFTER                               = UINT64_C(1) << 27,
+        UNIT_ATOM_TRIGGERS                            = UINT64_C(1) << 28,
+        UNIT_ATOM_TRIGGERED_BY                        = UINT64_C(1) << 29,
+        UNIT_ATOM_PROPAGATES_RELOAD_TO                = UINT64_C(1) << 30,
+        UNIT_ATOM_JOINS_NAMESPACE_OF                  = UINT64_C(1) << 31,
+        UNIT_ATOM_REFERENCES                          = UINT64_C(1) << 32,
+        UNIT_ATOM_REFERENCED_BY                       = UINT64_C(1) << 33,
+        UNIT_ATOM_IN_SLICE                            = UINT64_C(1) << 34,
+        UNIT_ATOM_SLICE_OF                            = UINT64_C(1) << 35,
+        _UNIT_DEPENDENCY_ATOM_MAX                     = (UINT64_C(1) << 36) - 1,
         _UNIT_DEPENDENCY_ATOM_INVALID                 = -EINVAL,
 } UnitDependencyAtom;
 

From 215ffd6c1f4707d65b0eee2deace668605c18b2a Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 23:48:42 +0200
Subject: [PATCH 3/7] core/job: trivial modernization

---
 src/core/job.c | 16 ++++++++++------
 src/core/job.h | 17 ++++++++---------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/core/job.c b/src/core/job.c
index 468571ae71..49cda53f22 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -162,6 +162,7 @@ static void job_set_state(Job *j, JobState state) {
 void job_uninstall(Job *j) {
         Job **pj;
 
+        assert(j);
         assert(j->installed);
 
         job_set_state(j, JOB_WAITING);
@@ -329,14 +330,16 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool
          * this means the 'anchor' job (i.e. the one the user
          * explicitly asked for) is the requester. */
 
-        l = new0(JobDependency, 1);
+        l = new(JobDependency, 1);
         if (!l)
                 return NULL;
 
-        l->subject = subject;
-        l->object = object;
-        l->matters = matters;
-        l->conflicts = conflicts;
+        *l = (JobDependency) {
+                .subject = subject,
+                .object = object,
+                .matters = matters,
+                .conflicts = conflicts,
+        };
 
         if (subject)
                 LIST_PREPEND(subject, subject->subject_list, l);
@@ -494,6 +497,7 @@ JobType job_type_collapse(JobType t, Unit *u) {
                 return JOB_RELOAD;
 
         default:
+                assert(t >= 0 && t < _JOB_TYPE_MAX_IN_TRANSACTION);
                 return t;
         }
 }
@@ -1243,7 +1247,7 @@ int job_deserialize(Job *j, FILE *f) {
         for (;;) {
                 _cleanup_free_ char *l = NULL;
                 size_t k;
-                char *v;
+                const char *v;
 
                 r = deserialize_read_line(f, &l);
                 if (r < 0)
diff --git a/src/core/job.h b/src/core/job.h
index 680b4a6016..7603f311f2 100644
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -127,7 +127,7 @@ struct Job {
         LIST_HEAD(JobDependency, object_list);
 
         /* Used for graph algs as a "I have been here" marker */
-        Job* marker;
+        Job *marker;
         unsigned generation;
 
         uint32_t id;
@@ -135,6 +135,10 @@ struct Job {
         JobType type;
         JobState state;
 
+        JobResult result;
+
+        unsigned run_queue_idx;
+
         sd_event_source *timer_event_source;
         usec_t begin_usec;
         usec_t begin_running_usec;
@@ -149,10 +153,6 @@ struct Job {
         sd_bus_track *bus_track;
         char **deserialized_clients;
 
-        JobResult result;
-
-        unsigned run_queue_idx;
-
         /* If the job had a specific trigger that needs to be advertised (eg: a path unit), store it. */
         ActivationDetails *activation_details;
 
@@ -171,9 +171,12 @@ Job* job_new(Unit *unit, JobType type);
 Job* job_new_raw(Unit *unit);
 void job_unlink(Job *job);
 Job* job_free(Job *job);
+DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free);
+
 Job* job_install(Job *j, bool refuse_late_merge);
 int job_install_deserialized(Job *j);
 void job_uninstall(Job *j);
+
 void job_dump(Job *j, FILE *f, const char *prefix);
 int job_serialize(Job *j, FILE *f);
 int job_deserialize(Job *j, FILE *f);
@@ -182,8 +185,6 @@ int job_coldplug(Job *j);
 JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts);
 void job_dependency_free(JobDependency *l);
 
-int job_merge(Job *j, Job *other);
-
 JobType job_type_lookup_merge(JobType a, JobType b) _pure_;
 
 _pure_ static inline bool job_type_is_mergeable(JobType a, JobType b) {
@@ -231,8 +232,6 @@ void job_add_to_gc_queue(Job *j);
 int job_get_before(Job *j, Job*** ret);
 int job_get_after(Job *j, Job*** ret);
 
-DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free);
-
 const char* job_type_to_string(JobType t) _const_;
 JobType job_type_from_string(const char *s) _pure_;
 

From d993ad6c6fa0cdb423a4e04697afa3e8dc67ded7 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 19:53:28 +0200
Subject: [PATCH 4/7] core/manager: introduce manager_add_job_full() which
 takes extra TransactionAddFlags

No functional change. Preparation for later commits.
---
 src/analyze/analyze-verify-util.c |  2 +-
 src/core/automount.c              |  4 ++--
 src/core/dbus-unit.c              |  2 +-
 src/core/dbus.c                   |  2 +-
 src/core/main.c                   |  4 ++--
 src/core/manager.c                | 19 +++++++++++--------
 src/core/manager.h                | 24 +++++++++++++++++++++---
 src/core/path.c                   |  2 +-
 src/core/service.c                |  5 ++---
 src/core/socket.c                 |  4 ++--
 src/core/timer.c                  |  2 +-
 src/core/transaction.h            | 21 +++++++++++++--------
 src/core/unit.c                   | 10 +++++-----
 src/test/test-engine.c            | 24 ++++++++++++------------
 14 files changed, 75 insertions(+), 50 deletions(-)

diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c
index 017d4b4672..97f5449b0b 100644
--- a/src/analyze/analyze-verify-util.c
+++ b/src/analyze/analyze-verify-util.c
@@ -251,7 +251,7 @@ static int verify_unit(Unit *u, bool check_man, const char *root) {
                 unit_dump(u, stdout, "\t");
 
         log_unit_debug(u, "Creating %s/start job", u->id);
-        r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, NULL, &error, NULL);
+        r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, &error, /* ret = */ NULL);
         if (r < 0)
                 log_unit_error_errno(u, r, "Failed to create %s/start: %s", u->id, bus_error_message(&error, r));
 
diff --git a/src/core/automount.c b/src/core/automount.c
index 9d0f7a1f6f..ac81875442 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -781,7 +781,7 @@ static void automount_enter_running(Automount *a) {
                 goto fail;
         }
 
-        r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL);
+        r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, &error, /* ret = */ NULL);
         if (r < 0) {
                 log_unit_warning(UNIT(a), "Failed to queue mount startup job: %s", bus_error_message(&error, r));
                 goto fail;
@@ -997,7 +997,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo
                         goto fail;
                 }
 
-                r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, NULL, &error, NULL);
+                r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, &error, /* ret = */ NULL);
                 if (r < 0) {
                         log_unit_warning(UNIT(a), "Failed to queue unmount job: %s", bus_error_message(&error, r));
                         goto fail;
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 2db473fbed..03f25402a2 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1864,7 +1864,7 @@ int bus_unit_queue_job_one(
                         return -ENOMEM;
         }
 
-        r = manager_add_job(u->manager, type, u, mode, affected, error, &j);
+        r = manager_add_job_full(u->manager, type, u, mode, /* extra_flags = */ 0, affected, error, &j);
         if (r < 0)
                 return r;
 
diff --git a/src/core/dbus.c b/src/core/dbus.c
index 2692dde151..744b97f991 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -175,7 +175,7 @@ static int signal_activation_request(sd_bus_message *message, void *userdata, sd
                 goto failed;
         }
 
-        r = manager_add_job(m, JOB_START, u, JOB_REPLACE, NULL, &error, NULL);
+        r = manager_add_job(m, JOB_START, u, JOB_REPLACE, &error, /* ret = */ NULL);
         if (r < 0)
                 goto failed;
 
diff --git a/src/core/main.c b/src/core/main.c
index 0c171a788d..93a1e221f7 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -2582,13 +2582,13 @@ static int do_queue_default_job(
 
         assert(target->load_state == UNIT_LOADED);
 
-        r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, NULL, &error, &job);
+        r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, &error, &job);
         if (r == -EPERM) {
                 log_debug_errno(r, "Default target could not be isolated, starting instead: %s", bus_error_message(&error, r));
 
                 sd_bus_error_free(&error);
 
-                r = manager_add_job(m, JOB_START, target, JOB_REPLACE, NULL, &error, &job);
+                r = manager_add_job(m, JOB_START, target, JOB_REPLACE, &error, &job);
                 if (r < 0) {
                         *ret_error_message = "Failed to start default target";
                         return log_struct_errno(LOG_EMERG, r,
diff --git a/src/core/manager.c b/src/core/manager.c
index 57b7c3cbd8..f58bc547a6 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1553,7 +1553,7 @@ static unsigned manager_dispatch_stop_when_unneeded_queue(Manager *m) {
                 }
 
                 /* Ok, nobody needs us anymore. Sniff. Then let's commit suicide */
-                r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, NULL, &error, NULL);
+                r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, /* ret = */ NULL);
                 if (r < 0)
                         log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r));
         }
@@ -1594,7 +1594,7 @@ static unsigned manager_dispatch_start_when_upheld_queue(Manager *m) {
                         continue;
                 }
 
-                r = manager_add_job(u->manager, JOB_START, u, JOB_FAIL, NULL, &error, NULL);
+                r = manager_add_job(u->manager, JOB_START, u, JOB_FAIL, &error, /* ret = */ NULL);
                 if (r < 0)
                         log_unit_warning_errno(u, r, "Failed to enqueue start job, ignoring: %s", bus_error_message(&error, r));
         }
@@ -1635,7 +1635,7 @@ static unsigned manager_dispatch_stop_when_bound_queue(Manager *m) {
                         continue;
                 }
 
-                r = manager_add_job(u->manager, JOB_STOP, u, JOB_REPLACE, NULL, &error, NULL);
+                r = manager_add_job(u->manager, JOB_STOP, u, JOB_REPLACE, &error, /* ret = */ NULL);
                 if (r < 0)
                         log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r));
         }
@@ -2109,11 +2109,12 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo
         return 0;
 }
 
-int manager_add_job(
+int manager_add_job_full(
                 Manager *m,
                 JobType type,
                 Unit *unit,
                 JobMode mode,
+                TransactionAddFlags extra_flags,
                 Set *affected_jobs,
                 sd_bus_error *error,
                 Job **ret) {
@@ -2122,9 +2123,10 @@ int manager_add_job(
         int r;
 
         assert(m);
-        assert(type < _JOB_TYPE_MAX);
+        assert(type >= 0 && type < _JOB_TYPE_MAX);
         assert(unit);
-        assert(mode < _JOB_MODE_MAX);
+        assert(mode >= 0 && mode < _JOB_MODE_MAX);
+        assert((extra_flags & ~_TRANSACTION_FLAGS_MASK_PUBLIC) == 0);
 
         if (mode == JOB_ISOLATE && type != JOB_START)
                 return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start.");
@@ -2154,7 +2156,8 @@ int manager_add_job(
                         TRANSACTION_MATTERS |
                         (IN_SET(mode, JOB_IGNORE_DEPENDENCIES, JOB_IGNORE_REQUIREMENTS) ? TRANSACTION_IGNORE_REQUIREMENTS : 0) |
                         (mode == JOB_IGNORE_DEPENDENCIES ? TRANSACTION_IGNORE_ORDER : 0) |
-                        (mode == JOB_RESTART_DEPENDENCIES ? TRANSACTION_PROPAGATE_START_AS_RESTART : 0),
+                        (mode == JOB_RESTART_DEPENDENCIES ? TRANSACTION_PROPAGATE_START_AS_RESTART : 0) |
+                        extra_flags,
                         error);
         if (r < 0)
                 return r;
@@ -2200,7 +2203,7 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode
                 return r;
         assert(unit);
 
-        return manager_add_job(m, type, unit, mode, affected_jobs, e, ret);
+        return manager_add_job_full(m, type, unit, mode, /* extra_flags = */ 0, affected_jobs, e, ret);
 }
 
 int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret) {
diff --git a/src/core/manager.h b/src/core/manager.h
index abf02bf079..c1f7f8c083 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -139,6 +139,7 @@ typedef enum WatchdogType {
 #include "job.h"
 #include "path-lookup.h"
 #include "show-status.h"
+#include "transaction.h"
 #include "unit-name.h"
 #include "unit.h"
 
@@ -554,9 +555,26 @@ int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_err
 int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret);
 int manager_load_unit_from_dbus_path(Manager *m, const char *s, sd_bus_error *e, Unit **_u);
 
-int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret);
-int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret);
-int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs,  Job **ret);
+int manager_add_job_full(
+                Manager *m,
+                JobType type,
+                Unit *unit,
+                JobMode mode,
+                TransactionAddFlags extra_flags,
+                Set *affected_jobs,
+                sd_bus_error *error,
+                Job **ret);
+static inline int manager_add_job(
+                Manager *m,
+                JobType type,
+                Unit *unit,
+                JobMode mode,
+                sd_bus_error *error,
+                Job **ret) {
+        return manager_add_job_full(m, type, unit, mode, 0, NULL, error, ret);
+}
+int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **ret);
+int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret);
 int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error *e);
 
 void manager_clear_jobs(Manager *m);
diff --git a/src/core/path.c b/src/core/path.c
index 50f6db1470..e0dc93aabd 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -546,7 +546,7 @@ static void path_enter_running(Path *p, char *trigger_path) {
                 goto fail;
         }
 
-        r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job);
+        r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, &error, &job);
         if (r < 0) {
                 log_unit_warning(UNIT(p), "Failed to queue unit startup job: %s", bus_error_message(&error, r));
                 goto fail;
diff --git a/src/core/service.c b/src/core/service.c
index 03b9601be1..b9c12140d3 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2647,11 +2647,10 @@ static void service_enter_restart(Service *s, bool shortcut) {
 
         /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves
          * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. */
-        r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, NULL, &error, NULL);
+        r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, &error, /* ret = */ NULL);
         if (r < 0) {
                 log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r));
-                service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false);
-                return;
+                return service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false);
         }
 
         /* Count the jobs we enqueue for restarting. This counter is maintained as long as the unit isn't
diff --git a/src/core/socket.c b/src/core/socket.c
index 8519aefa68..9b99ab2c35 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2337,7 +2337,7 @@ static void socket_enter_running(Socket *s, int cfd_in) {
                                 goto fail;
                         }
 
-                        r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, NULL, &error, NULL);
+                        r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, &error, /* ret = */ NULL);
                         if (r < 0)
                                 goto queue_error;
                 }
@@ -2405,7 +2405,7 @@ static void socket_enter_running(Socket *s, int cfd_in) {
 
                 s->n_connections++;
 
-                r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, NULL, &error, NULL);
+                r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, &error, /* ret = */ NULL);
                 if (r < 0) {
                         /* We failed to activate the new service, but it still exists. Let's make sure the
                          * service closes and forgets the connection fd again, immediately. */
diff --git a/src/core/timer.c b/src/core/timer.c
index e44298d5e8..b37a67f310 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -618,7 +618,7 @@ static void timer_enter_running(Timer *t) {
                 goto fail;
         }
 
-        r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job);
+        r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, &error, &job);
         if (r < 0) {
                 log_unit_warning(UNIT(t), "Failed to queue unit startup job: %s", bus_error_message(&error, r));
                 goto fail;
diff --git a/src/core/transaction.h b/src/core/transaction.h
index 151e02dd60..f2bf047f10 100644
--- a/src/core/transaction.h
+++ b/src/core/transaction.h
@@ -2,6 +2,7 @@
 #pragma once
 
 typedef struct Transaction Transaction;
+typedef enum TransactionAddFlags TransactionAddFlags;
 
 #include "hashmap.h"
 #include "job.h"
@@ -10,14 +11,14 @@ typedef struct Transaction Transaction;
 
 struct Transaction {
         /* Jobs to be added */
-        Hashmap *jobs;      /* Unit object => Job object list 1:1 */
-        Job *anchor_job;      /* the job the user asked for */
+        Hashmap *jobs;        /* Unit object => Job object list 1:1 */
+        Job *anchor_job;      /* The job the user asked for */
         bool irreversible;
 };
 
-Transaction *transaction_new(bool irreversible);
-Transaction *transaction_free(Transaction *tr);
-Transaction *transaction_abort_and_free(Transaction *tr);
+Transaction* transaction_new(bool irreversible);
+Transaction* transaction_free(Transaction *tr);
+Transaction* transaction_abort_and_free(Transaction *tr);
 DEFINE_TRIVIAL_CLEANUP_FUNC(Transaction*, transaction_abort_and_free);
 
 typedef enum TransactionAddFlags {
@@ -31,13 +32,19 @@ typedef enum TransactionAddFlags {
 
         /* Indicate that we're in the recursion for processing UNIT_ATOM_PROPAGATE_STOP_GRACEFUL units */
         TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL = 1 << 5,
+
+        _TRANSACTION_FLAGS_MASK_PUBLIC              = 0,
 } TransactionAddFlags;
 
 void transaction_add_propagate_reload_jobs(
                 Transaction *tr,
-                Unit *unit, Job *by,
+                Unit *unit,
+                Job *by,
                 TransactionAddFlags flags);
 
+int transaction_add_isolate_jobs(Transaction *tr, Manager *m);
+int transaction_add_triggering_jobs(Transaction *tr, Unit *u);
+
 int transaction_add_job_and_dependencies(
                 Transaction *tr,
                 JobType type,
@@ -47,5 +54,3 @@ int transaction_add_job_and_dependencies(
                 sd_bus_error *e);
 
 int transaction_activate(Transaction *tr, Manager *m, JobMode mode, Set *affected, sd_bus_error *e);
-int transaction_add_isolate_jobs(Transaction *tr, Manager *m);
-int transaction_add_triggering_jobs(Transaction *tr, Unit *u);
diff --git a/src/core/unit.c b/src/core/unit.c
index dda98bb861..eec08a2fbf 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2217,16 +2217,16 @@ static void retroactively_start_dependencies(Unit *u) {
         UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_START_REPLACE) /* Requires= + BindsTo= */
                 if (!unit_has_dependency(u, UNIT_ATOM_AFTER, other) &&
                     !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
-                        (void) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL, NULL);
+                        (void) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL);
 
         UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_START_FAIL) /* Wants= */
                 if (!unit_has_dependency(u, UNIT_ATOM_AFTER, other) &&
                     !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
-                        (void) manager_add_job(u->manager, JOB_START, other, JOB_FAIL, NULL, NULL, NULL);
+                        (void) manager_add_job(u->manager, JOB_START, other, JOB_FAIL, /* error = */ NULL, /* ret = */ NULL);
 
         UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_STOP_ON_START) /* Conflicts= (and inverse) */
                 if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
-                        (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL);
+                        (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL);
 }
 
 static void retroactively_stop_dependencies(Unit *u) {
@@ -2238,7 +2238,7 @@ static void retroactively_stop_dependencies(Unit *u) {
         /* Pull down units which are bound to us recursively if enabled */
         UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_STOP_ON_STOP) /* BoundBy= */
                 if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
-                        (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL);
+                        (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL);
 }
 
 void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom) {
@@ -2269,7 +2269,7 @@ void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom) {
                 if (n_jobs == 0)
                         log_unit_info(u, "Triggering %s dependencies.", dependency_name);
 
-                r = manager_add_job(u->manager, JOB_START, other, job_mode, NULL, &error, NULL);
+                r = manager_add_job(u->manager, JOB_START, other, job_mode, &error, /* ret = */ NULL);
                 if (r < 0)
                         log_unit_warning_errno(u, r, "Failed to enqueue %s%s job, ignoring: %s",
                                                dependency_name, other->id, bus_error_message(&error, r));
diff --git a/src/test/test-engine.c b/src/test/test-engine.c
index 0169f8320f..57774d04c5 100644
--- a/src/test/test-engine.c
+++ b/src/test/test-engine.c
@@ -106,7 +106,7 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test1: (Trivial)\n");
-        r = manager_add_job(m, JOB_START, c, JOB_REPLACE, NULL, &err, &j);
+        r = manager_add_job(m, JOB_START, c, JOB_REPLACE, &err, &j);
         if (sd_bus_error_is_set(&err))
                 log_error("error: %s: %s", err.name, err.message);
         assert_se(r == 0);
@@ -119,15 +119,15 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test2: (Cyclic Order, Unfixable)\n");
-        assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, NULL, &j) == -EDEADLK);
+        assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, &j) == -EDEADLK);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test3: (Cyclic Order, Fixable, Garbage Collector)\n");
-        assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test4: (Identical transaction)\n");
-        assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Load3:\n");
@@ -135,21 +135,21 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test5: (Colliding transaction, fail)\n");
-        assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK);
+        assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, &j) == -EDEADLK);
 
         printf("Test6: (Colliding transaction, replace)\n");
-        assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test7: (Unmergeable job type, fail)\n");
-        assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK);
+        assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, &j) == -EDEADLK);
 
         printf("Test8: (Mergeable job type, fail)\n");
-        assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test9: (Unmergeable job type, replace)\n");
-        assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Load4:\n");
@@ -157,7 +157,7 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test10: (Unmergeable job type of auxiliary job, fail)\n");
-        assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, &j) == 0);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Load5:\n");
@@ -168,7 +168,7 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test11: (Start/stop job ordering, execution cycle)\n");
-        assert_se(manager_add_job(m, JOB_START, i, JOB_FAIL, NULL, NULL, &j) == 0);
+        assert_se(manager_add_job(m, JOB_START, i, JOB_FAIL, NULL, &j) == 0);
         assert_se(unit_has_job_type(a, JOB_STOP));
         assert_se(unit_has_job_type(d, JOB_STOP));
         assert_se(unit_has_job_type(b, JOB_START));
@@ -181,7 +181,7 @@ int main(int argc, char *argv[]) {
         manager_dump_units(m, stdout, /* patterns= */ NULL, "\t");
 
         printf("Test12: (Trivial cycle, Unfixable)\n");
-        assert_se(manager_add_job(m, JOB_START, a_conj, JOB_REPLACE, NULL, NULL, &j) == -EDEADLK);
+        assert_se(manager_add_job(m, JOB_START, a_conj, JOB_REPLACE, NULL, &j) == -EDEADLK);
         manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t");
 
         assert_se(!hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b));

From 569269d02deed18ac4605c277220e84a150d6a56 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 23:49:09 +0200
Subject: [PATCH 5/7] core: make refuse_late_merge a proper attr of Job and
 introduce TRANSACTION_REENQUEUE_ANCHOR

---
 src/core/job.c         | 45 ++++++++++++++++++++++++++----------------
 src/core/job.h         | 13 ++++++++----
 src/core/service.c     | 15 ++++++++++++--
 src/core/transaction.c | 25 +++++++++++------------
 src/core/transaction.h |  5 ++++-
 5 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/src/core/job.c b/src/core/job.c
index 49cda53f22..8fbbe757fe 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -186,25 +186,37 @@ void job_uninstall(Job *j) {
         j->installed = false;
 }
 
-static bool job_type_allows_late_merge(JobType t) {
-        /* Tells whether it is OK to merge a job of type 't' with an already
-         * running job.
-         * Reloads cannot be merged this way. Think of the sequence:
-         * 1. Reload of a daemon is in progress; the daemon has already loaded
-         *    its config file, but hasn't completed the reload operation yet.
+static bool jobs_may_late_merge(const Job *j, const Job *uj) {
+        assert(j);
+        assert(!j->installed);
+        assert(uj);
+        assert(uj->installed);
+        assert(uj->state == JOB_RUNNING);
+
+        /* Tells whether it is OK to merge a job with an already running job. */
+
+        if (j->refuse_late_merge) /* refused when constructing transaction? */
+                return false;
+
+        /* Reloads cannot be merged this way. Think of the sequence:
+         * 1. Reload of a daemon is in progress; the daemon has already loaded its config file, but hasn't
+         *    completed the reload operation yet.
          * 2. Edit foo's config file.
          * 3. Trigger another reload to have the daemon use the new config.
-         * Should the second reload job be merged into the first one, the daemon
-         * would not know about the new config.
-         * JOB_RESTART jobs on the other hand can be merged, because they get
-         * patched into JOB_START after stopping the unit. So if we see a
-         * JOB_RESTART running, it means the unit hasn't stopped yet and at
-         * this time the merge is still allowed. */
-        return t != JOB_RELOAD;
+         * Should the second reload job be merged into the first one, the daemon would not know about the new config.
+         * JOB_RESTART jobs on the other hand can be merged, because they get patched into JOB_START
+         * after stopping the unit. So if we see a JOB_RESTART running, it means the unit hasn't stopped yet
+         * and at this time the merge is still allowed. */
+        if (j->type == JOB_RELOAD)
+                return false;
+
+        return job_type_is_superset(uj->type, j->type);
 }
 
 static void job_merge_into_installed(Job *j, Job *other) {
+        assert(j);
         assert(j->installed);
+        assert(other);
         assert(j->unit == other->unit);
 
         if (j->type != JOB_NOP) {
@@ -220,13 +232,13 @@ static void job_merge_into_installed(Job *j, Job *other) {
         j->ignore_order = j->ignore_order || other->ignore_order;
 }
 
-Job* job_install(Job *j, bool refuse_late_merge) {
+Job* job_install(Job *j) {
         Job **pj;
         Job *uj;
 
         assert(j);
         assert(!j->installed);
-        assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION);
+        assert(j->type >= 0 && j->type < _JOB_TYPE_MAX_IN_TRANSACTION);
         assert(j->state == JOB_WAITING);
 
         pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job;
@@ -238,8 +250,7 @@ Job* job_install(Job *j, bool refuse_late_merge) {
                 else {
                         /* not conflicting, i.e. mergeable */
 
-                        if (uj->state == JOB_WAITING ||
-                            (!refuse_late_merge && job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) {
+                        if (uj->state == JOB_WAITING || jobs_may_late_merge(j, uj)) {
                                 job_merge_into_installed(uj, j);
                                 log_unit_debug(uj->unit,
                                                "Merged %s/%s into installed job %s/%s as %"PRIu32,
diff --git a/src/core/job.h b/src/core/job.h
index 7603f311f2..f3daf9d094 100644
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -158,13 +158,18 @@ struct Job {
 
         bool installed:1;
         bool in_run_queue:1;
+
         bool matters_to_anchor:1;
-        bool in_dbus_queue:1;
-        bool sent_dbus_new_signal:1;
+        bool refuse_late_merge:1;
         bool ignore_order:1;
         bool irreversible:1;
-        bool in_gc_queue:1;
+
+        bool in_dbus_queue:1;
+        bool sent_dbus_new_signal:1;
+
         bool ref_by_private_bus:1;
+
+        bool in_gc_queue:1;
 };
 
 Job* job_new(Unit *unit, JobType type);
@@ -173,7 +178,7 @@ void job_unlink(Job *job);
 Job* job_free(Job *job);
 DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free);
 
-Job* job_install(Job *j, bool refuse_late_merge);
+Job* job_install(Job *j);
 int job_install_deserialized(Job *j);
 void job_uninstall(Job *j);
 
diff --git a/src/core/service.c b/src/core/service.c
index b9c12140d3..1185e2fbcf 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -47,6 +47,7 @@
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
+#include "transaction.h"
 #include "unit-name.h"
 #include "unit.h"
 #include "utf8.h"
@@ -2646,8 +2647,18 @@ static void service_enter_restart(Service *s, bool shortcut) {
         }
 
         /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves
-         * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. */
-        r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, &error, /* ret = */ NULL);
+         * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs.
+         *
+         * When RestartMode=direct is used, the service being restarted don't enter the inactive/failed state,
+         * i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous job might still
+         * be running (especially for Type=oneshot services).
+         * We need to refuse late merge and re-enqueue the anchor job. */
+        r = manager_add_job_full(UNIT(s)->manager,
+                                 JOB_START, UNIT(s),
+                                 JOB_RESTART_DEPENDENCIES,
+                                 TRANSACTION_REENQUEUE_ANCHOR,
+                                 /* affected_jobs = */ NULL,
+                                 &error, /* ret = */ NULL);
         if (r < 0) {
                 log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r));
                 return service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false);
diff --git a/src/core/transaction.c b/src/core/transaction.c
index 1ab59b823e..377ea05983 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -620,6 +620,9 @@ static int transaction_apply(
         Job *j;
         int r;
 
+        assert(tr);
+        assert(m);
+
         /* Moves the transaction jobs to the set of active jobs */
 
         if (IN_SET(mode, JOB_ISOLATE, JOB_FLUSH)) {
@@ -658,12 +661,7 @@ static int transaction_apply(
                 /* Clean the job dependencies */
                 transaction_unlink_job(tr, j, false);
 
-                /* When RestartMode=direct is used, the service being restarted don't enter the inactive/failed
-                 * state, i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous
-                 * job might still be running (especially for Type=oneshot services). We need to refuse
-                 * late merge and re-enqueue the anchor job. */
-                installed_job = job_install(j,
-                                            /* refuse_late_merge = */ mode == JOB_RESTART_DEPENDENCIES && j == tr->anchor_job);
+                installed_job = job_install(j);
                 if (installed_job != j) {
                         /* j has been merged into a previously installed job */
                         if (tr->anchor_job == j)
@@ -705,10 +703,10 @@ int transaction_activate(
         int r;
         unsigned generation = 1;
 
-        assert(tr);
+        /* This applies the changes recorded in tr->jobs to the actual list of jobs, if possible. */
 
-        /* This applies the changes recorded in tr->jobs to
-         * the actual list of jobs, if possible. */
+        assert(tr);
+        assert(m);
 
         /* Reset the generation counter of all installed jobs. The detection of cycles
          * looks at installed jobs. If they had a non-zero generation from some previous
@@ -739,7 +737,6 @@ int transaction_activate(
                 r = transaction_verify_order(tr, &generation, e);
                 if (r >= 0)
                         break;
-
                 if (r != -EAGAIN)
                         return log_warning_errno(r, "Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error_message(e, r));
 
@@ -754,7 +751,6 @@ int transaction_activate(
                 r = transaction_merge_jobs(tr, e);
                 if (r >= 0)
                         break;
-
                 if (r != -EAGAIN)
                         return log_warning_errno(r, "Requested transaction contains unmergeable jobs: %s", bus_error_message(e, r));
 
@@ -817,9 +813,6 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b
         if (!j)
                 return NULL;
 
-        j->generation = 0;
-        j->marker = NULL;
-        j->matters_to_anchor = false;
         j->irreversible = tr->irreversible;
 
         LIST_PREPEND(transaction, f, j);
@@ -944,6 +937,7 @@ int transaction_add_job_and_dependencies(
         int r;
 
         assert(tr);
+        assert(type >= 0);
         assert(type < _JOB_TYPE_MAX);
         assert(type < _JOB_TYPE_MAX_IN_TRANSACTION);
         assert(unit);
@@ -1008,6 +1002,9 @@ int transaction_add_job_and_dependencies(
                 /* If the job has no parent job, it is the anchor job. */
                 assert(!tr->anchor_job);
                 tr->anchor_job = ret;
+
+                if (FLAGS_SET(flags, TRANSACTION_REENQUEUE_ANCHOR))
+                        ret->refuse_late_merge = true;
         }
 
         if (!is_new || FLAGS_SET(flags, TRANSACTION_IGNORE_REQUIREMENTS) || type == JOB_NOP)
diff --git a/src/core/transaction.h b/src/core/transaction.h
index f2bf047f10..b6728a35d7 100644
--- a/src/core/transaction.h
+++ b/src/core/transaction.h
@@ -33,7 +33,10 @@ typedef enum TransactionAddFlags {
         /* Indicate that we're in the recursion for processing UNIT_ATOM_PROPAGATE_STOP_GRACEFUL units */
         TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL = 1 << 5,
 
-        _TRANSACTION_FLAGS_MASK_PUBLIC              = 0,
+        /* Always re-enqueue anchor job (refuse late merge) */
+        TRANSACTION_REENQUEUE_ANCHOR                = 1 << 6,
+
+        _TRANSACTION_FLAGS_MASK_PUBLIC              = TRANSACTION_REENQUEUE_ANCHOR,
 } TransactionAddFlags;
 
 void transaction_add_propagate_reload_jobs(

From 7a13937007ccc7246b8351c7d31d17252b9940a0 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 19:59:47 +0200
Subject: [PATCH 6/7] core/service: don't propagate stop jobs if
 RestartMode=direct

The goal of RestartMode=direct is to make restarts invisible
to dependents, so auto restart jobs shouldn't bring them down
at all. So far we only skipped going through failed/dead states
in service_enter_dead(), i.e. the unit would never be considered
dead. But when constructing restart transaction, the stop job
would be propagated to dependents. Consider the following 2 units:

dependent.target:
[Unit]
BindsTo=a.service
After=a.service

a.service:
[Service]
ExecStart=bash -c 'sleep 100 && exit 1'
Restart=on-failure
RestartMode=direct

Before this commit, even though BindsTo= isn't triggered since
a.service never failed, when a.service auto-restarts, dependent.target
is also restarted. Let's suppress it by using JOB_REPLACE instead of
JOB_RESTART_DEPENDENCIES in service_enter_restart().

Fixes #34758

The example above is subtly different from the original report,
to illustrate that the new behavior makes sense for less exotic
use cases too.
---
 man/systemd.service.xml | 2 +-
 src/core/service.c      | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 3066c91955..a5c0dc84e1 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -942,7 +942,7 @@
               <listitem>
                 <para>If set to <option>direct</option>, the service transitions to the activating
                 state directly during auto-restart, skipping failed/inactive state.
-                <varname>ExecStopPost=</varname> is invoked.
+                <varname>ExecStopPost=</varname> is still invoked.
                 <varname>OnSuccess=</varname> and <varname>OnFailure=</varname> are skipped.</para>
 
                 <para>This option is useful in cases where a dependency can fail temporarily but we don't
diff --git a/src/core/service.c b/src/core/service.c
index 1185e2fbcf..8cea17f853 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2646,16 +2646,17 @@ static void service_enter_restart(Service *s, bool shortcut) {
                 return;
         }
 
-        /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves
-         * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs.
+        /* Any units that are bound to this service must also be restarted, unless RestartMode=direct.
+         * We use JOB_START for ourselves but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART
+         * for those dependency jobs in the former case, plain JOB_REPLACE when RestartMode=direct.
          *
-         * When RestartMode=direct is used, the service being restarted don't enter the inactive/failed state,
+         * Also, when RestartMode=direct is used, the service being restarted don't enter the inactive/failed state,
          * i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous job might still
          * be running (especially for Type=oneshot services).
          * We need to refuse late merge and re-enqueue the anchor job. */
         r = manager_add_job_full(UNIT(s)->manager,
                                  JOB_START, UNIT(s),
-                                 JOB_RESTART_DEPENDENCIES,
+                                 s->restart_mode == SERVICE_RESTART_MODE_DIRECT ? JOB_REPLACE : JOB_RESTART_DEPENDENCIES,
                                  TRANSACTION_REENQUEUE_ANCHOR,
                                  /* affected_jobs = */ NULL,
                                  &error, /* ret = */ NULL);

From 9b42c58a2b0f0703ff48cd8e8b37eee1a36c1887 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Mon, 14 Oct 2024 22:16:17 +0200
Subject: [PATCH 7/7] TEST-03-JOBS: add test case for #34758

---
 .../TEST-03-JOBS.units/counter.service        |  8 ++++++
 .../TEST-03-JOBS.units/counter.sh             | 13 ++++++++++
 .../restart-on-failure.service                |  4 +++
 .../sleep-infinity-restart-direct.service     |  8 ++++++
 .../sleep-infinity-restart-normal.service     |  8 ++++++
 test/units/TEST-03-JOBS.sh                    | 25 +++++++++++++++++++
 6 files changed, 66 insertions(+)
 create mode 100644 test/TEST-03-JOBS/TEST-03-JOBS.units/counter.service
 create mode 100755 test/TEST-03-JOBS/TEST-03-JOBS.units/counter.sh
 create mode 100644 test/TEST-03-JOBS/TEST-03-JOBS.units/restart-on-failure.service
 create mode 100644 test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-direct.service
 create mode 100644 test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-normal.service

diff --git a/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.service b/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.service
new file mode 100644
index 0000000000..db2e49404c
--- /dev/null
+++ b/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.service
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+BindsTo=sleep-infinity-restart-normal.service sleep-infinity-restart-direct.service
+After=sleep-infinity-restart-normal.service sleep-infinity-restart-direct.service
+
+[Service]
+Type=notify
+ExecStart=/usr/lib/systemd/tests/testdata/TEST-03-JOBS.units/counter.sh
diff --git a/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.sh b/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.sh
new file mode 100755
index 0000000000..1d11ca613d
--- /dev/null
+++ b/test/TEST-03-JOBS/TEST-03-JOBS.units/counter.sh
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -eux
+
+COUNTER_FILE=/tmp/test-03-restart-counter
+
+COUNT="$(<"$COUNTER_FILE")"
+: $(( COUNT++ ))
+echo "$COUNT" >"$COUNTER_FILE"
+
+systemd-notify --ready
+
+sleep infinity
diff --git a/test/TEST-03-JOBS/TEST-03-JOBS.units/restart-on-failure.service b/test/TEST-03-JOBS/TEST-03-JOBS.units/restart-on-failure.service
new file mode 100644
index 0000000000..1ec1857e39
--- /dev/null
+++ b/test/TEST-03-JOBS/TEST-03-JOBS.units/restart-on-failure.service
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Service]
+Type=oneshot
+ExecStart=touch /tmp/test-03-restart-failure-flag
diff --git a/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-direct.service b/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-direct.service
new file mode 100644
index 0000000000..00316db1f1
--- /dev/null
+++ b/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-direct.service
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+OnFailure=restart-on-failure.service
+
+[Service]
+ExecStart=/bin/sleep infinity
+Restart=on-failure
+RestartMode=direct
diff --git a/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-normal.service b/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-normal.service
new file mode 100644
index 0000000000..dabba4c4f0
--- /dev/null
+++ b/test/TEST-03-JOBS/TEST-03-JOBS.units/sleep-infinity-restart-normal.service
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+OnFailure=restart-on-failure.service
+
+[Service]
+ExecStart=/bin/sleep infinity
+Restart=on-failure
+RestartMode=normal
diff --git a/test/units/TEST-03-JOBS.sh b/test/units/TEST-03-JOBS.sh
index 067f2ce67d..183439e913 100755
--- a/test/units/TEST-03-JOBS.sh
+++ b/test/units/TEST-03-JOBS.sh
@@ -156,6 +156,7 @@ assert_rc 3 systemctl --quiet is-active propagatestopto-and-pullin.target
 assert_rc 3 systemctl --quiet is-active sleep-infinity-simple.service
 
 # Test restart mode direct
+
 systemctl start succeeds-on-restart-restartdirect.target
 assert_rc 0 systemctl --quiet is-active succeeds-on-restart-restartdirect.target
 
@@ -168,6 +169,30 @@ assert_rc 3 systemctl --quiet is-active succeeds-on-restart.target
 systemctl start fails-on-restart.target || :
 assert_rc 3 systemctl --quiet is-active fails-on-restart.target
 
+COUNTER_FILE=/tmp/test-03-restart-counter
+export FAILURE_FLAG_FILE=/tmp/test-03-restart-failure-flag
+
+assert_rc 3 systemctl --quiet is-active sleep-infinity-restart-normal.service
+assert_rc 3 systemctl --quiet is-active sleep-infinity-restart-direct.service
+assert_rc 3 systemctl --quiet is-active counter.service
+echo 0 >"$COUNTER_FILE"
+
+systemctl start counter.service
+assert_eq "$(cat "$COUNTER_FILE")" "1"
+systemctl --quiet is-active sleep-infinity-restart-normal.service
+systemctl --quiet is-active sleep-infinity-restart-direct.service
+systemctl --quiet is-active counter.service
+
+systemctl kill --signal=KILL sleep-infinity-restart-direct.service
+systemctl --quiet is-active counter.service
+assert_eq "$(cat "$COUNTER_FILE")" "1"
+[[ ! -f "$FAILURE_FLAG_FILE" ]]
+
+systemctl kill --signal=KILL sleep-infinity-restart-normal.service
+timeout 10 bash -c 'while [[ ! -f $FAILURE_FLAG_FILE ]]; do sleep .5; done'
+timeout 10 bash -c 'while ! systemctl --quiet is-active counter.service; do sleep .5; done'
+assert_eq "$(cat "$COUNTER_FILE")" "2"
+
 # Test shortcutting auto restart
 
 export UNIT_NAME="TEST-03-JOBS-shortcut-restart.service"