From 6652258c2fa510f043d32be95ef4c44eaa2800dc Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 23 Apr 2015 15:54:28 -0700 Subject: Fix pool use count going unbalanced --- src/build.cc | 4 +++- src/build_test.cc | 44 ++++++++++++++++++++++++++++++++++++-------- src/state.h | 1 + 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/build.cc b/src/build.cc index ccdb37f..f14831e 100644 --- a/src/build.cc +++ b/src/build.cc @@ -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); } } diff --git a/src/build_test.cc b/src/build_test.cc index a313693..52a17c9 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -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_); diff --git a/src/state.h b/src/state.h index 5000138..d7987ba 100644 --- a/src/state.h +++ b/src/state.h @@ -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; } -- cgit v0.12