Annotation of /trunk/systemd/patches/systemd-37-manager-fix-a-crash-in-isolating.patch
Parent Directory | Revision Log
Revision 1562 -
(hide 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 | niro | 1562 | 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 |