summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichal Schmidt <mschmidt@redhat.com>2011-10-17 11:12:12 +0200
committerMichal Schmidt <mschmidt@redhat.com>2011-10-17 11:29:27 +0200
commit563ba9ea6e60774086555998b957edf923e24b46 (patch)
tree8dd5598841b48768d2164c650c395228a7b42739
parent64685e0cea62b4937f0804e47ce2cb7929f58223 (diff)
manager: fix a crash in isolating
HASHMAP_FOREACH is safe against the removal of the current entry, but not against the removal of other entries. job_finish_and_invalidate() can recursively remove other entries. It triggered an assertion failure: Assertion 'j->installed' failed at src/manager.c:1218, function transaction_apply(). Aborting. Fix the crash by iterating from the beginning when there is a possibility that the iterator could be invalid. It is O(n^2) in the worst case, but that's better than a crash. https://bugzilla.redhat.com/show_bug.cgi?id=717325
-rw-r--r--src/job.c19
-rw-r--r--src/manager.c7
2 files changed, 20 insertions, 6 deletions
diff --git a/src/job.c b/src/job.c
index 5c0913b7d8..20971da852 100644
--- a/src/job.c
+++ b/src/job.c
@@ -527,6 +527,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
Unit *other;
JobType t;
Iterator i;
+ bool recursed = false;
assert(j);
assert(j->installed);
@@ -573,23 +574,29 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
if (other->meta.job &&
(other->meta.job->type == JOB_START ||
other->meta.job->type == JOB_VERIFY_ACTIVE ||
- other->meta.job->type == JOB_RELOAD_OR_START))
+ other->meta.job->type == JOB_RELOAD_OR_START)) {
job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
+ recursed = true;
+ }
SET_FOREACH(other, u->meta.dependencies[UNIT_BOUND_BY], i)
if (other->meta.job &&
(other->meta.job->type == JOB_START ||
other->meta.job->type == JOB_VERIFY_ACTIVE ||
- other->meta.job->type == JOB_RELOAD_OR_START))
+ other->meta.job->type == JOB_RELOAD_OR_START)) {
job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
+ recursed = true;
+ }
SET_FOREACH(other, u->meta.dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)
if (other->meta.job &&
!other->meta.job->override &&
(other->meta.job->type == JOB_START ||
other->meta.job->type == JOB_VERIFY_ACTIVE ||
- other->meta.job->type == JOB_RELOAD_OR_START))
+ other->meta.job->type == JOB_RELOAD_OR_START)) {
job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
+ recursed = true;
+ }
} else if (t == JOB_STOP) {
@@ -597,8 +604,10 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
if (other->meta.job &&
(other->meta.job->type == JOB_START ||
other->meta.job->type == JOB_VERIFY_ACTIVE ||
- other->meta.job->type == JOB_RELOAD_OR_START))
+ other->meta.job->type == JOB_RELOAD_OR_START)) {
job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
+ recursed = true;
+ }
}
}
@@ -626,7 +635,7 @@ finish:
manager_check_finished(u->meta.manager);
- return 0;
+ return recursed;
}
int job_start_timer(Job *j) {
diff --git a/src/manager.c b/src/manager.c
index e626347dec..6d20258893 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -1214,13 +1214,18 @@ static int transaction_apply(Manager *m, JobMode mode) {
/* When isolating first kill all installed jobs which
* aren't part of the new transaction */
+ rescan:
HASHMAP_FOREACH(j, m->jobs, i) {
assert(j->installed);
if (hashmap_get(m->transaction_jobs, j->unit))
continue;
- job_finish_and_invalidate(j, JOB_CANCELED);
+ /* 'j' itself is safe to remove, but if other jobs
+ are invalidated recursively, our iterator may become
+ invalid and we need to start over. */
+ if (job_finish_and_invalidate(j, JOB_CANCELED) > 0)
+ goto rescan;
}
}