Magellan Linux

  • Back to homepage
    • /[pkg-src]/trunk/systemd/patches/systemd-37-manager-fix-a-crash-in-isolating.patch

    Annotation of /trunk/systemd/patches/systemd-37-manager-fix-a-crash-in-isolating.patch

    Parent Directory Parent Directory | Revision Log Revision Log

    Revision 1562 - (hide annotations) (download)
    Thu Nov 17 14:00:05 2011 UTC (12 years, 7 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