aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2020-06-25 09:00:12 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2020-06-25 09:00:12 +0200
commita17b6b40510b8ec5ca18dd5203e4b229aa6fee8a (patch)
treefccf8e19b16775cbcf714a19a386bdc86ef1ec05
parent3875999b2642013dfddd6cb1fa7b3ee7ac14f9d7 (diff)
Add more instrumentation for unassigned path race
-rw-r--r--libbuild2/algorithm.hxx8
-rw-r--r--libbuild2/algorithm.ixx10
-rw-r--r--libbuild2/cc/compile-rule.cxx55
-rw-r--r--libbuild2/install/rule.cxx4
-rw-r--r--libbuild2/target.hxx12
5 files changed, 64 insertions, 25 deletions
diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx
index de09469..33bc624 100644
--- a/libbuild2/algorithm.hxx
+++ b/libbuild2/algorithm.hxx
@@ -287,8 +287,8 @@ namespace build2
// If it is unmatch::unchanged then only unmatch the target if it is known
// to be unchanged after match. If it is unmatch::safe, then unmatch the
// target if it is safe (this includes unchanged or if we know that someone
- // else will execute this target). Return true if unmatch succeeded. Always
- // throw if failed.
+ // else will execute this target). Return true in first half of the pair if
+ // unmatch succeeded. Always throw if failed.
//
enum class unmatch {none, unchanged, safe};
@@ -298,7 +298,7 @@ namespace build2
pair<bool, target_state>
try_match (action, const target&, bool fail = true);
- bool
+ pair<bool, target_state>
match (action, const target&, unmatch);
// Start asynchronous match. Return target_state::postponed if the
@@ -337,7 +337,7 @@ namespace build2
target_state
match_inner (action, const target&);
- bool
+ pair<bool, target_state>
match_inner (action, const target&, unmatch);
// The standard prerequisite search and match implementations. They call
diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx
index 5f9143a..d75ad16 100644
--- a/libbuild2/algorithm.ixx
+++ b/libbuild2/algorithm.ixx
@@ -312,7 +312,7 @@ namespace build2
return r;
}
- inline bool
+ inline pair<bool, target_state>
match (action a, const target& t, unmatch um)
{
assert (t.ctx.phase == run_phase::match);
@@ -328,7 +328,7 @@ namespace build2
case unmatch::unchanged:
{
if (s == target_state::unchanged)
- return true;
+ return make_pair (true, s);
break;
}
@@ -340,14 +340,14 @@ namespace build2
//
if (s == target_state::unchanged ||
t[a].dependents.load (memory_order_consume) != 0)
- return true;
+ return make_pair (true, s);
break;
}
}
match_inc_dependens (a, t);
- return false;
+ return make_pair (false, s);;
}
inline target_state
@@ -437,7 +437,7 @@ namespace build2
return match (a.inner_action (), t);
}
- inline bool
+ inline pair<bool, target_state>
match_inner (action a, const target& t, unmatch um)
{
assert (a.outer ());
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index 55bfc2c..5e757f1 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -686,6 +686,9 @@ namespace build2
//
wait_guard wg (ctx, ctx.count_busy (), t[a].task_count, true);
+ target_state src_ts1, src_ts2;
+
+ size_t src_i (~0); // Index of src target.
size_t start (pts.size ()); // Index of the first to be added.
for (prerequisite_member p: group_prerequisite_members (a, t))
{
@@ -737,7 +740,9 @@ namespace build2
else if (pi == include_type::normal &&
(p.is_a<bmi> () || p.is_a (tts.bmi) ||
p.is_a<hbmi> () || p.is_a (tts.hbmi)))
+ {
continue;
+ }
else
{
pt = &p.search (t);
@@ -746,12 +751,24 @@ namespace build2
continue;
}
- match_async (a, *pt, ctx.count_busy (), t[a].task_count);
+ target_state ts (
+ match_async (a, *pt, ctx.count_busy (), t[a].task_count));
+
+ if (p == md.src)
+ {
+ src_i = pts.size ();
+ src_ts1 = ts;
+ }
+
pts.push_back (prerequisite_target (pt, pi));
}
+ size_t src_tc1 (t[a].task_count.load (memory_order_consume));
+
wg.wait ();
+ size_t src_tc2 (t[a].task_count.load (memory_order_consume));
+
// Finish matching all the targets that we have started.
//
for (size_t i (start), n (pts.size ()); i != n; ++i)
@@ -765,13 +782,18 @@ namespace build2
// match in link::apply() it will be safe unless someone is building
// an obj?{} target directory.
//
- if (build2::match (
- a,
- *pt,
- pt->is_a<liba> () || pt->is_a<libs> () || pt->is_a<libux> ()
- ? unmatch::safe
- : unmatch::none))
+ pair<bool, target_state> mr (
+ build2::match (
+ a,
+ *pt,
+ pt->is_a<liba> () || pt->is_a<libs> () || pt->is_a<libux> ()
+ ? unmatch::safe
+ : unmatch::none));
+
+ if (mr.first)
pt = nullptr; // Ignore in execute.
+ else if (i == src_i)
+ src_ts2 = mr.second;
}
// Inject additional prerequisites. We only do it when performing update
@@ -780,11 +802,7 @@ namespace build2
//
if (a == perform_update_id)
{
- // The cached prerequisite target should be the same as what is in
- // t.prerequisite_targets since we used standard search() and match()
- // above.
- //
- const file& src (*md.src.load (memory_order_relaxed)->is_a<file> ());
+ const file& src (pts[src_i]->as<file> ());
// Figure out if __symexport is used. While normally it is specified
// on the project root (which we cached), it can be overridden with
@@ -902,12 +920,21 @@ namespace build2
// We seem to have a race condition here but can't quite put our
// finger on it.
//
+ // NOTE: remember to get rid of src_ts*, etc., once done.
+ //
// assert (!p.empty ()); // Sanity check.
if (p.empty ())
{
+ target_state src_ts3 (src.matched_state (a, false));
+
info << "unassigned path for target " << src <<
- info << "target state: " << src.matched_state (a, false) <<
- info << "is empty_path: " << (&p == &empty_path);
+ info << "is empty_path: " << (&p == &empty_path) <<
+ info << "target state 1: " << src_ts1 <<
+ info << "target state 2: " << src_ts2 <<
+ info << "target state 3: " << src_ts3 <<
+ info << "target count 1: " << src_tc1 <<
+ info << "target count 2: " << src_tc2;
+
assert (false);
}
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index b8d6a34..901b048 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -313,7 +313,7 @@ namespace build2
//
optional<bool> unchanged;
if (a.operation () == update_id)
- unchanged = match_inner (a, t, unmatch::unchanged);
+ unchanged = match_inner (a, t, unmatch::unchanged).first;
auto& pts (t.prerequisite_targets[a]);
@@ -367,7 +367,7 @@ namespace build2
// when updating static installable content (headers, documentation,
// etc).
//
- if (build2::match (a, *pt, unmatch::unchanged))
+ if (build2::match (a, *pt, unmatch::unchanged).first)
pt = nullptr;
}
else if (!try_match (a, *pt).first)
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index c2954f8..d4b287b 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -1013,6 +1013,18 @@ namespace build2
static_assert (std::is_trivially_destructible<prerequisite_member>::value,
"prerequisite_member is not trivially destructible");
+ inline bool
+ operator== (const prerequisite_member& x, const prerequisite_member& y)
+ {
+ return &x.prerequisite == &y.prerequisite && x.member == y.member;
+ }
+
+ inline bool
+ operator!= (const prerequisite_member& x, const prerequisite_member& y)
+ {
+ return !(x == y);
+ }
+
inline ostream&
operator<< (ostream& os, const prerequisite_member& pm)
{