Add notion of load phase generation

8 files changed, 86 insertions, 55 deletions

diff --git a/build2/context b/build2/context
index cf5483e..2f0a76f 100644
--- a/build2/context
+++ b/build2/context
@@ -29,7 +29,7 @@ namespace build2
   // The build system model (internal state) is protected at the top level by
   // the model mutex. During serial execution the model mutex is unlocked.
   //
-  extern shared_mutex model;
+  extern shared_mutex model_mutex;
 
   // Parallel execution always starts with acquiring a shared model lock (by
   // creating model_slock; see below). Pointers to these locks are cached in
@@ -38,14 +38,18 @@ namespace build2
   // The build system starts with a "serial load" phase and then continues
   // with parallel search & match and execute. Search & match, however, can be
   // interrupted with an "exclusive load" by re-locking the shared lock as
-  // exclusive, changing the phase, and loading additional buildfiles.
+  // exclusive (using model_rlock below), changing the phase, and loading
+  // additional buildfiles.
   //
   // Serial load can perform arbitrary changes to the model. Exclusive load,
-  // however, can only perform "pure appends". That is, it can create new
-  // "nodes" (variables, scopes, etc) but not change already existing nodes
-  // or invalidate any references to such (the idea here is that one should
-  // be able to load additional buildfiles as long as they don't interfere
-  // with the existing build state).
+  // however, can only perform "island appends". That is, it can create new
+  // "nodes" (variables, scopes, etc) but not change already existing nodes or
+  // invalidate any references to such (the idea here is that one should be
+  // able to load additional buildfiles as long as they don't interfere with
+  // the existing build state). The "islands" are identified by the
+  // load_generation number (0 for serial load). It is incremented/restored by
+  // phase_guard and is stored in various "nodes" (variables, etc) to allow
+  // modifications "within the islands".
   //
   // @@ MT: do we really have to hold shared lock during execute?
   // @@ MT: we can also interrupt load s&m with execute -- neither handled
@@ -59,10 +63,27 @@ namespace build2
 #endif
   slock* model_lock;
 
+  extern size_t load_generation;
+
   struct phase_guard
   {
-    explicit phase_guard (run_phase p): o (phase) {phase = p;}
-    ~phase_guard () {phase = o;}
+    explicit
+    phase_guard (run_phase p)
+        : o (phase)
+    {
+      phase = p;
+
+      if (phase == run_phase::load)
+        ++load_generation;
+    }
+
+    ~phase_guard ()
+    {
+      if (phase == run_phase::load)
+        --load_generation;
+
+      phase = o;
+    }
     run_phase o;
   };
 
@@ -124,7 +145,7 @@ namespace build2
       if (slock* l = model_lock)
         assert (l->owns_lock ());
       else
-        model_lock = &(l_ = slock (model));
+        model_lock = &(l_ = slock (model_mutex));
     }
 
     ~model_slock ()
@@ -140,6 +161,38 @@ namespace build2
     slock l_;
   };
 
+  // Re-lock shared to exclusive for the lifetime or rlock.
+  //
+  struct model_rlock
+  {
+    model_rlock ()
+        : sl_ (model_lock)
+    {
+      if (sl_ != nullptr)
+      {
+        sl_->unlock ();
+        ul_ = ulock (*sl_->mutex ());
+      }
+    }
+
+    ~model_rlock ()
+    {
+      if (sl_ != nullptr)
+      {
+        ul_.unlock ();
+        sl_->lock ();
+      }
+    }
+
+    // Can be treated as const ulock.
+    //
+    operator const ulock& () const {return ul_;}
+
+  private:
+    slock* sl_;
+    ulock ul_;
+  };
+
   // Cached variables.
   //
   extern const variable* var_src_root;
