job: allow job_free() only on already unlinked jobs

job_free() is IMO too helpful when it unlinks the job from the transaction.
The callers should ensure the job is already unlinked before freeing.
The added assertions check if anyone gets it wrong.

diff --git a/src/core/job.c b/src/core/job.c
index f3c76d66b59dd85f1acd7948ca290c8ef7c9d0d3..5ea717eae1e58b421f07fc8e905865c6d065f110 100644 (file)
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -71,8 +71,10 @@ void job_free(Job *j) {
                 j->installed = false;
         }
 
-        /* Detach from next 'smaller' objects */
-        manager_transaction_unlink_job(j->manager, j, true);
+        assert(!j->transaction_prev);
+        assert(!j->transaction_next);
+        assert(!j->subject_list);
+        assert(!j->object_list);
 
         if (j->in_run_queue)
                 LIST_REMOVE(Job, run_queue, j->manager->run_queue, j);

diff --git a/src/core/manager.c b/src/core/manager.c
index 95056296ad21d9f4949136d3d135dfb5cb82fcf1..aa918f1bd0d0d1cde30c4a69da2358af01564aab 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -662,13 +662,15 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         return r;
 }
 
+static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies);
+
 static void transaction_delete_job(Manager *m, Job *j, bool delete_dependencies) {
         assert(m);
         assert(j);
 
         /* Deletes one job from the transaction */
 
-        manager_transaction_unlink_job(m, j, delete_dependencies);
+        transaction_unlink_job(m, j, delete_dependencies);
 
         if (!j->installed)
                 job_free(j);
@@ -710,8 +712,10 @@ static void transaction_abort(Manager *m) {
         while ((j = hashmap_first(m->transaction_jobs)))
                 if (j->installed)
                         transaction_delete_job(m, j, true);
-                else
+                else {
+                        transaction_unlink_job(m, j, true);
                         job_free(j);
+                }
 
         assert(hashmap_isempty(m->transaction_jobs));
 
@@ -1441,6 +1445,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o
         LIST_PREPEND(Job, transaction, f, j);
 
         if (hashmap_replace(m->transaction_jobs, unit, f) < 0) {
+                LIST_REMOVE(Job, transaction, f, j);
                 job_free(j);
                 return NULL;
         }
@@ -1453,7 +1458,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o
         return j;
 }
 
-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) {
+static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) {
         assert(m);
         assert(j);
 

diff --git a/src/core/manager.h b/src/core/manager.h
index 39e16aee98d9915eab858535587e9afa0d200abc..0e9c0d7b4ea264d7669fea822f611d7e7e73abb5 100644 (file)
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -257,8 +257,6 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode
 void manager_dump_units(Manager *s, FILE *f, const char *prefix);
 void manager_dump_jobs(Manager *s, FILE *f, const char *prefix);
 
-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies);
-
 void manager_clear_jobs(Manager *m);
 
 unsigned manager_dispatch_load_queue(Manager *m);