aboutsummaryrefslogtreecommitdiff
path: root/libbuild2
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2023-11-07 07:21:06 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2023-11-07 07:21:06 +0200
commit98c7247ff06decba8130e8aea67ea839d084d2dd (patch)
treefea6d2a7f5172c78992ccada6229db86cc1e10b2 /libbuild2
parenta8ff78d8b90d6f7a45c9586449986dadb65809c8 (diff)
Account for match options re-locking when checking if target is matched
Diffstat (limited to 'libbuild2')
-rw-r--r--libbuild2/algorithm.cxx15
-rw-r--r--libbuild2/algorithm.hxx2
-rw-r--r--libbuild2/algorithm.ixx3
-rw-r--r--libbuild2/target.cxx12
-rw-r--r--libbuild2/target.hxx2
-rw-r--r--libbuild2/target.ixx30
6 files changed, 49 insertions, 15 deletions
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 0e995d5..1bb46d8 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -311,8 +311,15 @@ namespace build2
{
// First lock for this operation.
//
+ // Note that we use 0 match_extra::cur_options_ as an indication of not
+ // being applied yet. In particular, in the match phase, this is used to
+ // distinguish between the "busy because not applied yet" and "busy
+ // because relocked to reapply match options" cases. See
+ // target::matched() for details.
+ //
s.rule = nullptr;
s.dependents.store (0, memory_order_release);
+ s.match_extra.cur_options_.store (0, memory_order_relaxed);
offset = target::offset_touched;
}
@@ -981,6 +988,7 @@ namespace build2
recipe re (ar != nullptr ? f (*ar, a, t, me) : ru.apply (a, t, me));
me.free (); // Note: cur_options are still in use.
+ assert (me.cur_options != 0); // Match options cannot be 0 after apply().
me.cur_options_.store (me.cur_options, memory_order_relaxed);
return re;
}
@@ -1044,6 +1052,7 @@ namespace build2
// Note: for now no adhoc_reapply().
//
ru.reapply (a, t, me);
+ assert (me.cur_options != 0); // Match options cannot be 0 after reapply().
me.cur_options_.store (me.cur_options, memory_order_relaxed);
}
@@ -1352,6 +1361,12 @@ namespace build2
{
s.state = target_state::failed;
l.offset = target::offset_applied;
+
+ // Make sure we don't relock a failed target.
+ //
+ match_extra& me (s.match_extra);
+ me.cur_options = match_extra::all_options;
+ me.cur_options_.store (me.cur_options, memory_order_relaxed);
}
if (s.state == target_state::failed)
diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx
index 11e655b..a0f8cd6 100644
--- a/libbuild2/algorithm.hxx
+++ b/libbuild2/algorithm.hxx
@@ -440,7 +440,7 @@ namespace build2
//
// Note that there will be no way to rematch on options change (since there
// is no rule), so passing anything other than all_options is most likely a
- // bad idea.
+ // bad idea. Passing 0 for options is illegal.
//
void
match_recipe (target_lock&,
diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx
index e384433..c7b4b7e 100644
--- a/libbuild2/algorithm.ixx
+++ b/libbuild2/algorithm.ixx
@@ -614,7 +614,8 @@ namespace build2
inline void
match_recipe (target_lock& l, recipe r, uint64_t options)
{
- assert (l.target != nullptr &&
+ assert (options != 0 &&
+ l.target != nullptr &&
l.offset < target::offset_matched &&
l.target->ctx.phase == run_phase::match);
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index fb47b6d..b4c3968 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -1004,15 +1004,19 @@ namespace build2
case run_phase::load: break;
case run_phase::match:
{
- // Similar logic to matched_state_impl().
+ // Similar logic to target::matched().
//
const opstate& s (state[action () /* inner */]);
- // Note: already synchronized.
- size_t c (s.task_count.load (memory_order_relaxed));
+ // Note: use acquire for group_state().
+ //
+ size_t c (s.task_count.load (memory_order_acquire));
size_t b (ctx.count_base ()); // Note: cannot do (c - b)!
- if (c != (b + offset_applied) && c != (b + offset_executed))
+ if (!(c == (b + offset_applied) ||
+ c == (b + offset_executed) ||
+ (c >= (b + offset_busy) &&
+ s.match_extra.cur_options_.load (memory_order_relaxed) != 0)))
break;
}
// Fall through.
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index c61f241..76b7158 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -249,6 +249,8 @@ namespace build2
// - match to resolve members (resolve_members())
// - match of ad hoc group via one of its ad hoc members
//
+ // Note that the 0 cur_options value is illegal.
+ //
// When it comes to match options specified for group members, the
// semantics differs between explicit and ad hoc groups. For explicit
// groups, the standard semantics described above applies and the group's
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index ad09cd4..8136fe9 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -243,17 +243,25 @@ namespace build2
assert (ctx.phase == run_phase::match ||
ctx.phase == run_phase::execute);
- size_t c (state[a].task_count.load (mo));
+ const opstate& s (state[a]);
+ size_t c (s.task_count.load (mo));
size_t b (ctx.count_base ()); // Note: cannot do (c - b)!
if (ctx.phase == run_phase::match)
{
- // While it will normally be applied, it could also be already executed.
+ // While it will normally be applied, it could also be already executed
+ // or being relocked to reapply match options (see lock_impl() for
+ // background).
+ //
+ // Note that we can't just do >= offset_applied since offset_busy can
+ // also mean it is being matched.
//
- // Note that we can't do >= offset_applied since offset_busy means it is
- // being matched.
+ // See also matched_state_impl(), mtime() for similar logic.
//
- return c == (b + offset_applied) || c == (b + offset_executed);
+ return (c == (b + offset_applied) ||
+ c == (b + offset_executed) ||
+ (c >= (b + offset_busy) &&
+ s.match_extra.cur_options_.load (memory_order_relaxed) != 0));
}
else
{
@@ -308,11 +316,15 @@ namespace build2
return make_pair (false, target_state::unknown);
else
{
- // Normally applied but can also be already executed. Note that in the
- // latter case we are guaranteed to be synchronized since we are in the
- // match phase.
+ // The same semantics as in target::matched(). Note that in the executed
+ // case we are guaranteed to be synchronized since we are in the match
+ // phase.
//
- assert (c == (b + offset_applied) || c == (b + offset_executed));
+ assert (c == (b + offset_applied) ||
+ c == (b + offset_executed) ||
+ (c >= (b + offset_busy) &&
+ s.match_extra.cur_options_.load (memory_order_relaxed) != 0));
+
return make_pair (true, (group_state (a) ? group->state[a] : s).state);
}
}