Fix a GN race condition.
The loader maintains a reference count on the "pending work". But the
Setup object was starting the load before incrementing this reference count.
It the loader happened to load anything on the background thread before the
main thread could increment the reference count, GN would think there is no
work left and exit early.
There is another possible race condition between defining items and doing
work on the thread pool can cause the build to be marked complete before
everything is actually done. The race condition is described in more detail
in the added comment in setup.cc.
BUG=674213
Review-Url: https://codereview.chromium.org/2617253002
Cr-Original-Commit-Position: refs/heads/master@{#442416}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 717182cd794aae4577fbae651a88e3ba6d973dc8
diff --git a/tools/gn/setup.cc b/tools/gn/setup.cc
index 8a433bc..06062e9 100644
--- a/tools/gn/setup.cc
+++ b/tools/gn/setup.cc
@@ -137,15 +137,31 @@
return FindDotFile(up_one_dir);
}
+void ForwardItemDefinedToBuilderInMainThread(
+ Builder* builder_call_on_main_thread_only,
+ std::unique_ptr<Item> item) {
+ builder_call_on_main_thread_only->ItemDefined(std::move(item));
+
+ // Pair to the Increment in ItemDefinedCallback.
+ g_scheduler->DecrementWorkCount();
+}
+
// Called on any thread. Post the item to the builder on the main thread.
void ItemDefinedCallback(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
Builder* builder_call_on_main_thread_only,
std::unique_ptr<Item> item) {
DCHECK(item);
+
+ // Increment the work count for the duration of defining the item with the
+ // builder. Otherwise finishing this callback will race finishing loading
+ // files. If there is no other pending work at any point in the middle of
+ // this call completing on the main thread, the 'Complete' function will
+ // be signaled and we'll stop running with an incomplete build.
+ g_scheduler->IncrementWorkCount();
task_runner->PostTask(
FROM_HERE,
- base::Bind(&Builder::ItemDefined,
+ base::Bind(&ForwardItemDefinedToBuilderInMainThread,
base::Unretained(builder_call_on_main_thread_only),
base::Passed(&item)));
}
@@ -321,11 +337,11 @@
}
void Setup::RunPreMessageLoop() {
- // Load the root build file.
- loader_->Load(root_build_file_, LocationRange(), Label());
-
// Will be decremented with the loader is drained.
g_scheduler->IncrementWorkCount();
+
+ // Load the root build file.
+ loader_->Load(root_build_file_, LocationRange(), Label());
}
bool Setup::RunPostMessageLoop() {
