diff options
| -rw-r--r-- | libbuild2/adhoc-rule-regex-pattern.cxx | 36 | ||||
| -rw-r--r-- | libbuild2/dyndep.cxx | 58 | ||||
| -rw-r--r-- | libbuild2/target.hxx | 7 |
3 files changed, 64 insertions, 37 deletions
diff --git a/libbuild2/adhoc-rule-regex-pattern.cxx b/libbuild2/adhoc-rule-regex-pattern.cxx index 0f2ecc3..c4b4cab 100644 --- a/libbuild2/adhoc-rule-regex-pattern.cxx +++ b/libbuild2/adhoc-rule-regex-pattern.cxx @@ -400,21 +400,31 @@ namespace build2 if (find (ms.begin (), ms.end (), &t) != ms.end ()) continue; - // We can only update the group under lock. + // Check if we already belong to this group. Note that this not a + // mere optimization since we may be in the member->group->member + // chain and trying to lock the member the second time would + // deadlock (this can be triggered, for example, by dist, which sort + // of depends on such members directly @@ maybe this should be fixed + // there?). // - target_lock tl (lock (a, t)); - - if (!tl) - fail << "group " << *g << " member " << t << " is already matched" << - info << "static group members specified by pattern rules cannot " - << "be used as prerequisites directly, only via group"; - - if (t.group == nullptr) - tl.target->group = g; - else if (t.group != g) + if (t.group != g) // Note: atomic. { - fail << "group " << *g << " member " << t - << " is already member of group " << *t.group; + // We can only update the group under lock. + // + target_lock tl (lock (a, t)); + + if (!tl) + fail << "group " << *g << " member " << t << " is already matched" << + info << "static group members specified by pattern rules cannot " + << "be used as prerequisites directly, only via group"; + + if (t.group == nullptr) + tl.target->group = g; + else if (t.group != g) + { + fail << "group " << *g << " member " << t + << " is already member of group " << *t.group; + } } } diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index 8781812..5ac9648 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -834,6 +834,9 @@ namespace build2 const function<dyndep_rule::group_filter_func>& fl, bool skip_match) { + // NOTE: see adhoc_rule_regex_pattern::apply_group_members() for a variant + // of the same code. + // We expect that nobody else can insert these members (seems reasonable // seeing that their names are dynamically discovered). // @@ -870,33 +873,44 @@ namespace build2 return pair<const file&, bool> (t, false); } - // This shouldn't normally fail since we are the only ones that should - // know about this target (otherwise why is it dynamicaly discovered). - // However, nothing prevents the user from depending on such a target, - // however misguided. + // Check if we already belong to this group. Note that this not a mere + // optimization since we may be in the member->group->member chain and + // trying to lock the member the second time would deadlock (this can be + // triggered, for example, by dist, which sort of depends on such members + // directly @@ maybe this should be fixed there?). // - target_lock tl (lock (a, t)); + if (t.group == &g && skip_match) // Note: group query is atomic. + t.path (move (f)); + else + { + // This shouldn't normally fail since we are the only ones that should + // know about this target (otherwise why is it dynamicaly discovered). + // However, nothing prevents the user from depending on such a target, + // however misguided. + // + target_lock tl (lock (a, t)); - if (!tl) - fail << "group " << g << " member " << t << " is already matched" << - info << "dynamically extracted group members cannot be used as " - << "prerequisites directly, only via group"; + if (!tl) + fail << "group " << g << " member " << t << " is already matched" << + info << "dynamically extracted group members cannot be used as " + << "prerequisites directly, only via group"; - if (!locked) - { - if (t.group == nullptr) - tl.target->group = &g; - else if (t.group != &g) - fail << "group " << g << " member " << t - << " is already member of group " << *t.group; - } + if (!locked) + { + if (t.group == nullptr) + tl.target->group = &g; + else if (t.group != &g) + fail << "group " << g << " member " << t + << " is already member of group " << *t.group; + } - t.path (move (f)); + t.path (move (f)); - if (!skip_match) - { - match_inc_dependents (a, g); - match_recipe (tl, group_recipe); + if (!skip_match) + { + match_inc_dependents (a, g); + match_recipe (tl, group_recipe); + } } return pair<const file&, bool> (t, true); diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 3f73e63..c7b1131 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -380,9 +380,12 @@ namespace build2 // // Note that the group-member link-up can happen anywhere between the // member creation and rule matching so reading the group before the - // member has been matched can be racy. + // member has been matched can be racy. However, once the member is linked + // up to the group, this relationship is immutable. As a result, one can + // atomically query the current value to see if already linked up (can be + // used as an optimization, to avoid deadlocks, etc). // - const target* group = nullptr; + relaxed_atomic<const target*> group = nullptr; // What has been described above is an "explicit" group. That is, there is // a dedicated target type that explicitly serves as a group and there is |