From 63f6a8256e3f9fb47cb941be63baa70e2be48d3b Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 2 Feb 2017 15:35:44 +0200 Subject: Implement target_set locking, including extension update --- build2/target.cxx | 172 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 131 insertions(+), 41 deletions(-) (limited to 'build2/target.cxx') diff --git a/build2/target.cxx b/build2/target.cxx index 10b1432..b250597 100644 --- a/build2/target.cxx +++ b/build2/target.cxx @@ -56,6 +56,31 @@ namespace build2 clear_data (); } + const string& target:: + ext (string v) + { + ulock l (targets.mutex_); + + // Once the extension is set, it is immutable. However, it is possible + // that someone has already "branded" this target with a different + // extension. + // + optional& e (*ext_); + + if (!e) + e = move (v); + else if (*e != v) + { + string o (*e); + l.unlock (); + + fail << "conflicting extensions '" << o << "' and '" << v << "' " + << "for target " << *this; + } + + return *e; + } + void target:: recipe (action_type a, recipe_type r) { @@ -201,34 +226,54 @@ namespace build2 target* target_set:: find (const target_key& k, tracer& trace) const { - target* t (nullptr); + slock sl (mutex_); map_type::const_iterator i (map_.find (k)); - if (i != map_.end ()) + if (i == map_.end ()) + return nullptr; + + target& t (*i->second); + optional& ext (i->first.ext); + + if (ext != k.ext) { - t = i->second.get (); - optional& ext (i->first.ext); + ulock ul; // Keep locked for trace. - if (ext != k.ext) + if (k.ext) { - l5 ([&]{ - diag_record r (trace); - r << "assuming target " << *t << " is the same as the one with "; - if (!k.ext) - r << "unspecified extension"; - else if (k.ext->empty ()) - r << "no extension"; - else - r << "extension " << *k.ext; - }); + // To update the extension we have to re-lock for exclusive access. + // Between us releasing the shared lock and acquiring unique the + // extension could change and possibly a new target that matches the + // key could be inserted. In this case we simply re-run find (). + // + sl.unlock (); + ul = ulock (mutex_); + if (ext) // Someone set the extension. + { + ul.unlock (); + return find (k, trace); + } - if (k.ext) - ext = k.ext; + ext = k.ext; } + + l5 ([&]{ + diag_record r (trace); + r << "assuming target "; + to_stream (r.os, t.key (), 0); // Don't print the extension. + r << " is the same as the one with "; + + if (!k.ext) + r << "unspecified extension"; + else if (k.ext->empty ()) + r << "no extension"; + else + r << "extension " << *k.ext; + }); } - return t; + return &t; } pair target_set:: @@ -241,35 +286,78 @@ namespace build2 tracer& trace) { target_key tk {&tt, &dir, &out, &name, move (ext)}; - target* t (find (tk, trace)); - bool r (t == nullptr); - if (r) + if (t == nullptr) { - pair> p ( + pair> te ( tt.factory ( tt, move (dir), move (out), move (name), move (tk.ext))); - t = p.first; + t = te.first; + + // Re-lock for exclusive access. In the meantime, someone could have + // inserted this target so emplace() below could return false, in which + // case we proceed pretty much like find() except already under the + // exclusive lock. + // + ulock ul (mutex_); - map_type::iterator i ( + auto p ( map_.emplace ( - target_key {&tt, &t->dir, &t->out, &t->name, move (p.second)}, - p.first).first); + target_key {&tt, &t->dir, &t->out, &t->name, te.second}, + unique_ptr (te.first))); - t->ext_ = &i->first.ext; - t->implied = implied; + map_type::iterator i (p.first); + + if (p.second) + { + t->ext_ = &i->first.ext; + t->implied = implied; + return pair (*t, true); + } + + // The "tail" of find(). + // + t = i->second.get (); + optional& ext (i->first.ext); + + if (ext != te.second) + { + if (te.second) + ext = te.second; + + l5 ([&]{ + diag_record r (trace); + r << "assuming target "; + to_stream (r.os, t->key (), 0); // Don't print the extension. + r << " is the same as the one with "; + + if (!te.second) + r << "unspecified extension"; + else if (te.second->empty ()) + r << "no extension"; + else + r << "extension " << *te.second; + }); + } + + // Fall through (continue as if the first find() returned this target). } - else if (!implied) + + if (!implied) { + // The implied flag can only be cleared during the load phase. + // + assert (phase == run_phase::load); + // Clear the implied flag. // if (t->implied) t->implied = false; } - return pair (*t, r); + return pair (*t, false); } ostream& @@ -358,31 +446,33 @@ namespace build2 // assert (de == nullptr || type ().extension != nullptr); - optional& ext (*ext_); //@@ MT lock - - if (!ext) + if (const string* p = ext ()) + // Note that returning by reference is now MT-safe since once the + // extension is specified, it is immutable. + // + return *p; + else { + optional e; + // If the target type has the extension function then try that first. // The reason for preferring it over what's been provided by the caller // is that this function will often use the 'extension' variable which // the user can use to override extensions. // if (auto f = type ().extension) - ext = f (key (), base_scope (), search); + e = f (key (), base_scope (), search); - if (!ext) + if (!e) { if (de != nullptr) - ext = de; + e = de; else fail << "no default extension for target " << *this; } - } - // Note that returning by reference is now MT-safe since once the - // extension is specified, it is immutable. - // - return *ext; + return ext (move (*e)); + } } const path& path_target:: -- cgit v1.1