Revert 232657 "GN: toolchain threading cleanup"

trunk/src/tools/gn/toolchain_manager.cc

Index: trunk/src/tools/gn/toolchain_manager.cc
===================================================================
--- trunk/src/tools/gn/toolchain_manager.cc	(revision 232660)
+++ trunk/src/tools/gn/toolchain_manager.cc	(working copy)
@@ -78,19 +78,13 @@
        const Label& toolchain_name,
        const std::string& output_subdir_name)
       : state(TOOLCHAIN_NOT_LOADED),
-        settings(build_settings, output_subdir_name),
-        toolchain(new Toolchain(&settings, toolchain_name)),
+        toolchain(toolchain_name),
         toolchain_set(false),
+        settings(build_settings, &toolchain, output_subdir_name),
         toolchain_file_loaded(false),
         item_node(NULL) {
-    settings.set_toolchain_label(toolchain_name);
   }
 
-  ~Info() {
-    if (!item_node)  // See toolchain definition for more.
-      delete toolchain;
-  }
-
   // Makes sure that an ItemNode is created for the toolchain, which lets
   // targets depend on the (potentially future) loading of the toolchain.
   //
@@ -101,13 +95,17 @@
   void EnsureItemNode() {
     if (!item_node) {
       ItemTree& tree = settings.build_settings()->item_tree();
-      item_node = new ItemNode(toolchain);
+      item_node = new ItemNode(&toolchain);
       tree.AddNodeLocked(item_node);
     }
   }
 
   ToolchainState state;
 
+  Toolchain toolchain;
+  bool toolchain_set;
+  LocationRange toolchain_definition_location;
+
   // The first place in the build that we saw a reference for this toolchain.
   // This allows us to report errors if it can't be loaded and blame some
   // reasonable place of the code. This will be empty for the default toolchain.
@@ -120,13 +118,6 @@
   // is only ever read or written inside the lock.
   Settings settings;
 
-  // When we create an item node, this pointer will be owned by that node
-  // so it's lifetime is managed by the dependency graph. Before we've created
-  // the ItemNode, this class has to takre responsibility for this pointer.
-  Toolchain* toolchain;
-  bool toolchain_set;
-  LocationRange toolchain_definition_location;
-
   // Set when the file corresponding to the toolchain definition is loaded.
   // This will normally be set right after "toolchain_set". However, if the
   // toolchain definition is missing, the file might be marked loaded but the
@@ -211,7 +202,7 @@
   // Since we don't allow defining a toolchain more than once, we know that
   // once it's set it won't be mutated, so we can safely return this pointer
   // for reading outside the lock.
-  return found->second->toolchain;
+  return &found->second->toolchain;
 }
 
 bool ToolchainManager::SetDefaultToolchainUnlocked(
@@ -270,10 +261,13 @@
   }
 
   // The labels should match or else we're setting the wrong one!
-  CHECK(info->toolchain->label() == tc.label());
+  CHECK(info->toolchain.label() == tc.label());
 
-  // Save the toolchain. We can just overwrite our definition.
-  *info->toolchain = tc;
+  // Save the toolchain. We can just overwrite our definition, but we need to
+  // be careful to preserve the is_default flag.
+  bool is_default = info->toolchain.is_default();
+  info->toolchain = tc;
+  info->toolchain.set_is_default(is_default);
 
   if (info->toolchain_set) {
     *err = Err(defined_from, "Duplicate toolchain definition.");
@@ -429,9 +423,8 @@
   // to set the label, but we can assign the toolchain to a new one. Loading
   // the build config can not change the toolchain, so we won't be overwriting
   // anything useful.
-  *info->toolchain = Toolchain(&info->settings, default_toolchain_);
-  info->settings.set_is_default(true);
-  info->settings.set_toolchain_label(default_toolchain_);
+  info->toolchain = Toolchain(default_toolchain_);
+  info->toolchain.set_is_default(true);
   info->EnsureItemNode();
 
   // The default toolchain is loaded in greedy mode so all targets we
@@ -493,8 +486,8 @@
     Scope* base_config = info->settings.base_config();
     base_config->set_source_dir(SourceDir("//"));
 
-    info->settings.build_settings()->build_args().SetupRootScope(
-        base_config, info->toolchain->args());
+    info->settings.build_settings()->build_args().SetupRootScope(base_config,
+        info->settings.toolchain()->args());
 
     base_config->SetProcessingBuildConfig();
     if (is_default)
@@ -502,7 +495,7 @@
 
     ScopedTrace trace(TraceItem::TRACE_FILE_EXECUTE,
         info->settings.build_settings()->build_config_file().value());
-    trace.SetToolchain(info->settings.toolchain_label());
+    trace.SetToolchain(info->settings.toolchain()->label());
 
     const BlockNode* root_block = root->AsBlock();
     Err err;
@@ -547,7 +540,7 @@
   if (root && !g_scheduler->is_failed()) {
     if (g_scheduler->verbose_logging()) {
       g_scheduler->Log("Running", file_name.value() + " with toolchain " +
-                       info->toolchain->label().GetUserVisibleName(false));
+                       info->toolchain.label().GetUserVisibleName(false));
     }
 
     Scope our_scope(info->settings.base_config());
@@ -555,7 +548,7 @@
     our_scope.set_source_dir(file_name.GetDir());
 
     ScopedTrace trace(TraceItem::TRACE_FILE_EXECUTE, file_name.value());
-    trace.SetToolchain(info->settings.toolchain_label());
+    trace.SetToolchain(info->settings.toolchain()->label());
 
     Err err;
     root->Execute(&our_scope, &err);