Terminate waiting threads if coming off failed load phase

In this case the build state may no longer be valid.

4 files changed, 169 insertions, 88 deletions

diff --git a/build2/c/init.cxx b/build2/c/init.cxx
index 4158a09..59c6a40 100644
--- a/build2/c/init.cxx
+++ b/build2/c/init.cxx
@@ -241,7 +241,6 @@ namespace build2
       return true;
     }
 
-
     static const target_type* const hdr[] =
     {
       &h::static_type,
diff --git a/build2/context.cxx b/build2/context.cxx
index 68c7e6d..1b5bb0b 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -5,12 +5,15 @@
 #include <build2/context.hxx>
 
 #include <sstream>
+#include <exception> // uncaught_exception[s]()
 
 #include <build2/rule.hxx>
 #include <build2/scope.hxx>
 #include <build2/target.hxx>
 #include <build2/diagnostics.hxx>
 
+#include <libbutl/ft/exception.hxx> // uncaught_exceptions
+
 // For command line variable parsing.
 //
 #include <build2/token.hxx>
@@ -38,9 +41,11 @@ namespace build2
 #endif
   phase_lock* phase_lock::instance;
 
-  void phase_mutex::
+  bool phase_mutex::
   lock (run_phase p)
   {
+    bool r;
+
     {
       mlock l (m_);
       bool u (lc_ == 0 && mc_ == 0 && ec_ == 0); // Unlocked.
@@ -60,20 +65,31 @@ namespace build2
       // since there is nobody waiting (all counters are zero).
       //
       if (u)
+      {
         phase = p;
+        r = !fail_;
+      }
       else if (phase != p)
       {
         sched.deactivate ();
         for (; phase != p; v->wait (l)) ;
+        r = !fail_;
         l.unlock (); // Important: activate() can block.
         sched.activate ();
       }
+      else
+        r = !fail_;
     }
 
     // In case of load, acquire the exclusive access mutex.
     //
     if (p == run_phase::load)
+    {
       lm_.lock ();
+      r = !fail_; // Re-query.
+    }
+
+    return r;
   }
 
   void phase_mutex::
@@ -119,7 +135,7 @@ namespace build2
     }
   }
 
-  void phase_mutex::
+  bool phase_mutex::
   relock (run_phase o, run_phase n)
   {
     // Pretty much a fused unlock/lock implementation except that we always
@@ -127,6 +143,8 @@ namespace build2
     //
     assert (o != n);
 
+    bool r;
+
     if (o == run_phase::load)
       lm_.unlock ();
 
@@ -154,6 +172,7 @@ namespace build2
       if (u)
       {
         phase = n;
+        r = !fail_;
 
         // Notify others that could be waiting for this phase.
         //
@@ -167,13 +186,140 @@ namespace build2
       {
         sched.deactivate ();
         for (; phase != n; v->wait (l)) ;
+        r = !fail_;
         l.unlock (); // Important: activate() can block.
         sched.activate ();
       }
     }
 
     if (n == run_phase::load)
+    {
       lm_.lock ();
+      r = !fail_; // Re-query.
+    }
+
+    return r;
+  }
+
+  // C++17 deprecated uncaught_exception() so use uncaught_exceptions() if
+  // available.
+  //
+  static inline bool
+  uncaught_exception ()
+  {
+#ifdef __cpp_lib_uncaught_exceptions
+    return std::uncaught_exceptions () != 0;
+#else
+    return std::uncaught_exception ();
+#endif
+  }
+
+  // phase_lock
+  //
+  phase_lock::
+  phase_lock (run_phase p)
+      : p (p)
+  {
+    if (phase_lock* l = instance)
+      assert (l->p == p);
+    else
+    {
+      if (!phase_mutex::instance.lock (p))
+      {
+        phase_mutex::instance.unlock (p);
+        throw failed ();
+      }
+
+      instance = this;
+
+      //text << this_thread::get_id () << " phase acquire " << p;
+    }
+  }
+
+  phase_lock::
+  ~phase_lock ()
+  {
+    if (instance == this)
+    {
+      instance = nullptr;
+      phase_mutex::instance.unlock (p);
+
+      //text << this_thread::get_id () << " phase release " << p;
+    }
+  }
+
+  // phase_unlock
+  //
+  phase_unlock::
+  phase_unlock (bool u)
+      : l (u ? phase_lock::instance : nullptr)
+  {
+    if (u)
+    {
+      phase_lock::instance = nullptr;
+      phase_mutex::instance.unlock (l->p);
+
+      //text << this_thread::get_id () << " phase unlock  " << l->p;
+    }
+  }
+
+  phase_unlock::
+  ~phase_unlock () noexcept (false)
+  {
+    if (l != nullptr)
+    {
+      bool r (phase_mutex::instance.lock (l->p));
+      phase_lock::instance = l;
+
+      // Fail unless we are already failing. Note that we keep the phase
+      // locked since there will be phase_lock down the stack to unlock it.
+      //
+      if (!r && !uncaught_exception ())
+        throw failed ();
+
+      //text << this_thread::get_id () << " phase lock    " << l->p;
+    }
+  }
+
+  // phase_switch
+  //
+  phase_switch::
+  phase_switch (run_phase n)
+      : o (phase), n (n)
+  {
+    if (!phase_mutex::instance.relock (o, n))
+    {
+      phase_mutex::instance.relock (n, o);
+      throw failed ();
+    }
+
+    phase_lock::instance->p = n;
+
+    if (n == run_phase::load) // Note: load lock is exclusive.
+      load_generation++;
+
+    //text << this_thread::get_id () << " phase switch  " << o << " " << n;
+  }
+
+  phase_switch::
+  ~phase_switch () noexcept (false)
+  {
+    // If we are coming off a failed load phase, mark the phase_mutex as
+    // failed to terminate all other threads since the build state may no
+    // longer be valid.
+    //
+    if (n == run_phase::load && uncaught_exception ())
+      phase_mutex::instance.fail_ = true;
+
+    bool r (phase_mutex::instance.relock (n, o));
+    phase_lock::instance->p = o;
+
+    // Similar logic to ~phase_unlock().
+    //
+    if (!r && !uncaught_exception ())
+      throw failed ();
+
+    //text << this_thread::get_id () << " phase restore " << n << " " << o;
   }
 
   const variable* var_src_root;
