Contents of /trunk/systemd/patches/systemd-37-manager-fix-a-crash-in-isolating.patch
Parent Directory | Revision Log
Revision 1562 -
(show annotations)
(download)
Thu Nov 17 14:00:05 2011 UTC (12 years, 10 months ago) by niro
File size: 5203 byte(s)
Thu Nov 17 14:00:05 2011 UTC (12 years, 10 months ago) by niro
File size: 5203 byte(s)
fedora and upstream fixes
1 | From 563ba9ea6e60774086555998b957edf923e24b46 Mon Sep 17 00:00:00 2001 |
2 | From: Michal Schmidt <mschmidt@redhat.com> |
3 | Date: Mon, 17 Oct 2011 11:12:12 +0200 |
4 | Subject: [PATCH 2/5] manager: fix a crash in isolating |
5 | |
6 | HASHMAP_FOREACH is safe against the removal of the current entry, but |
7 | not against the removal of other entries. job_finish_and_invalidate() |
8 | can recursively remove other entries. |
9 | |
10 | It triggered an assertion failure: |
11 | Assertion 'j->installed' failed at src/manager.c:1218, function |
12 | transaction_apply(). Aborting. |
13 | |
14 | Fix the crash by iterating from the beginning when there is a |
15 | possibility that the iterator could be invalid. |
16 | |
17 | It is O(n^2) in the worst case, but that's better than a crash. |
18 | |
19 | https://bugzilla.redhat.com/show_bug.cgi?id=717325 |
20 | --- |
21 | src/job.c | 19 ++++++++++++++----- |
22 | src/manager.c | 7 ++++++- |
23 | 2 files changed, 20 insertions(+), 6 deletions(-) |
24 | |
25 | diff --git a/src/job.c b/src/job.c |
26 | index 5c0913b..20971da 100644 |
27 | --- a/src/job.c |
28 | +++ b/src/job.c |
29 | @@ -527,6 +527,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) { |
30 | Unit *other; |
31 | JobType t; |
32 | Iterator i; |
33 | + bool recursed = false; |
34 | |
35 | assert(j); |
36 | assert(j->installed); |
37 | @@ -573,23 +574,29 @@ int job_finish_and_invalidate(Job *j, JobResult result) { |
38 | if (other->meta.job && |
39 | (other->meta.job->type == JOB_START || |
40 | other->meta.job->type == JOB_VERIFY_ACTIVE || |
41 | - other->meta.job->type == JOB_RELOAD_OR_START)) |
42 | + other->meta.job->type == JOB_RELOAD_OR_START)) { |
43 | job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); |
44 | + recursed = true; |
45 | + } |
46 | |
47 | SET_FOREACH(other, u->meta.dependencies[UNIT_BOUND_BY], i) |
48 | if (other->meta.job && |
49 | (other->meta.job->type == JOB_START || |
50 | other->meta.job->type == JOB_VERIFY_ACTIVE || |
51 | - other->meta.job->type == JOB_RELOAD_OR_START)) |
52 | + other->meta.job->type == JOB_RELOAD_OR_START)) { |
53 | job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); |
54 | + recursed = true; |
55 | + } |
56 | |
57 | SET_FOREACH(other, u->meta.dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i) |
58 | if (other->meta.job && |
59 | !other->meta.job->override && |
60 | (other->meta.job->type == JOB_START || |
61 | other->meta.job->type == JOB_VERIFY_ACTIVE || |
62 | - other->meta.job->type == JOB_RELOAD_OR_START)) |
63 | + other->meta.job->type == JOB_RELOAD_OR_START)) { |
64 | job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); |
65 | + recursed = true; |
66 | + } |
67 | |
68 | } else if (t == JOB_STOP) { |
69 | |
70 | @@ -597,8 +604,10 @@ int job_finish_and_invalidate(Job *j, JobResult result) { |
71 | if (other->meta.job && |
72 | (other->meta.job->type == JOB_START || |
73 | other->meta.job->type == JOB_VERIFY_ACTIVE || |
74 | - other->meta.job->type == JOB_RELOAD_OR_START)) |
75 | + other->meta.job->type == JOB_RELOAD_OR_START)) { |
76 | job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); |
77 | + recursed = true; |
78 | + } |
79 | } |
80 | } |
81 | |
82 | @@ -626,7 +635,7 @@ finish: |
83 | |
84 | manager_check_finished(u->meta.manager); |
85 | |
86 | - return 0; |
87 | + return recursed; |
88 | } |
89 | |
90 | int job_start_timer(Job *j) { |
91 | diff --git a/src/manager.c b/src/manager.c |
92 | index e626347..6d20258 100644 |
93 | --- a/src/manager.c |
94 | +++ b/src/manager.c |
95 | @@ -1214,13 +1214,18 @@ static int transaction_apply(Manager *m, JobMode mode) { |
96 | |
97 | /* When isolating first kill all installed jobs which |
98 | * aren't part of the new transaction */ |
99 | + rescan: |
100 | HASHMAP_FOREACH(j, m->jobs, i) { |
101 | assert(j->installed); |
102 | |
103 | if (hashmap_get(m->transaction_jobs, j->unit)) |
104 | continue; |
105 | |
106 | - job_finish_and_invalidate(j, JOB_CANCELED); |
107 | + /* 'j' itself is safe to remove, but if other jobs |
108 | + are invalidated recursively, our iterator may become |
109 | + invalid and we need to start over. */ |
110 | + if (job_finish_and_invalidate(j, JOB_CANCELED) > 0) |
111 | + goto rescan; |
112 | } |
113 | } |
114 | |
115 | -- |
116 | 1.7.4.4 |
117 |