Fix crash-on-shutdown around all "CE" powered commands (#534)

* Fix crash-on-shutdown around all "CE" powered commands introduced in 0840ffd These commands called into CE bits, and exit_with_code, while holding the lock. This causes an immediate crash on shutdown because the lock is reentered. This change ensures that a minimal amount of code runs while holding the lock. * Protect access to filter for being engaged. * You want that to compile?
microsoft · May 4, 2022 · 67e17c1 · 67e17c1
1 parent 8e70366
commit 67e17c1
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git a/include/vcpkg/base/hash.h b/include/vcpkg/base/hash.h
@@ -31,5 +31,6 @@ namespace vcpkg::Hash
 
     std::string get_bytes_hash(const void* first, const void* last, Algorithm algo);
     std::string get_string_hash(StringView s, Algorithm algo);
+    std::string get_string_sha256(StringView s);
     ExpectedL<std::string> get_file_hash(const Filesystem& fs, const Path& target, Algorithm algo);
 }
diff --git a/src/vcpkg/base/hash.cpp b/src/vcpkg/base/hash.cpp
@@ -558,6 +558,8 @@ namespace vcpkg::Hash
         return get_bytes_hash(sv.data(), sv.data() + sv.size(), algo);
     }
 
+    std::string get_string_sha256(StringView s) { return get_string_hash(s, Hash::Algorithm::Sha256); }
+
     ExpectedL<std::string> get_file_hash(const Filesystem& fs, const Path& path, Algorithm algo)
     {
         std::error_code ec;

diff --git a/src/vcpkg/commands.add.cpp b/src/vcpkg/commands.add.cpp
@@ -64,10 +64,12 @@ namespace vcpkg::Commands
                                    msg::command_line = "vcpkg add artifact");
 
             auto artifact_name = args.command_arguments[1];
-
-            auto metrics = LockGuardPtr<Metrics>(g_metrics);
-            metrics->track_property("command_context", "artifact");
-            metrics->track_property("command_args", Hash::get_string_hash(artifact_name, Hash::Algorithm::Sha256));
+            auto artifact_hash = Hash::get_string_hash(artifact_name, Hash::Algorithm::Sha256);
+            {
+                auto metrics = LockGuardPtr<Metrics>(g_metrics);
+                metrics->track_property("command_context", "artifact");
+                metrics->track_property("command_args", artifact_hash);
+            } // unlock g_metrics
 
             std::string ce_args[] = {"add", artifact_name};
             Checks::exit_with_code(VCPKG_LINE_INFO, run_configure_environment_command(paths, ce_args));
@@ -135,12 +137,14 @@ namespace vcpkg::Commands
                 manifest->path, Json::stringify(serialize_manifest(manifest_scf), {}), VCPKG_LINE_INFO);
             msg::println(msgAddPortSucceded);
 
-            auto metrics = LockGuardPtr<Metrics>(g_metrics);
-            metrics->track_property("command_context", "port");
-            metrics->track_property("command_args",
-                                    Strings::join(" ", Util::fmap(specs, [](auto&& spec) -> std::string {
-                                                      return Hash::get_string_hash(spec.name, Hash::Algorithm::Sha256);
-                                                  })));
+            auto command_args_hash = Strings::join(" ", Util::fmap(specs, [](auto&& spec) -> std::string {
+                                                       return Hash::get_string_hash(spec.name, Hash::Algorithm::Sha256);
+                                                   }));
+            {
+                auto metrics = LockGuardPtr<Metrics>(g_metrics);
+                metrics->track_property("command_context", "port");
+                metrics->track_property("command_args", command_args_hash);
+            } // unlock metrics
 
             Checks::exit_success(VCPKG_LINE_INFO);
         }

diff --git a/src/vcpkg/commands.find.cpp b/src/vcpkg/commands.find.cpp
@@ -226,27 +226,32 @@ namespace vcpkg::Commands
                 print2(Color::warning, "--x-json has no effect on find artifact\n");
             }
 
-            auto metrics = LockGuardPtr<Metrics>(g_metrics);
-            metrics->track_property("command_context", "artifact");
-            if (filter.has_value())
+            Optional<std::string> filter_hash = filter.map(Hash::get_string_sha256);
+            auto args_hash = Hash::get_string_hash(filter.value_or_exit(VCPKG_LINE_INFO), Hash::Algorithm::Sha256);
             {
-                metrics->track_property(
-                    "command_args",
-                    Hash::get_string_hash(filter.value_or_exit(VCPKG_LINE_INFO), Hash::Algorithm::Sha256));
-            }
+                auto metrics = LockGuardPtr<Metrics>(g_metrics);
+                metrics->track_property("command_context", "artifact");
+                if (auto p_filter_hash = filter_hash.get())
+                {
+                    metrics->track_property("command_args", *p_filter_hash);
+                }
+            } // unlock metrics
+
             perform_find_artifact_and_exit(paths, filter);
         }
 
         if (selector == "port")
         {
-            auto metrics = LockGuardPtr<Metrics>(g_metrics);
-            metrics->track_property("command_context", "port");
-            if (filter.has_value())
+            Optional<std::string> filter_hash = filter.map(Hash::get_string_sha256);
             {
-                metrics->track_property(
-                    "command_args",
-                    Hash::get_string_hash(filter.value_or_exit(VCPKG_LINE_INFO), Hash::Algorithm::Sha256));
-            }
+                auto metrics = LockGuardPtr<Metrics>(g_metrics);
+                metrics->track_property("command_context", "port");
+                if (auto p_filter_hash = filter_hash.get())
+                {
+                    metrics->track_property("command_args", *p_filter_hash);
+                }
+            } // unlock metrics
+
             perform_find_port_and_exit(paths, full_description, enable_json, filter, args.overlay_ports);
         }