From ca6a61f432f3ff0257e868bed36a58540623ab49 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 11 Dec 2020 20:53:32 +0300 Subject: Fix bug in create_new_target_locked() --- libbuild2/algorithm.cxx | 2 +- libbuild2/bash/rule.cxx | 2 +- libbuild2/cc/common.txx | 2 +- libbuild2/cc/compile-rule.cxx | 4 ++-- libbuild2/file.cxx | 2 +- libbuild2/search.cxx | 11 +++++++++-- libbuild2/target.hxx | 16 +++++++++++++++- libbuild2/target.ixx | 20 +++++++++++++++++++- 8 files changed, 49 insertions(+), 10 deletions(-) diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index f859eef..39df018 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -312,7 +312,7 @@ namespace build2 target_decl::implied, trace)); - assert (r.second.owns_lock ()); + assert (r.second); target& m (r.first); *mp = &m; diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx index 9fc89d7..8b47fb0 100644 --- a/libbuild2/bash/rule.cxx +++ b/libbuild2/bash/rule.cxx @@ -168,7 +168,7 @@ namespace build2 // Only set path/mtime on first insertion. // - if (rp.second.owns_lock ()) + if (rp.second) pt.path_mtime (move (ap), mt); // Save the length of the import path in auxuliary data. We diff --git a/libbuild2/cc/common.txx b/libbuild2/cc/common.txx index ce922cc..d14f966 100644 --- a/libbuild2/cc/common.txx +++ b/libbuild2/cc/common.txx @@ -27,7 +27,7 @@ namespace build2 target_decl::implied, trace)); - assert (!exist || !p.second.owns_lock ()); + assert (!exist || !p.second); r = &p.first.template as (); return move (p.second); } diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index a11b7ab..b96c39d 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -6051,7 +6051,7 @@ namespace build2 // Note that this is racy and someone might have created this target // while we were preparing the prerequisite list. // - if (p.second.owns_lock ()) + if (p.second) { bt.prerequisites (move (ps)); @@ -6129,7 +6129,7 @@ namespace build2 // Note that this is racy and someone might have created this target // while we were preparing the prerequisite list. // - if (p.second.owns_lock ()) + if (p.second) bt.prerequisites (move (ps)); return bt; diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index def49b8..219a5f1 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -2682,7 +2682,7 @@ namespace build2 // a simple name via PATH search and as an absolute path in which case // the first import will determine the path). // - if (r.second.owns_lock ()) + if (r.second) { r.first.as ().process_path (move (pp)); r.second.unlock (); diff --git a/libbuild2/search.cxx b/libbuild2/search.cxx index b341c85..25a4199 100644 --- a/libbuild2/search.cxx +++ b/libbuild2/search.cxx @@ -278,9 +278,16 @@ namespace build2 tk.ext, target_decl::prereq_new, trace)); + l5 ([&] + { + diag_record dr (trace); + if (r.second) + dr << "new target " << r.first.key_locked (); + else + dr << "existing target " << r.first; + dr << " for prerequisite " << pk; + }); - l5 ([&]{trace << (r.second ? "new" : "existing") << " target " << r.first - << " for prerequisite " << pk;}); return r; } } diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index e1d91e4..8c7ccee 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -147,6 +147,10 @@ namespace build2 const string* ext () const; // Return NULL if not specified. const string& ext (string); + // As above but assume targets mutex is locked. + // + const string* ext_locked () const; + const dir_path& out_dir () const {return out.empty () ? dir : out;} @@ -284,6 +288,11 @@ namespace build2 target_key key () const; + // As above but assume targets mutex is locked. + // + target_key + key_locked () const; + names as_name () const; @@ -811,6 +820,11 @@ namespace build2 inline bool operator!= (const target& x, const target& y) {return !(x == y);} + // Note that if the targets mutex is locked, then calling this operator + // will lead to a deadlock. Instead, do: + // + // ... << t.key_locked () << ... + // ostream& operator<< (ostream&, const target&); @@ -1385,7 +1399,7 @@ namespace build2 decl, t)); - return pair (p.first, p.second.owns_lock ()); + return pair (p.first, p.second); } // Note that the following versions always enter implied targets. diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx index df1cf5b..2d1906e 100644 --- a/libbuild2/target.ixx +++ b/libbuild2/target.ixx @@ -12,10 +12,16 @@ namespace build2 // target // inline const string* target:: + ext_locked () const + { + return *ext_ ? &**ext_ : nullptr; + } + + inline const string* target:: ext () const { slock l (ctx.targets.mutex_); - return *ext_ ? &**ext_ : nullptr; + return ext_locked (); } inline target_key target:: @@ -30,6 +36,18 @@ namespace build2 e != nullptr ? optional (*e) : nullopt}; } + inline target_key target:: + key_locked () const + { + const string* e (ext_locked ()); + return target_key { + &type (), + &dir, + &out, + &name, + e != nullptr ? optional (*e) : nullopt}; + } + inline names target:: as_name () const { -- cgit v1.1