Fix pool use count going unbalanced
authorScott Graham <scottmg@chromium.org>
Thu, 23 Apr 2015 22:54:28 +0000 (15:54 -0700)
committerScott Graham <scottmg@chromium.org>
Thu, 23 Apr 2015 22:56:09 +0000 (15:56 -0700)
src/build.cc
src/build_test.cc
src/state.h

index ccdb37f..f14831e 100644 (file)
@@ -411,7 +411,9 @@ void Plan::NodeFinished(Node* node) {
         ScheduleWork(*oe);
       } else {
         // We do not need to build this edge, but we might need to build one of
-        // its dependents.
+        // its dependents. Make sure the pool schedules it before it's finished
+        // otherwise the pool use count may be invalid.
+        (*oe)->pool()->EdgeScheduled(**oe);
         EdgeFinished(*oe);
       }
     }
index a313693..52a17c9 100644 (file)
@@ -495,8 +495,8 @@ struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser {
   /// State of command_runner_ and logs contents (if specified) ARE MODIFIED.
   /// Handy to check for NOOP builds, and higher-level rebuild tests.
   void RebuildTarget(const string& target, const char* manifest,
-                     const char* log_path = NULL,
-                     const char* deps_path = NULL);
+                     const char* log_path = NULL, const char* deps_path = NULL,
+                     State* state = NULL);
 
   // Mark a path dirty.
   void Dirty(const string& path);
@@ -516,10 +516,13 @@ struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser {
 };
 
 void BuildTest::RebuildTarget(const string& target, const char* manifest,
-                              const char* log_path, const char* deps_path) {
-  State state;
-  ASSERT_NO_FATAL_FAILURE(AddCatRule(&state));
-  AssertParse(&state, manifest);
+                              const char* log_path, const char* deps_path,
+                              State* state) {
+  State local_state, *pstate = &local_state;
+  if (state)
+    pstate = state;
+  ASSERT_NO_FATAL_FAILURE(AddCatRule(pstate));
+  AssertParse(pstate, manifest);
 
   string err;
   BuildLog build_log, *pbuild_log = NULL;
@@ -532,13 +535,13 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest,
 
   DepsLog deps_log, *pdeps_log = NULL;
   if (deps_path) {
-    ASSERT_TRUE(deps_log.Load(deps_path, &state, &err));
+    ASSERT_TRUE(deps_log.Load(deps_path, pstate, &err));
     ASSERT_TRUE(deps_log.OpenForWrite(deps_path, &err));
     ASSERT_EQ("", err);
     pdeps_log = &deps_log;
   }
 
-  Builder builder(&state, config_, pbuild_log, pdeps_log, &fs_);
+  Builder builder(pstate, config_, pbuild_log, pdeps_log, &fs_);
   EXPECT_TRUE(builder.AddTarget(target, &err));
 
   command_runner_.commands_ran_.clear();
@@ -1092,6 +1095,31 @@ TEST_F(BuildTest, SwallowFailuresLimit) {
   ASSERT_EQ("cannot make progress due to previous errors", err);
 }
 
+TEST_F(BuildTest, PoolEdgesReadyButNotWanted) {
+  fs_.Create("x", "");
+
+  const char* manifest =
+    "pool some_pool\n"
+    "  depth = 4\n"
+    "rule touch\n"
+    "  command = touch $out\n"
+    "  pool = some_pool\n"
+    "rule cc\n"
+    "  command = touch grit\n"
+    "\n"
+    "build B.d.stamp: cc | x\n"
+    "build C.stamp: touch B.d.stamp\n"
+    "build final.stamp: touch || C.stamp\n";
+
+  RebuildTarget("final.stamp", manifest);
+
+  fs_.RemoveFile("B.d.stamp");
+
+  State save_state;
+  RebuildTarget("final.stamp", manifest, NULL, NULL, &save_state);
+  EXPECT_GE(save_state.LookupPool("some_pool")->current_use(), 0);
+}
+
 struct BuildWithLogTest : public BuildTest {
   BuildWithLogTest() {
     builder_.SetBuildLog(&build_log_);
index 5000138..d7987ba 100644 (file)
@@ -44,6 +44,7 @@ struct Pool {
   bool is_valid() const { return depth_ >= 0; }
   int depth() const { return depth_; }
   const string& name() const { return name_; }
+  int current_use() const { return current_use_; }
 
   /// true if the Pool might delay this edge
   bool ShouldDelayEdge() const { return depth_ != 0; }