Work around popen not being thread safe on MacOS

include/vcpkg/base/messages.h

@@ -897,6 +897,10 @@ namespace vcpkg
                     "Embedding `vcpkg-configuration` in a manifest file is an EXPERIMENTAL feature.");
     DECLARE_MESSAGE(EmptyArg, (msg::option), "", "The option --{option} must be passed a non-empty argument.");
     DECLARE_MESSAGE(EmptyLicenseExpression, (), "", "SPDX license expression was empty.");
+    DECLARE_MESSAGE(EnvInvalidMaxConcurrency,
+                    (msg::env_var, msg::value),
+                    "{value} is the invalid value of an environment variable",
+                    "{env_var} is {value}, must be > 0");
     DECLARE_MESSAGE(EndOfStringInCodeUnit, (), "", "found end of string in middle of code point");
     DECLARE_MESSAGE(EnvStrFailedToExtract, (), "", "could not expand the environment string:");
     DECLARE_MESSAGE(ErrorDetectingCompilerInfo,

include/vcpkg/base/system.h

@@ -60,7 +60,7 @@ namespace vcpkg
 
     const Optional<Path>& get_program_files_platform_bitness();
 
-    int get_concurrency();
+    unsigned int get_concurrency();
 
     Optional<CPUArchitecture> guess_visual_studio_prompt_target_architecture();
 }

locales/messages.json

@@ -261,6 +261,8 @@
   "_EmptyArg.comment": "An example of {option} is editable.",
   "EmptyLicenseExpression": "SPDX license expression was empty.",
   "EndOfStringInCodeUnit": "found end of string in middle of code point",
+  "EnvInvalidMaxConcurrency": "{env_var} is {value}, must be > 0",
+  "_EnvInvalidMaxConcurrency.comment": "{value} is the invalid value of an environment variable An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
   "EnvStrFailedToExtract": "could not expand the environment string:",
   "ErrorDetectingCompilerInfo": "while detecting compiler information:\nThe log file content at \"{path}\" is:",
   "_ErrorDetectingCompilerInfo.comment": "An example of {path} is /foo/bar.",

src/vcpkg-test/system.cpp

@@ -142,3 +142,37 @@ TEST_CASE ("cmdlinebuilder", "[system]")
     REQUIRE(cmd.command_line() == "\"trailing\\\\slash\\\\\" \"inner\\\"quotes\"");
 #endif
 }
+
+TEST_CASE ("cmd_execute_and_capture_output_parallel", "[system]")
+{
+    std::vector<vcpkg::Command> vec;
+    for (size_t i = 0; i < 50; i++)
+    {
+#if defined(_WIN32)
+        vcpkg::Command cmd("cmd.exe");
+        cmd.string_arg("/c");
+        const auto cmd_str = "echo " + std::to_string(i);
+        cmd.string_arg(cmd_str);
+#else
+        vcpkg::Command cmd("echo");
+        const auto cmd_str = std::string(i, 'a');
+        cmd.string_arg(cmd_str);
+#endif
+        vec.emplace_back(std::move(cmd));
+    }
+
+    auto res = vcpkg::cmd_execute_and_capture_output_parallel(vcpkg::View<vcpkg::Command>(vec));
+
+    for (size_t i = 0; i < res.size(); ++i)
+    {
+        auto out = res[i].get();
+        REQUIRE(out != nullptr);
+        REQUIRE(out->exit_code == 0);
+
+#if defined(_WIN32)
+        REQUIRE(out->output == (std::to_string(i) + "\r\n"));
+#else
+        REQUIRE(out->output == (std::string(i, 'a') + "\n"));
+#endif
+    }
+}

src/vcpkg/archives.cpp

@@ -346,22 +346,6 @@ namespace vcpkg
     {
         auto results =
             cmd_execute_and_capture_output_parallel(jobs, default_working_directory, get_clean_environment());
-#ifdef __APPLE__
-        size_t i = 0;
-        for (auto& maybe_result : results)
-        {
-            if (const auto result = maybe_result.get())
-            {
-                if (result->exit_code == 127 && result->output.empty())
-                {
-                    Debug::print(jobs[i].command_line(), ": pclose returned 127, try again \n");
-                    maybe_result =
-                        cmd_execute_and_capture_output(jobs[i], default_working_directory, get_clean_environment());
-                }
-            }
-            ++i;
-        }
-#endif
 
         std::vector<ExpectedL<Unit>> filtered_results;
         filtered_results.reserve(jobs.size());

src/vcpkg/base/messages.cpp

@@ -554,6 +554,7 @@ namespace vcpkg
     REGISTER_MESSAGE(EmptyArg);
     REGISTER_MESSAGE(EmptyLicenseExpression);
     REGISTER_MESSAGE(EndOfStringInCodeUnit);
+    REGISTER_MESSAGE(EnvInvalidMaxConcurrency);
     REGISTER_MESSAGE(EnvStrFailedToExtract);
     REGISTER_MESSAGE(ErrorDetectingCompilerInfo);
     REGISTER_MESSAGE(ErrorIndividualPackagesUnsupported);

src/vcpkg/base/system.cpp

@@ -468,17 +468,35 @@ namespace vcpkg
         return ProgramW6432;
     }
 
