Distinguish between internal/external wait deactivation in scheduler

This turns out to be necessary for the deadlock detection to work properly.

3 files changed, 38 insertions, 22 deletions

diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index d56abb3..d1bfc33 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -62,11 +62,11 @@ namespace build2
       }
       else if (phase != p)
       {
-        sched.deactivate ();
+        sched.deactivate (false /* external */);
         for (; phase != p; v->wait (l)) ;
         r = !fail_;
         l.unlock (); // Important: activate() can block.
-        sched.activate ();
+        sched.activate (false /* external */);
       }
       else
         r = !fail_;
@@ -175,11 +175,11 @@ namespace build2
       }
       else // phase != n
       {
-        sched.deactivate ();
+        sched.deactivate (false /* external */);
         for (; phase != n; v->wait (l)) ;
         r = !fail_;
         l.unlock (); // Important: activate() can block.
-        sched.activate ();
+        sched.activate (false /* external */);
       }
     }
 
diff --git a/libbuild2/scheduler.cxx b/libbuild2/scheduler.cxx
index 437f904..bb56810 100644
--- a/libbuild2/scheduler.cxx
+++ b/libbuild2/scheduler.cxx
@@ -97,7 +97,7 @@ namespace build2
   }
 
   void scheduler::
-  deactivate ()
+  deactivate (bool external)
   {
     if (max_active_ == 1) // Serial execution.
       return;
@@ -106,6 +106,8 @@ namespace build2
 
     active_--;
     waiting_++;
+    if (external)
+      external_++;
     progress_.fetch_add (1, memory_order_relaxed);
 
     if (waiting_ > stat_max_waiters_)
@@ -122,7 +124,7 @@ namespace build2
     {
       activate_helper (l);
     }
-    else if (active_ == 0)
+    else if (active_ == 0 && external_ == 0)
     {
       // Note that we tried to handle this directly in this thread but that
       // wouldn't work for the phase lock case where we cal; deactivate and
@@ -135,7 +137,7 @@ namespace build2
   }
 
   void scheduler::
-  activate (bool collision)
+  activate (bool external, bool collision)
   {
     if (max_active_ == 1) // Serial execution.
       return;
@@ -148,6 +150,8 @@ namespace build2
     // If we have spare active threads, then become active. Otherwise it
     // enters the ready queue.
     //
+    if (external)
+      external_--;
     waiting_--;
     ready_++;
     progress_.fetch_add (1, memory_order_relaxed);
@@ -166,7 +170,7 @@ namespace build2
   void scheduler::
   sleep (const duration& d)
   {
-    deactivate ();
+    deactivate (true /* external */);
 
     // MINGW GCC 4.9 doesn't implement this_thread so use Win32 Sleep().
     //
@@ -178,7 +182,7 @@ namespace build2
     Sleep (static_cast<DWORD> (duration_cast<milliseconds> (d).count ()));
 #endif
 
-    activate ();
+    activate (true /* external */);
   }
 
   size_t scheduler::
@@ -190,7 +194,7 @@ namespace build2
 
     // This thread is no longer active.
     //
-    deactivate ();
+    deactivate (false /* external */);
 
     // Note that the task count is checked while holding the lock. We also
     // have to notify while holding the lock (see resume()). The aim here
@@ -227,7 +231,7 @@ namespace build2
 
     // This thread is no longer waiting.
     //
-    activate (collision);
+    activate (false /* external */, collision);
 
     return tc;
   }
@@ -465,6 +469,8 @@ namespace build2
         l.lock ();
       }
 
+      assert (external_ == 0);
+
       // Wait for the deadlock monitor (the only remaining thread).
       //
       if (orig_max_active_ != 1) // See tune() for why not max_active_.
@@ -759,7 +765,7 @@ namespace build2
         //
         if (s.ready_ != 0)
           s.ready_condv_.notify_one ();
-        else if (s.active_ == 0)
+        else if (s.active_ == 0 && s.external_ == 0)
           s.dead_condv_.notify_one ();
       }
 
@@ -802,7 +808,7 @@ namespace build2
     {
       s.dead_condv_.wait (l);
 
-      while (s.active_ == 0 && !s.shutdown_)
+      while (s.active_ == 0 && s.external_ == 0 && !s.shutdown_)
       {
         // We may have a deadlock which can happen because of dependency
         // cycles.
@@ -829,9 +835,10 @@ namespace build2
         }
         l.lock ();
 
-        // Re-check active count for good measure (maybe spinning too fast).
+        // Re-check active/external counts for good measure (maybe we are
+        // spinning too fast).
         //
-        if (np == op && s.active_ == 0 && !s.shutdown_)
+        if (np == op && s.active_ == 0 && s.external_ == 0 && !s.shutdown_)
         {
           // Shutting things down cleanly is tricky: we could have handled it
           // in the scheduler (e.g., by setting a flag and then waking
diff --git a/libbuild2/scheduler.hxx b/libbuild2/scheduler.hxx
index af1deba..7ceb239 100644
--- a/libbuild2/scheduler.hxx
+++ b/libbuild2/scheduler.hxx
@@ -132,15 +132,19 @@ namespace build2
     resume (const atomic_count& task_count);
 
     // An active thread that is about to wait for potentially significant time
-    // on something other than task_count (e.g., mutex, condition variable)
-    // should deactivate itself with the scheduler and then reactivate once
-    // done waiting.
+    // on something other than task_count (e.g., mutex, condition variable,
+    // timer, etc) should deactivate itself with the scheduler and then
+    // reactivate once done waiting.
+    //
+    // The external flag indicates whether the wait is for an event external
+    // to the scheduler, that is, triggered by something other than one of the
+    // threads managed by the scheduler.
     //
     void
-    deactivate ();
+    deactivate (bool external);
 
     void
-    activate (bool collision = false);
+    activate (bool external, bool = false);
 
     // Sleep for the specified duration, deactivating the thread before going
     // to sleep and re-activating it after waking up (which means this
@@ -413,8 +417,9 @@ namespace build2
 
     size_t helpers_     = 0; // Number of helper threads created so far.
 
-    // Every thread that we manage must be accounted for in one of these
-    // counters. And their sum should equal (init_active + helpers).
+    // Every thread that we manage (except for the special deadlock monitor)
+    // must be accounted for in one of these counters. And their sum should
+    // equal (init_active + helpers).
     //
     size_t active_   = 0;  // Active master threads executing a task.
     size_t idle_     = 0;  // Idle helper threads waiting for a task.
@@ -422,6 +427,10 @@ namespace build2
     size_t ready_    = 0;  // Ready master thread waiting to become active.
     size_t starting_ = 0;  // Helper threads starting up.
 
+    // Number of waiting threads that are waiting for an external event.
+    //
+    size_t external_ = 0;
+
     // Original values (as specified during startup) that can be altered via
     // tuning.
     //