diff --git a/build2/context.cxx b/build2/context.cxx
index a8fe2df..a326002 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -25,7 +25,7 @@ namespace build2
 {
   run_phase phase = run_phase::load;
 
-  shared_mutex model;
+  shared_mutex model_mutex;
 
 #ifdef __cpp_thread_local
   thread_local
@@ -34,6 +34,8 @@ namespace build2
 #endif
   slock* model_lock;
 
+  size_t load_generation;
+
   const variable* var_src_root;
   const variable* var_out_root;
   const variable* var_src_base;
@@ -188,7 +190,8 @@ namespace build2
       // Add it if not found.
       //
       if (o->override == nullptr)
-        o->override.reset (new variable {n + k, nullptr, nullptr, v});
+        const_cast<variable*> (o)->override.reset (
+          new variable {n + k, nullptr, nullptr, v, 0});
 
       o = o->override.get ();
 
diff --git a/build2/parser.cxx b/build2/parser.cxx
index fe12b98..af6a6f5 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -1593,7 +1593,7 @@ namespace build2
     if (type != nullptr)
     {
       if (var.type == nullptr)
-        var.type = type;
+        var_pool.update (const_cast<variable&> (var), type);
       else if (var.type != type)
         fail (l) << "changing variable " << var << " type from "
                  << var.type->name << " to " << type->name;
diff --git a/build2/target.cxx b/build2/target.cxx
index 24bd246..f04e26f 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -630,7 +630,7 @@ namespace build2
       {
         // Relock for exclusive access and change to the load phase.
         //
-        rlock rl (model_lock);
+        model_rlock rl;
         phase_guard pg (run_phase::load);
 
         pair<scope&, scope*> sp (switch_scope (*s.root_scope (), out_base));
diff --git a/build2/types b/build2/types
index 1e7a347..daf10d3 100644
--- a/build2/types
+++ b/build2/types
@@ -86,39 +86,6 @@ namespace build2
   using slock = std::shared_lock<shared_mutex>;
   using ulock = std::unique_lock<shared_mutex>;
 
-  // Re-lock shared to exclusive for the lifetime or rlock.
-  //
-  struct rlock
-  {
-    explicit
-    rlock (slock* sl)
-        : sl_ (sl)
-    {
-      if (sl_ != nullptr)
-      {
-        sl_->unlock ();
-        ul_ = ulock (*sl_->mutex ());
-      }
-    }
-
-    ~rlock ()
-    {
-      if (sl_ != nullptr)
-      {
-        ul_.unlock ();
-        sl_->lock ();
-      }
-    }
-
-    // Can be treated as const ulock.
-    //
-    operator const ulock& () const {return ul_;}
-
-  private:
-    slock* sl_;
-    ulock ul_;
-  };
-
   // Exceptions.
   //
   // While <exception> is included, there is no using for std::exception --
diff --git a/build2/variable b/build2/variable
index 876ae2b..e60c7a1 100644
--- a/build2/variable
+++ b/build2/variable
@@ -116,9 +116,10 @@ namespace build2
   struct variable
   {
     string name;
-    mutable const value_type* type;        // If NULL, then not (yet) typed.
-    mutable unique_ptr<variable> override;
+    const value_type* type;        // If NULL, then not (yet) typed.
+    unique_ptr<variable> override;
     variable_visibility visibility;
+    size_t generation;             // load_generation of this variable.
   };
 
   inline bool
@@ -912,11 +913,12 @@ namespace build2
     void
     update (variable&,
             const build2::value_type*,
-            const variable_visibility*,
-            const bool*) const;
+            const variable_visibility* = nullptr,
+            const bool* = nullptr) const;
 
     // Entities that can access bypassing the lock.
     //
+    friend class parser;
     friend class scope;
     friend variable_overrides reset (const strings&);
 
diff --git a/build2/variable.cxx b/build2/variable.cxx
index 8695623..16fbe74 100644
--- a/build2/variable.cxx
+++ b/build2/variable.cxx
@@ -922,9 +922,12 @@ namespace build2
     bool uv (v != nullptr && var.visibility != *v);
 
     // In the global pool existing variables can only be updated during
-    // serial load.
+    // the same load generation or during serial execution.
     //
-    assert (!global_ || !(ut || uv) || model_lock == nullptr);
+    assert (!global_             ||
+            !(ut || uv)          ||
+            load_generation == 0 ||
+            var.generation == load_generation);
 
     // Update type?
     //
@@ -1012,7 +1015,8 @@ namespace build2
           move (n),
           t,
           nullptr,
-          v != nullptr ? *v : variable_visibility::normal}));
+          v != nullptr ? *v : variable_visibility::normal,
+          load_generation}));
 
     variable& r (p.first->second);
 
@@ -1034,6 +1038,8 @@ namespace build2
                   bool o,
                   variable_visibility v)
   {
+    assert (!global_ || phase == run_phase::load);
+
     size_t pn (p.size ());
 
     size_t w (p.find ('*'));
diff --git a/unit-tests/test/script/parser/driver.cxx b/unit-tests/test/script/parser/driver.cxx
index f1e7483..c054669 100644
--- a/unit-tests/test/script/parser/driver.cxx
+++ b/unit-tests/test/script/parser/driver.cxx
@@ -182,7 +182,7 @@ namespace build2
           // be absolute. However, the testscript implementation doesn't
           // really care.
           //
-          ulock ml (model);
+          ulock ml (model_mutex);
 
           file& tt (
             targets.insert<file> (work,