Fix data race in ad hoc member state when group is postponed

3 files changed, 28 insertions, 9 deletions

diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 16f1503..d2d1eb6 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -2077,7 +2077,11 @@ namespace build2
       case target_state::unchanged:
         break;
       case target_state::postponed:
-        ts = t[a].state = target_state::unchanged;
+        // Keep the target state postponed (see group_action() for details)
+        // but translate the result from postponed to unchanged, similar to
+        // executed_state_impl().
+        //
+        ts = target_state::unchanged;
         break;
       case target_state::group:
         ts = (*t.group)[a].state;
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index b008347..72d6dbc 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -953,10 +953,14 @@ namespace build2
       mutable bool           recipe_keep;         // Keep after execution.
       bool                   recipe_group_action; // Recipe is group_action.
 
-      // Target state for this operation. Note that it is undetermined until
-      // a rule is matched and recipe applied (see set_recipe()).
+      // Target state for this operation.
       //
-      target_state state;
+      // Note that it is undetermined until a rule is matched and recipe
+      // applied (see set_recipe()). However, we need it to be not postponed
+      // for ad hoc members that are not matched (see group_state()) so
+      // initialize it to unknown.
+      //
+      target_state state = target_state::unknown;
 
       // Set to true (only for the inner action) if this target has been
       // matched but not executed as a result of the resolve_members() call.
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index 39b81e7..5e24b20 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -286,6 +286,7 @@ namespace build2
     // @@ Hm, I wonder why not just return s.recipe_group_action now that we
     //    cache it.
     //
+    const opstate& s (state[a]);
 
     // This special hack allows us to do things like query an ad hoc member's
     // state or mtime without matching/executing the member, only the group.
@@ -293,14 +294,17 @@ namespace build2
     // this feels harmless (ad hoc membership cannot be changed during the
     // execute phase).
     //
+    // Note: if the member state is postponed, then the group state may not be
+    // yet known (see group_action() for details).
+    //
     // Note: this test must come first since the member may not be matched and
-    // thus its state uninitialized.
+    // thus its state set (but it won't be postponed; see opstate::state).
     //
-    if (ctx.phase == run_phase::execute && adhoc_group_member ())
+    if (ctx.phase == run_phase::execute &&
+        adhoc_group_member ()           &&
+        s.state != target_state::postponed)
       return true;
 
-    const opstate& s (state[a]);
-
     if (s.state == target_state::group)
       return true;
 
@@ -342,7 +346,14 @@ namespace build2
   inline target_state target::
   executed_state_impl (action a) const
   {
-    return (group_state (a) ? group->state : state)[a].state;
+    target_state ts ((group_state (a) ? group->state : state)[a].state);
+
+    // Translate postponed to unchanged, similar to execute_recipe().
+    //
+    if (ts == target_state::postponed)
+      ts = target_state::unchanged;
+
+    return ts;
   }
 
   inline target_state target::