job: job_uninstall()

Split the uninstallation of the job from job_free() into a separate function.
Adjust the callers.

job_free() now only works on unlinked and uninstalled jobs. This enforces clear
thinking about job lifetimes.

4 files changed, 26 insertions, 15 deletions

diff --git a/src/core/job.c b/src/core/job.c
index 5ea717eae1..9199cf6a4b 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -55,22 +55,24 @@ Job* job_new(Manager *m, JobType type, Unit *unit) {
         return j;
 }
 
-void job_free(Job *j) {
-        assert(j);
-
+void job_uninstall(Job *j) {
+        assert(j->installed);
         /* Detach from next 'bigger' objects */
-        if (j->installed) {
-                bus_job_send_removed_signal(j);
 
-                if (j->unit->job == j) {
-                        j->unit->job = NULL;
-                        unit_add_to_gc_queue(j->unit);
-                }
+        bus_job_send_removed_signal(j);
 
-                hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id));
-                j->installed = false;
+        if (j->unit->job == j) {
+                j->unit->job = NULL;
+                unit_add_to_gc_queue(j->unit);
         }
 
+        hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id));
+        j->installed = false;
+}
+
+void job_free(Job *j) {
+        assert(j);
+        assert(!j->installed);
         assert(!j->transaction_prev);
         assert(!j->transaction_next);
         assert(!j->subject_list);
@@ -492,6 +494,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
 
         u = j->unit;
         t = j->type;
+        job_uninstall(j);
         job_free(j);
 
         job_print_status_message(u, t, result);
diff --git a/src/core/job.h b/src/core/job.h
index 3ce2d65a0d..442b13ef24 100644
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -137,6 +137,7 @@ struct Job {
 };
 
 Job* job_new(Manager *m, JobType type, Unit *unit);
+void job_uninstall(Job *j);
 void job_free(Job *job);
 void job_dump(Job *j, FILE*f, const char *prefix);
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 546b3233b0..445931cb83 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1275,13 +1275,17 @@ static int transaction_apply(Manager *m, JobMode mode) {
         }
 
         while ((j = hashmap_steal_first(m->transaction_jobs))) {
+                Job *uj;
                 if (j->installed) {
                         /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */
                         continue;
                 }
 
-                if (j->unit->job)
-                        job_free(j->unit->job);
+                uj = j->unit->job;
+                if (uj) {
+                        job_uninstall(uj);
+                        job_free(uj);
+                }
 
                 j->unit->job = j;
                 j->installed = true;
diff --git a/src/core/unit.c b/src/core/unit.c
index df79fe3a69..ed3f176393 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -352,8 +352,11 @@ void unit_free(Unit *u) {
         SET_FOREACH(t, u->names, i)
                 hashmap_remove_value(u->manager->units, t, u);
 
-        if (u->job)
-                job_free(u->job);
+        if (u->job) {
+                Job *j = u->job;
+                job_uninstall(j);
+                job_free(j);
+        }
 
         for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++)
                 bidi_set_free(u, u->dependencies[d]);