diff --git a/build2/context.hxx b/build2/context.hxx
index c3e0595..5815f51 100644
--- a/build2/context.hxx
+++ b/build2/context.hxx
@@ -68,13 +68,22 @@ namespace build2
   // process may pick up headers as they are being generated. As a result, we
   // either have everyone treat the external state as read-only or write-only.
   //
+  // There is also one more complication: if we are returning from a load
+  // phase that has failed, then the build state could be seriously messed up
+  // (things like scopes not being setup completely, etc). And once we release
+  // the lock, other threads that are waiting will start relying on this
+  // messed up state. So a load phase can mark the phase_mutex as failed in
+  // which case all currently blocked and future lock()/relock() calls return
+  // false. Note that in this case we still switch to the desired phase. See
+  // the phase_{lock,switch,unlock} implementations for details.
+  //
   class phase_mutex
   {
   public:
     // Acquire a phase lock potentially blocking (unless already in the
     // desired phase) until switching to the desired phase is possible.
     //
-    void
+    bool
     lock (run_phase);
 
     // Release the phase lock potentially allowing (unless there are other
@@ -86,7 +95,7 @@ namespace build2
     // Switch from one phase to another. Semantically, just unlock() followed
     // by lock() but more efficient.
     //
-    void
+    bool
     relock (run_phase unlock, run_phase lock);
 
   private:
@@ -94,7 +103,11 @@ namespace build2
     friend struct phase_unlock;
     friend struct phase_switch;
 
-    phase_mutex (): lc_ (0), mc_ (0), ec_ (0) {phase = run_phase::load;}
+    phase_mutex ()
+        : fail_ (false), lc_ (0), mc_ (0), ec_ (0)
+    {
+      phase = run_phase::load;
+    }
 
     static phase_mutex instance;
 
@@ -110,6 +123,9 @@ namespace build2
     // is always changed to load (this is also the initial state).
     //
     mutex m_;
+
+    bool fail_;
+
     size_t lc_;
     size_t mc_;
     size_t ec_;
@@ -201,7 +217,7 @@ namespace build2
   struct phase_unlock
   {
     phase_unlock (bool unlock = true);
-    ~phase_unlock ();
+    ~phase_unlock () noexcept (false);
 
     phase_lock* l;
   };
@@ -212,7 +228,7 @@ namespace build2
   struct phase_switch
   {
     explicit phase_switch (run_phase);
-    ~phase_switch ();
+    ~phase_switch () noexcept (false);
 
     run_phase o, n;
   };
diff --git a/build2/context.ixx b/build2/context.ixx
index aee7e32..2e878d0 100644
--- a/build2/context.ixx
+++ b/build2/context.ixx
@@ -4,86 +4,6 @@
 
 namespace build2
 {
-  // phase_lock
-  //
-  inline phase_lock::
-  phase_lock (run_phase p)
-      : p (p)
-  {
-    if (phase_lock* l = instance)
-      assert (l->p == p);
-    else
-    {
-      phase_mutex::instance.lock (p);
-      instance = this;
-
-      //text << this_thread::get_id () << " phase acquire " << p;
-    }
-  }
-
-  inline phase_lock::
-  ~phase_lock ()
-  {
-    if (instance == this)
-    {
-      instance = nullptr;
-      phase_mutex::instance.unlock (p);
-
-      //text << this_thread::get_id () << " phase release " << p;
-    }
-  }
-
-  // phase_unlock
-  //
-  inline phase_unlock::
-  phase_unlock (bool u)
-      : l (u ? phase_lock::instance : nullptr)
-  {
-    if (u)
-    {
-      phase_lock::instance = nullptr;
-      phase_mutex::instance.unlock (l->p);
-
-      //text << this_thread::get_id () << " phase unlock  " << l->p;
-    }
-  }
-
-  inline phase_unlock::
-  ~phase_unlock ()
-  {
-    if (l != nullptr)
-    {
-      phase_mutex::instance.lock (l->p);
-      phase_lock::instance = l;
-
-      //text << this_thread::get_id () << " phase lock    " << l->p;
-    }
-  }
-
-  // phase_switch
-  //
-  inline phase_switch::
-  phase_switch (run_phase n)
-      : o (phase), n (n)
-  {
-    phase_mutex::instance.relock (o, n);
-    phase_lock::instance->p = n;
-
-    if (n == run_phase::load) // Note: load lock is exclusive.
-      load_generation++;
-
-    //text << this_thread::get_id () << " phase switch  " << o << " " << n;
-  }
-
-  inline phase_switch::
-  ~phase_switch ()
-  {
-    phase_mutex::instance.relock (n, o);
-    phase_lock::instance->p = o;
-
-    //text << this_thread::get_id () << " phase restore " << n << " " << o;
-  }
-
   // wait_guard
   //
   inline wait_guard::