-    int get_concurrency()
+    unsigned int get_concurrency()
     {
-        static int concurrency = [] {
+        static unsigned int concurrency = [] {
             auto user_defined_concurrency = get_environment_variable("VCPKG_MAX_CONCURRENCY");
             if (user_defined_concurrency)
             {
-                return std::stoi(user_defined_concurrency.value_or_exit(VCPKG_LINE_INFO));
+                int res = -1;
+                try
+                {
+                    res = std::stoi(user_defined_concurrency.value_or_exit(VCPKG_LINE_INFO));
+                }
+                catch (std::exception&)
+                {
+                    Checks::msg_exit_with_message(
+                        VCPKG_LINE_INFO, msgOptionMustBeInteger, msg::option = "VCPKG_MAX_CONCURRENCY");
+                }
+
+                if (!(res > 0))
+                {
+                    Checks::msg_exit_with_message(VCPKG_LINE_INFO,
+                                                  msgEnvInvalidMaxConcurrency,
+                                                  msg::env_var = "VCPKG_MAX_CONCURRENCY",
+                                                  msg::value = res);
+                }
+                return static_cast<unsigned int>(res);
             }
             else
             {
-                return static_cast<int>(std::thread::hardware_concurrency()) + 1;
+                return std::thread::hardware_concurrency() + 1;
             }
         }();
 

src/vcpkg/base/system.process.cpp

@@ -466,8 +466,8 @@ namespace vcpkg
                                                                                       const WorkingDirectory& wd,
                                                                                       const Environment& env)
     {
-        std::vector<ExpectedL<ExitCodeAndOutput>> res(cmd_lines.size(), LocalizedString());
-        if (cmd_lines.size() == 0)
+        std::vector<ExpectedL<ExitCodeAndOutput>> res(cmd_lines.size(), LocalizedString{});
+        if (cmd_lines.empty())
         {
             return res;
         }
@@ -480,7 +480,7 @@ namespace vcpkg
 
         std::atomic<size_t> work_item{0};
         const auto num_threads =
-            static_cast<size_t>(std::max(1, std::min(get_concurrency(), static_cast<int>(cmd_lines.size()))));
+            std::max(static_cast<size_t>(1), std::min(static_cast<size_t>(get_concurrency()), cmd_lines.size()));
 
         auto work = [&]() {
             std::size_t item;
@@ -491,6 +491,7 @@ namespace vcpkg
         };
 
         std::vector<std::future<void>> workers;
+        workers.reserve(num_threads - 1);
         for (size_t x = 0; x < num_threads - 1; ++x)
         {
             workers.emplace_back(std::async(std::launch::async | std::launch::deferred, work));
@@ -901,7 +902,7 @@ namespace vcpkg
                 return output.wait_and_stream_output(data_cb, encoding);
             });
         g_ctrl_c_state.transition_from_spawn_process();
-#else  // ^^^ _WIN32 // !_WIN32 vvv
+#else // ^^^ _WIN32 // !_WIN32 vvv
         Checks::check_exit(VCPKG_LINE_INFO, encoding == Encoding::Utf8);
 
         std::string actual_cmd_line;
@@ -924,7 +925,23 @@ namespace vcpkg
         // Flush stdout before launching external process
         fflush(stdout);
 
-        const auto pipe = popen(actual_cmd_line.c_str(), "r");
+        FILE* pipe = nullptr;
+#if defined(__APPLE__)
+        static std::mutex mtx;
+#endif
+
+        // Scope for lock guard
+        {
+#if defined(__APPLE__)
+            // `popen` sometimes returns 127 on OSX when executed in parallel.
+            // Related: https://github.com/microsoft/vcpkg-tool/pull/695#discussion_r973364608
+
+            std::lock_guard guard(mtx);
+#endif
+
+            pipe = popen(actual_cmd_line.c_str(), "r");
+        }
+
         if (pipe == nullptr)
         {
             return format_system_error_message("popen", errno);
@@ -942,7 +959,15 @@ namespace vcpkg
             return format_system_error_message("feof", errno);
         }
 
-        int ec = pclose(pipe);
+        int ec;
+        // Scope for lock guard
+        {
+#if defined(__APPLE__)
+            // See the comment above at the call to `popen`.
+            std::lock_guard guard(mtx);
+#endif
+            ec = pclose(pipe);
+        }
         if (WIFEXITED(ec))
         {
             ec = WEXITSTATUS(ec);

src/vcpkg/commands.dependinfo.cpp

@@ -120,7 +120,7 @@ namespace vcpkg::Commands::DependInfo
                 std::string value = iter->second;
                 try
                 {
-                    return std::stoi(value);
+                    return std::max(std::stoi(value), NO_RECURSE_LIMIT_VALUE);
                 }
                 catch (std::exception&)
                 {