From 6b204c150ef34578d72e12b52db6ddf0a6360e0d Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Wed, 12 Aug 2026 17:28:28 -0300 Subject: [PATCH 1/4] Add `spawn_and_capture` and give `spawn` a shared input settings type Signed-off-by: Juan Cruz Viotti --- src/lang/process/CMakeLists.txt | 6 +- src/lang/process/command_line.h | 58 ++ .../process/include/sourcemeta/core/process.h | 84 ++- src/lang/process/spawn.cc | 654 +++++++++++++++--- test/process/CMakeLists.txt | 15 +- .../process/process_spawn_and_capture_main.cc | 193 ++++++ .../process/process_spawn_and_capture_test.cc | 330 +++++++++ test/process/process_spawn_input_test.cc | 117 ++++ test/process/process_spawn_main.cc | 2 +- test/process/process_spawn_test_unix.cc | 4 +- test/process/process_spawn_test_windows.cc | 4 +- 11 files changed, 1371 insertions(+), 96 deletions(-) create mode 100644 src/lang/process/command_line.h create mode 100644 test/process/process_spawn_and_capture_main.cc create mode 100644 test/process/process_spawn_and_capture_test.cc create mode 100644 test/process/process_spawn_input_test.cc diff --git a/src/lang/process/CMakeLists.txt b/src/lang/process/CMakeLists.txt index b44d36c88d..93380e06ac 100644 --- a/src/lang/process/CMakeLists.txt +++ b/src/lang/process/CMakeLists.txt @@ -1,7 +1,11 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME process PRIVATE_HEADERS error.h - SOURCES spawn.cc) + SOURCES spawn.cc command_line.h) if(SOURCEMETA_CORE_INSTALL) sourcemeta_library_install(NAMESPACE sourcemeta PROJECT core NAME process) endif() + +if(WIN32) + target_link_libraries(sourcemeta_core_process PRIVATE sourcemeta::core::text) +endif() diff --git a/src/lang/process/command_line.h b/src/lang/process/command_line.h new file mode 100644 index 0000000000..c343f42859 --- /dev/null +++ b/src/lang/process/command_line.h @@ -0,0 +1,58 @@ +#ifndef SOURCEMETA_CORE_PROCESS_COMMAND_LINE_H_ +#define SOURCEMETA_CORE_PROCESS_COMMAND_LINE_H_ + +#if defined(_WIN32) && !defined(__MSYS__) && !defined(__CYGWIN__) && \ + !defined(__MINGW32__) && !defined(__MINGW64__) + +#include // std::size_t +#include // std::string +#include // std::string_view + +namespace sourcemeta::core { + +namespace { + +// Quote a single argument for the inverse of CommandLineToArgvW, so that the +// child reconstructs the exact same argument vector +auto append_quoted_argument(std::string &command_line, + const std::string_view argument) -> void { + const bool needs_quoting{argument.empty() || + argument.find_first_of(" \t\"") != + std::string_view::npos}; + + if (!needs_quoting) { + command_line.append(argument); + return; + } + + command_line.push_back('"'); + + for (auto cursor = argument.cbegin();; ++cursor) { + std::size_t backslash_count{0}; + while (cursor != argument.cend() && *cursor == '\\') { + ++cursor; + ++backslash_count; + } + + if (cursor == argument.cend()) { + command_line.append(backslash_count * 2, '\\'); + break; + } else if (*cursor == '"') { + command_line.append(backslash_count * 2 + 1, '\\'); + command_line.push_back('"'); + } else { + command_line.append(backslash_count, '\\'); + command_line.push_back(*cursor); + } + } + + command_line.push_back('"'); +} + +} // namespace + +} // namespace sourcemeta::core + +#endif + +#endif diff --git a/src/lang/process/include/sourcemeta/core/process.h b/src/lang/process/include/sourcemeta/core/process.h index 237cf9eb9f..3b861e424f 100644 --- a/src/lang/process/include/sourcemeta/core/process.h +++ b/src/lang/process/include/sourcemeta/core/process.h @@ -11,7 +11,10 @@ #include // std::filesystem #include // std::initializer_list +#include // std::map +#include // std::optional #include // std::span +#include // std::string #include // std::string_view /// @defgroup process Process @@ -25,11 +28,25 @@ namespace sourcemeta::core { +/// @ingroup process +/// The settings for running a program. +struct ProcessInput { + /// The working directory of the program. It must be an absolute path to an + /// existing directory + std::filesystem::path directory{std::filesystem::current_path()}; + /// The entire environment of the program, replacing rather than extending the + /// environment of the caller. Without a value, the caller's environment is + /// inherited as it stands. The referenced names and values must outlive the + /// call + std::optional> environment; + /// The bytes to feed the program on its standard input. The referenced buffer + /// must outlive the call + std::string_view standard_input; +}; + /// @ingroup process /// /// Spawn a program piping its output to the current stdio configuration. -/// The directory parameter specifies the working directory for the spawned -/// process. It must be an absolute path to an existing directory. /// /// ```cpp /// #include @@ -41,15 +58,12 @@ namespace sourcemeta::core { SOURCEMETA_CORE_PROCESS_EXPORT auto spawn(const std::string &program, std::initializer_list arguments, - const std::filesystem::path &directory = - std::filesystem::current_path()) -> int; + const ProcessInput &input = {}) -> int; /// @ingroup process /// /// Spawn a program piping its output to the current stdio configuration. /// This overload accepts a span for dynamic argument lists. -/// The directory parameter specifies the working directory for the spawned -/// process. It must be an absolute path to an existing directory. /// /// ```cpp /// #include @@ -62,10 +76,60 @@ auto spawn(const std::string &program, /// assert(exit_code == 0); /// ``` SOURCEMETA_CORE_PROCESS_EXPORT -auto spawn( - const std::string &program, std::span arguments, - const std::filesystem::path &directory = std::filesystem::current_path()) - -> int; +auto spawn(const std::string &program, + std::span arguments, + const ProcessInput &input = {}) -> int; + +/// @ingroup process +/// The result of running a program while capturing what it writes. +struct ProcessOutput { + /// The code the program exited with, or no value if it terminated abnormally, + /// such as by a signal + std::optional exit_code; + /// Everything the program wrote to its standard output + std::string standard_output; + /// Everything the program wrote to its standard error + std::string standard_error; +}; + +/// @ingroup process +/// +/// Spawn a program, feeding it the given input and capturing both of its output +/// streams in full. +/// +/// ```cpp +/// #include +/// #include +/// +/// const auto result{sourcemeta::core::spawn_and_capture("echo", {"foo"})}; +/// assert(result.exit_code.value() == 0); +/// assert(result.standard_output == "foo\n"); +/// ``` +SOURCEMETA_CORE_PROCESS_EXPORT +auto spawn_and_capture(const std::string &program, + std::initializer_list arguments, + const ProcessInput &input = {}) -> ProcessOutput; + +/// @ingroup process +/// +/// Spawn a program, feeding it the given input and capturing both of its output +/// streams in full. This overload accepts a span for dynamic argument lists. +/// +/// ```cpp +/// #include +/// #include +/// #include +/// #include +/// +/// std::vector arguments{"foo", "bar"}; +/// const auto result{sourcemeta::core::spawn_and_capture("echo", arguments)}; +/// assert(result.exit_code.value() == 0); +/// assert(result.standard_output == "foo bar\n"); +/// ``` +SOURCEMETA_CORE_PROCESS_EXPORT +auto spawn_and_capture(const std::string &program, + std::span arguments, + const ProcessInput &input = {}) -> ProcessOutput; } // namespace sourcemeta::core diff --git a/src/lang/process/spawn.cc b/src/lang/process/spawn.cc index 43cb1aaa10..d1dd11bb70 100644 --- a/src/lang/process/spawn.cc +++ b/src/lang/process/spawn.cc @@ -1,112 +1,474 @@ #include +// NOLINTBEGIN(misc-include-cleaner) +#include "command_line.h" +// NOLINTEND(misc-include-cleaner) + +#include // std::array #include // assert -#include // ENOENT, EINTR, errno +#include // EAGAIN, EINTR, ENOENT, errno +#include // std::size_t #include // std::filesystem #include // std::initializer_list #include // std::span #include // std::string +#include // std::string_view +#include // std::move #include // std::vector #if defined(_WIN32) && !defined(__MSYS__) && !defined(__CYGWIN__) && \ !defined(__MINGW32__) && !defined(__MINGW64__) #define WIN32_LEAN_AND_MEAN -#include // std::size_t -#include // CreateProcess, PROCESS_INFORMATION, STARTUPINFO, WaitForSingleObject, GetExitCodeProcess, WAIT_FAILED +#define NOMINMAX +#include + +#include // std::sort +#include // std::ref +#include // std::map +#include // std::thread +#include // CreateProcessW, CreatePipe, ReadFile, WriteFile, SetHandleInformation, STARTUPINFOW, PROCESS_INFORMATION, WaitForSingleObject, GetExitCodeProcess, MultiByteToWideChar, CloseHandle, WAIT_FAILED #else -#include // posix_spawnp, posix_spawnattr_t, posix_spawnattr_init, posix_spawnattr_destroy, posix_spawn_file_actions_t, posix_spawn_file_actions_init, posix_spawn_file_actions_destroy, pid_t +#include // sigset_t, sigemptyset, sigaddset, sigismember, sigpending, sigwait, SIGPIPE, SIG_BLOCK, SIG_SETMASK +#include // fcntl, FD_CLOEXEC, F_GETFD, F_SETFD, F_GETFL, F_SETFL, O_NONBLOCK +#include // poll, pollfd, POLLIN, POLLOUT, POLLERR, POLLHUP, POLLNVAL +#include // pthread_sigmask +#include // posix_spawnp, posix_spawnattr_t, posix_spawnattr_init, posix_spawnattr_destroy, posix_spawn_file_actions_t, posix_spawn_file_actions_init, posix_spawn_file_actions_destroy, posix_spawn_file_actions_adddup2, pid_t #include // waitpid, WIFEXITED, WEXITSTATUS - -#if defined(__MSYS__) || defined(__CYGWIN__) || defined(__MINGW32__) || \ - defined(__MINGW64__) -#include // chdir -#endif +#include // pipe, read, write, close, chdir, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO extern char **environ; #endif +namespace { + +// Large enough that a chatty program is drained in a handful of system calls, +// small enough to sit on the stack of any thread +constexpr std::size_t TRANSFER_BUFFER_SIZE{16384}; + +} // namespace + #if defined(_WIN32) && !defined(__MSYS__) && !defined(__CYGWIN__) && \ !defined(__MINGW32__) && !defined(__MINGW64__) + +namespace { + +class Handle { +public: + Handle() = default; + explicit Handle(HANDLE value) : value_{value} {} + ~Handle() { this->close(); } + Handle(const Handle &) = delete; + auto operator=(const Handle &) -> Handle & = delete; + Handle(Handle &&other) noexcept : value_{other.value_} { + other.value_ = nullptr; + } + auto operator=(Handle &&other) noexcept -> Handle & { + if (this != &other) { + this->close(); + this->value_ = other.value_; + other.value_ = nullptr; + } + + return *this; + } + + [[nodiscard]] auto get() const noexcept -> HANDLE { return this->value_; } + + [[nodiscard]] auto valid() const noexcept -> bool { + return this->value_ != nullptr && this->value_ != INVALID_HANDLE_VALUE; + } + + auto close() noexcept -> void { + if (this->valid()) { + CloseHandle(this->value_); + } + + this->value_ = nullptr; + } + +private: + HANDLE value_{nullptr}; +}; + +auto to_wide(const std::string_view input) -> std::wstring { + if (input.empty()) { + return {}; + } + + const int length{MultiByteToWideChar( + CP_UTF8, 0, input.data(), static_cast(input.size()), nullptr, 0)}; + if (length <= 0) { + return {}; + } + + std::wstring result(static_cast(length), L'\0'); + MultiByteToWideChar(CP_UTF8, 0, input.data(), static_cast(input.size()), + result.data(), length); + return result; +} + +// An inheritable pipe whose parent-side handle is explicitly made +// non-inheritable, so that the child never holds the end the parent works with +auto make_pipe(Handle &read_end, Handle &write_end, const bool inherit_read) + -> bool { + SECURITY_ATTRIBUTES attributes{}; + attributes.nLength = sizeof(attributes); + attributes.lpSecurityDescriptor = nullptr; + attributes.bInheritHandle = TRUE; + + HANDLE raw_read{nullptr}; + HANDLE raw_write{nullptr}; + if (!CreatePipe(&raw_read, &raw_write, &attributes, 0)) { + return false; + } + + read_end = Handle{raw_read}; + write_end = Handle{raw_write}; + const HANDLE parent_end{inherit_read ? raw_write : raw_read}; + return SetHandleInformation(parent_end, HANDLE_FLAG_INHERIT, 0) != 0; +} + +auto read_handle_to_string(HANDLE handle, std::string &destination) -> void { + std::array buffer{}; + DWORD count{0}; + while (ReadFile(handle, buffer.data(), static_cast(buffer.size()), + &count, nullptr) && + count > 0) { + destination.append(buffer.data(), count); + } +} + +// The block is a run of null terminated name=value strings closed by an extra +// null. Windows expects the names ordered case-insensitively +auto to_environment_block( + const std::map &environment) + -> std::wstring { + std::vector *> + entries; + entries.reserve(environment.size()); + for (const auto &entry : environment) { + entries.push_back(&entry); + } + + std::sort( + entries.begin(), entries.end(), [](const auto *left, const auto *right) { + return sourcemeta::core::less_ignore_case(left->first, right->first); + }); + + std::wstring block; + for (const auto *entry : entries) { + block.append(to_wide(entry->first)); + block.push_back(L'='); + block.append(to_wide(entry->second)); + block.push_back(L'\0'); + } + + block.push_back(L'\0'); + return block; +} + +} // namespace + +#else + namespace { -// Quote a single argument for the inverse of CommandLineToArgvW, so that the -// child reconstructs the exact same argument vector -auto append_quoted_argument(std::string &command_line, - const std::string_view argument) -> void { - const bool needs_quoting{argument.empty() || - argument.find_first_of(" \t\"") != - std::string_view::npos}; +class Descriptor { +public: + Descriptor() = default; + explicit Descriptor(const int value) : value_{value} {} + ~Descriptor() { this->close(); } + Descriptor(const Descriptor &) = delete; + auto operator=(const Descriptor &) -> Descriptor & = delete; + Descriptor(Descriptor &&other) noexcept : value_{other.value_} { + other.value_ = -1; + } + auto operator=(Descriptor &&other) noexcept -> Descriptor & { + if (this != &other) { + this->close(); + this->value_ = other.value_; + other.value_ = -1; + } + + return *this; + } + + [[nodiscard]] auto get() const noexcept -> int { return this->value_; } + + [[nodiscard]] auto valid() const noexcept -> bool { + return this->value_ != -1; + } + + auto close() noexcept -> void { + if (this->value_ != -1) { + ::close(this->value_); + this->value_ = -1; + } + } + +private: + int value_{-1}; +}; + +// Writing to a pipe whose read end is gone raises SIGPIPE, whose default +// disposition terminates the process before the write can report EPIPE. A +// library cannot install a process-wide disposition, so the signal is blocked +// for the calling thread alone and any instance raised meanwhile is consumed +// before the previous mask is restored +class SignalGuard { +public: + SignalGuard() { + sigset_t blocked; + sigemptyset(&blocked); + sigaddset(&blocked, SIGPIPE); + this->active_ = pthread_sigmask(SIG_BLOCK, &blocked, &this->previous_) == 0; + } + + ~SignalGuard() { + if (!this->active_) { + return; + } + + // Only consume it if the caller was not already blocking it, as otherwise + // the instance belongs to whoever established that mask + if (!sigismember(&this->previous_, SIGPIPE)) { + sigset_t pending; + sigemptyset(&pending); + if (sigpending(&pending) == 0 && sigismember(&pending, SIGPIPE)) { + sigset_t target; + sigemptyset(&target); + sigaddset(&target, SIGPIPE); + int signal_number{0}; + sigwait(&target, &signal_number); + } + } + + pthread_sigmask(SIG_SETMASK, &this->previous_, nullptr); + } + + SignalGuard(const SignalGuard &) = delete; + auto operator=(const SignalGuard &) -> SignalGuard & = delete; + SignalGuard(SignalGuard &&) = delete; + auto operator=(SignalGuard &&) -> SignalGuard & = delete; + +private: + sigset_t previous_{}; + bool active_{false}; +}; - if (!needs_quoting) { - command_line.append(argument); +auto set_close_on_exec(const int descriptor) -> bool { + const int flags{fcntl(descriptor, F_GETFD)}; + return flags != -1 && fcntl(descriptor, F_SETFD, flags | FD_CLOEXEC) != -1; +} + +auto set_non_blocking(const int descriptor) -> bool { + const int flags{fcntl(descriptor, F_GETFL)}; + return flags != -1 && fcntl(descriptor, F_SETFL, flags | O_NONBLOCK) != -1; +} + +// Both ends are marked close-on-exec so that a spawn running concurrently on +// another thread cannot leak them into its own child. The descriptors this call +// hands to its child are duplicated onto the standard ones, and duplication +// clears that flag on the copy +auto make_pipe(Descriptor &read_end, Descriptor &write_end) -> bool { + std::array descriptors{}; + if (::pipe(descriptors.data()) != 0) { + return false; + } + + read_end = Descriptor{descriptors[0]}; + write_end = Descriptor{descriptors[1]}; + return set_close_on_exec(read_end.get()) && + set_close_on_exec(write_end.get()); +} + +// On every platform this builds for, a would-block error shares its value with +// EAGAIN +auto is_retryable_error() -> bool { return errno == EINTR || errno == EAGAIN; } + +// Returns whether the stream is still open +auto drain_stream(Descriptor &descriptor, std::string &destination) -> bool { + std::array buffer{}; + const auto count{::read(descriptor.get(), buffer.data(), buffer.size())}; + if (count > 0) { + destination.append(buffer.data(), static_cast(count)); + return true; + } else if (count == -1 && is_retryable_error()) { + return true; + } + + descriptor.close(); + return false; +} + +auto write_stream(Descriptor &descriptor, const std::string_view input, + std::size_t &offset) -> void { + const auto count{ + ::write(descriptor.get(), input.data() + offset, input.size() - offset)}; + if (count > 0) { + offset += static_cast(count); + if (offset >= input.size()) { + descriptor.close(); + } + + return; + } else if (count == -1 && is_retryable_error()) { return; } - command_line.push_back('"'); + // The program is not interested in the rest of its input, such as when it + // exited before consuming it + descriptor.close(); +} - for (auto cursor = argument.cbegin();; ++cursor) { - std::size_t backslash_count{0}; - while (cursor != argument.cend() && *cursor == '\\') { - ++cursor; - ++backslash_count; +auto transfer(Descriptor &input_descriptor, Descriptor &output_descriptor, + Descriptor &error_descriptor, const std::string_view input, + std::string &output, std::string &error) -> bool { + const SignalGuard guard; + std::size_t offset{0}; + + while (input_descriptor.valid() || output_descriptor.valid() || + error_descriptor.valid()) { + std::array descriptors{}; + std::size_t count{0}; + std::size_t input_index{3}; + std::size_t output_index{3}; + std::size_t error_index{3}; + + if (input_descriptor.valid()) { + descriptors[count].fd = input_descriptor.get(); + descriptors[count].events = POLLOUT; + input_index = count; + count += 1; } - if (cursor == argument.cend()) { - command_line.append(backslash_count * 2, '\\'); - break; - } else if (*cursor == '"') { - command_line.append(backslash_count * 2 + 1, '\\'); - command_line.push_back('"'); - } else { - command_line.append(backslash_count, '\\'); - command_line.push_back(*cursor); + if (output_descriptor.valid()) { + descriptors[count].fd = output_descriptor.get(); + descriptors[count].events = POLLIN; + output_index = count; + count += 1; + } + + if (error_descriptor.valid()) { + descriptors[count].fd = error_descriptor.get(); + descriptors[count].events = POLLIN; + error_index = count; + count += 1; + } + + if (poll(descriptors.data(), static_cast(count), -1) == -1) { + if (errno == EINTR) { + continue; + } + + return false; + } + + if (input_index < count) { + const auto events{descriptors[input_index].revents}; + if ((events & (POLLERR | POLLHUP | POLLNVAL)) != 0) { + input_descriptor.close(); + } else if ((events & POLLOUT) != 0) { + write_stream(input_descriptor, input, offset); + } + } + + // A hangup still delivers whatever the pipe holds, so the stream is only + // considered finished once a read reports the end of it + if (output_index < count && (descriptors[output_index].revents & + (POLLIN | POLLHUP | POLLERR)) != 0) { + drain_stream(output_descriptor, output); + } + + if (error_index < count && (descriptors[error_index].revents & + (POLLIN | POLLHUP | POLLERR)) != 0) { + drain_stream(error_descriptor, error); } } - command_line.push_back('"'); + return true; } } // namespace + #endif -namespace sourcemeta::core { +namespace { -auto spawn(const std::string &program, - std::span arguments, - const std::filesystem::path &directory) -> int { - assert(directory.is_absolute()); - assert(std::filesystem::exists(directory)); - assert(std::filesystem::is_directory(directory)); +// The two entry points differ only in what they pipe. Capturing always replaces +// the output streams, and always replaces the input stream so that a program +// never reaches back to whatever the caller had on its own standard input. +// Without capturing, a stream is only replaced when there is input to deliver +auto execute(const std::string &program, + std::span arguments, + const sourcemeta::core::ProcessInput &input, const bool capture) + -> sourcemeta::core::ProcessOutput { + using sourcemeta::core::ProcessProgramNotFoundError; + using sourcemeta::core::ProcessSpawnError; + + assert(input.directory.is_absolute()); + assert(std::filesystem::exists(input.directory)); + assert(std::filesystem::is_directory(input.directory)); + + const bool input_piped{capture || !input.standard_input.empty()}; + const bool output_piped{capture}; + + sourcemeta::core::ProcessOutput result; #if defined(_WIN32) && !defined(__MSYS__) && !defined(__CYGWIN__) && \ !defined(__MINGW32__) && !defined(__MINGW64__) + Handle input_read; + Handle input_write; + Handle output_read; + Handle output_write; + Handle error_read; + Handle error_write; + if (input_piped && !make_pipe(input_read, input_write, true)) { + throw ProcessSpawnError{program, arguments}; + } + + if (output_piped && (!make_pipe(output_read, output_write, false) || + !make_pipe(error_read, error_write, false))) { + throw ProcessSpawnError{program, arguments}; + } + std::string command_line; append_quoted_argument(command_line, program); - for (const auto &argument : arguments) { command_line.push_back(' '); append_quoted_argument(command_line, argument); } - std::vector cmd_line(command_line.begin(), command_line.end()); - cmd_line.push_back('\0'); + std::wstring wide_command_line{to_wide(command_line)}; + wide_command_line.push_back(L'\0'); + const std::wstring working_directory{to_wide(input.directory.string())}; + std::wstring environment_block; + if (input.environment.has_value()) { + environment_block = to_environment_block(input.environment.value()); + } - STARTUPINFOA startup_info{}; + STARTUPINFOW startup_info{}; startup_info.cb = sizeof(startup_info); + // Naming any handle means naming all three, so the ones left alone are filled + // with whatever the caller already has + if (input_piped || output_piped) { + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = + input_piped ? input_read.get() : GetStdHandle(STD_INPUT_HANDLE); + startup_info.hStdOutput = + output_piped ? output_write.get() : GetStdHandle(STD_OUTPUT_HANDLE); + startup_info.hStdError = + output_piped ? error_write.get() : GetStdHandle(STD_ERROR_HANDLE); + } + PROCESS_INFORMATION process_info{}; - const std::string working_dir = directory.string(); - const BOOL success = - CreateProcessA(nullptr, // lpApplicationName - cmd_line.data(), // lpCommandLine (modifiable) - nullptr, // lpProcessAttributes - nullptr, // lpThreadAttributes - TRUE, // bInheritHandles - 0, // dwCreationFlags - nullptr, // lpEnvironment - working_dir.c_str(), // lpCurrentDirectory - &startup_info, // lpStartupInfo - &process_info // lpProcessInformation - ); + const DWORD flags{input.environment.has_value() + ? static_cast(CREATE_UNICODE_ENVIRONMENT) + : static_cast(0)}; + const BOOL success{CreateProcessW( + nullptr, wide_command_line.data(), nullptr, nullptr, TRUE, flags, + input.environment.has_value() ? environment_block.data() : nullptr, + working_directory.c_str(), &startup_info, &process_info)}; if (!success) { const DWORD error_code{GetLastError()}; @@ -118,24 +480,76 @@ auto spawn(const std::string &program, throw ProcessSpawnError{program, arguments}; } - if (WaitForSingleObject(process_info.hProcess, INFINITE) == WAIT_FAILED) { - CloseHandle(process_info.hProcess); - CloseHandle(process_info.hThread); - throw ProcessSpawnError{program, arguments}; + const Handle process{process_info.hProcess}; + const Handle process_thread{process_info.hThread}; + + // The parent must let go of the ends it handed over, as otherwise the reads + // below never see the end of either stream + input_read.close(); + output_write.close(); + error_write.close(); + + // An anonymous pipe cannot be waited on, so each output stream is drained by + // a thread of its own while this one feeds the input. Otherwise a program + // that fills one pipe blocks forever against a parent blocked on writing + std::thread output_reader; + std::thread error_reader; + if (output_piped) { + output_reader = std::thread{read_handle_to_string, output_read.get(), + std::ref(result.standard_output)}; + error_reader = std::thread{read_handle_to_string, error_read.get(), + std::ref(result.standard_error)}; } - DWORD exit_code; - if (!GetExitCodeProcess(process_info.hProcess, &exit_code)) { - CloseHandle(process_info.hProcess); - CloseHandle(process_info.hThread); + std::size_t offset{0}; + while (input_piped && offset < input.standard_input.size()) { + DWORD written{0}; + if (!WriteFile(input_write.get(), input.standard_input.data() + offset, + static_cast(input.standard_input.size() - offset), + &written, nullptr) || + written == 0) { + break; + } + + offset += written; + } + + input_write.close(); + if (output_piped) { + output_reader.join(); + error_reader.join(); + } + + if (WaitForSingleObject(process.get(), INFINITE) == WAIT_FAILED) { throw ProcessSpawnError{program, arguments}; } - CloseHandle(process_info.hProcess); - CloseHandle(process_info.hThread); + DWORD exit_code{0}; + if (!GetExitCodeProcess(process.get(), &exit_code)) { + throw ProcessSpawnError{program, arguments}; + } - return static_cast(exit_code); + result.exit_code = static_cast(exit_code); + return result; #else + Descriptor input_read; + Descriptor input_write; + Descriptor output_read; + Descriptor output_write; + Descriptor error_read; + Descriptor error_write; + if (input_piped && (!make_pipe(input_read, input_write) || + !set_non_blocking(input_write.get()))) { + throw ProcessSpawnError{program, arguments}; + } + + if (output_piped && (!make_pipe(output_read, output_write) || + !make_pipe(error_read, error_write) || + !set_non_blocking(output_read.get()) || + !set_non_blocking(error_read.get()))) { + throw ProcessSpawnError{program, arguments}; + } + std::vector owned_arguments; owned_arguments.reserve(arguments.size()); for (const auto &argument : arguments) { @@ -145,19 +559,54 @@ auto spawn(const std::string &program, std::vector argv; argv.reserve(owned_arguments.size() + 2); argv.push_back(program.c_str()); - for (const auto &argument : owned_arguments) { argv.push_back(argument.c_str()); } argv.push_back(nullptr); + std::vector owned_environment; + std::vector envp; + if (input.environment.has_value()) { + owned_environment.reserve(input.environment.value().size()); + for (const auto &entry : input.environment.value()) { + std::string variable; + variable.reserve(entry.first.size() + entry.second.size() + 1); + variable.append(entry.first); + variable.push_back('='); + variable.append(entry.second); + owned_environment.emplace_back(std::move(variable)); + } + + envp.reserve(owned_environment.size() + 1); + for (auto &entry : owned_environment) { + envp.push_back(entry.data()); + } + + envp.push_back(nullptr); + } + posix_spawnattr_t attributes; posix_spawnattr_init(&attributes); posix_spawn_file_actions_t file_actions; posix_spawn_file_actions_init(&file_actions); + const bool wiring{ + (!input_piped || + posix_spawn_file_actions_adddup2(&file_actions, input_read.get(), + STDIN_FILENO) == 0) && + (!output_piped || + (posix_spawn_file_actions_adddup2(&file_actions, output_write.get(), + STDOUT_FILENO) == 0 && + posix_spawn_file_actions_adddup2(&file_actions, error_write.get(), + STDERR_FILENO) == 0))}; + if (!wiring) { + posix_spawn_file_actions_destroy(&file_actions); + posix_spawnattr_destroy(&attributes); + throw ProcessSpawnError{program, arguments}; + } + #if defined(__MSYS__) || defined(__CYGWIN__) || defined(__MINGW32__) || \ defined(__MINGW64__) // These platforms lack a child-directory file action, so we change the @@ -166,15 +615,15 @@ auto spawn(const std::string &program, // directory while the spawn is in flight const std::filesystem::path original_directory{ std::filesystem::current_path()}; - std::filesystem::current_path(directory); + std::filesystem::current_path(input.directory); #else // The standardized child-directory file action is not yet provided by every // system and toolchain this builds on, so we keep using the long-standing // platform extension and silence the deprecation that newer SDKs attach to it #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" - const int addchdir_result{ - posix_spawn_file_actions_addchdir_np(&file_actions, directory.c_str())}; + const int addchdir_result{posix_spawn_file_actions_addchdir_np( + &file_actions, input.directory.c_str())}; #pragma GCC diagnostic pop if (addchdir_result != 0) { posix_spawn_file_actions_destroy(&file_actions); @@ -186,7 +635,8 @@ auto spawn(const std::string &program, pid_t process_id; const int spawn_result{ posix_spawnp(&process_id, program.c_str(), &file_actions, &attributes, - const_cast(argv.data()), environ)}; + const_cast(argv.data()), + input.environment.has_value() ? envp.data() : environ)}; posix_spawn_file_actions_destroy(&file_actions); posix_spawnattr_destroy(&attributes); @@ -204,6 +654,20 @@ auto spawn(const std::string &program, throw ProcessSpawnError{program, arguments}; } + // The parent must let go of the ends it handed over, as otherwise the reads + // below never see the end of either stream + input_read.close(); + output_write.close(); + error_write.close(); + + if (input.standard_input.empty()) { + input_write.close(); + } + + const bool transferred{transfer(input_write, output_read, error_read, + input.standard_input, result.standard_output, + result.standard_error)}; + int status{0}; while (waitpid(process_id, &status, 0) == -1) { if (errno == EINTR) { @@ -213,21 +677,55 @@ auto spawn(const std::string &program, throw ProcessSpawnError{program, arguments}; } + if (!transferred) { + throw ProcessSpawnError{program, arguments}; + } + if (WIFEXITED(status)) { - return WEXITSTATUS(status); + result.exit_code = WEXITSTATUS(status); } - throw ProcessSpawnError{program, arguments}; + return result; #endif } +} // namespace + +namespace sourcemeta::core { + +auto spawn(const std::string &program, + std::span arguments, + const ProcessInput &input) -> int { + const auto result{execute(program, arguments, input, false)}; + if (!result.exit_code.has_value()) { + throw ProcessSpawnError{program, arguments}; + } + + return result.exit_code.value(); +} + auto spawn(const std::string &program, std::initializer_list arguments, - const std::filesystem::path &directory) -> int { + const ProcessInput &input) -> int { return spawn( program, std::span{arguments.begin(), arguments.size()}, - directory); + input); +} + +auto spawn_and_capture(const std::string &program, + std::span arguments, + const ProcessInput &input) -> ProcessOutput { + return execute(program, arguments, input, true); +} + +auto spawn_and_capture(const std::string &program, + std::initializer_list arguments, + const ProcessInput &input) -> ProcessOutput { + return spawn_and_capture( + program, + std::span{arguments.begin(), arguments.size()}, + input); } } // namespace sourcemeta::core diff --git a/test/process/CMakeLists.txt b/test/process/CMakeLists.txt index 432d0eda9a..5ee7b8cb2e 100644 --- a/test/process/CMakeLists.txt +++ b/test/process/CMakeLists.txt @@ -4,13 +4,15 @@ if(WIN32) "${CMAKE_CURRENT_SOURCE_DIR}/process_spawn_test.ps1" "$") sourcemeta_test(NAMESPACE sourcemeta PROJECT core NAME process - SOURCES process_error_test.cc process_spawn_test_windows.cc) + SOURCES process_error_test.cc process_spawn_test_windows.cc + process_spawn_and_capture_test.cc process_spawn_input_test.cc) else() add_test(NAME core.process.spawn.e2e COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/process_spawn_test.sh" "$") sourcemeta_test(NAMESPACE sourcemeta PROJECT core NAME process - SOURCES process_error_test.cc process_spawn_test_unix.cc) + SOURCES process_error_test.cc process_spawn_test_unix.cc + process_spawn_and_capture_test.cc process_spawn_input_test.cc) endif() target_link_libraries(sourcemeta_core_process_unit PRIVATE sourcemeta::core::process) @@ -18,3 +20,12 @@ target_link_libraries(sourcemeta_core_process_unit PRIVATE sourcemeta::core::pro add_executable(sourcemeta_core_process_unit_spawn_main process_spawn_main.cc) target_link_libraries(sourcemeta_core_process_unit_spawn_main PRIVATE sourcemeta::core::process) + +add_executable(sourcemeta_core_process_unit_capture_main + process_spawn_and_capture_main.cc) +sourcemeta_add_default_options(PRIVATE sourcemeta_core_process_unit_capture_main) + +add_dependencies(sourcemeta_core_process_unit + sourcemeta_core_process_unit_capture_main) +target_compile_definitions(sourcemeta_core_process_unit + PRIVATE HELPER_BINARY="$") diff --git a/test/process/process_spawn_and_capture_main.cc b/test/process/process_spawn_and_capture_main.cc new file mode 100644 index 0000000000..d983a51d03 --- /dev/null +++ b/test/process/process_spawn_and_capture_main.cc @@ -0,0 +1,193 @@ +#include // std::size_t +#include // std::fread, std::fwrite, std::fflush, std::FILE +#include // std::atoi, std::atoll, std::getenv +#include // std::filesystem::current_path +#include // std::string, std::to_string +#include // std::string_view + +#if defined(_WIN32) +#include // _O_BINARY +#include // _setmode, _fileno +#include // _environ +#else +#include // std::raise, SIGKILL +extern char **environ; +#endif + +namespace { + +auto environment_entries() -> char ** { +#if defined(_WIN32) + return _environ; +#else + return environ; +#endif +} + +auto write_all(std::FILE *stream, const std::string_view payload) -> void { + if (!payload.empty()) { + std::fwrite(payload.data(), 1, payload.size(), stream); + } + + std::fflush(stream); +} + +auto read_all(std::FILE *stream) -> std::string { + std::string result; + char buffer[4096]; + std::size_t count{0}; + while ((count = std::fread(buffer, 1, sizeof(buffer), stream)) > 0) { + result.append(buffer, count); + } + + return result; +} + +// A payload whose bytes vary, so that a truncated or reordered transfer cannot +// pass by accident +auto payload_of(const std::size_t size) -> std::string { + std::string result; + result.reserve(size); + for (std::size_t index = 0; index < size; ++index) { + result.push_back(static_cast('a' + (index % 26))); + } + + return result; +} + +auto to_size(const char *value) -> std::size_t { + return static_cast(std::atoll(value)); +} + +} // namespace + +auto main(int argc, char *argv[]) -> int { +#if defined(_WIN32) + // Otherwise the C runtime rewrites every line feed on the way out, and the + // captured bytes stop matching what the program wrote + _setmode(_fileno(stdin), _O_BINARY); + _setmode(_fileno(stdout), _O_BINARY); + _setmode(_fileno(stderr), _O_BINARY); +#endif + + if (argc < 2) { + return 2; + } + + const std::string_view command{argv[1]}; + + if (command == "exit") { + return std::atoi(argv[2]); + } + + if (command == "stdout") { + write_all(stdout, argv[2]); + return 0; + } + + if (command == "stderr") { + write_all(stderr, argv[2]); + return 0; + } + + if (command == "both") { + write_all(stdout, argv[2]); + write_all(stderr, argv[3]); + return 0; + } + + if (command == "silent") { + return 0; + } + + if (command == "cat") { + write_all(stdout, read_all(stdin)); + return 0; + } + + if (command == "count-stdin") { + write_all(stdout, std::to_string(read_all(stdin).size())); + return 0; + } + + // Fills a stream before touching the input, which deadlocks any parent that + // writes the whole input before reading anything + if (command == "flood-stderr-then-cat") { + write_all(stderr, payload_of(to_size(argv[2]))); + write_all(stdout, read_all(stdin)); + return 0; + } + + if (command == "flood-both") { + const auto payload{payload_of(to_size(argv[2]))}; + write_all(stdout, payload); + write_all(stderr, payload); + return 0; + } + + // Exits without reading anything, so a parent still writing its input meets a + // pipe with no reader + if (command == "ignore-stdin") { + write_all(stdout, "done"); + return 0; + } + + if (command == "environment") { + std::string result; + for (char **entry = environment_entries(); entry != nullptr && *entry; + ++entry) { + result.append(*entry); + result.push_back('\n'); + } + + write_all(stdout, result); + return 0; + } + + // The commands below report through the exit code alone, so that a caller + // that does not capture anything can still assert on what the program saw + if (command == "expect-stdin") { + return read_all(stdin) == argv[2] ? 0 : 1; + } + + // Checks a payload too large to hand over as an argument + if (command == "expect-stdin-size") { + return read_all(stdin).size() == to_size(argv[2]) ? 0 : 1; + } + + if (command == "expect-environment") { + const char *value{std::getenv(argv[2])}; + return value != nullptr && std::string_view{value} == argv[3] ? 0 : 1; + } + + if (command == "expect-no-environment") { + return std::getenv(argv[2]) == nullptr ? 0 : 1; + } + + if (command == "directory") { + write_all(stdout, std::filesystem::current_path().string()); + return 0; + } + + if (command == "arguments") { + std::string result; + for (int index = 2; index < argc; ++index) { + result.append(argv[index]); + result.push_back('\n'); + } + + write_all(stdout, result); + return 0; + } + +#if !defined(_WIN32) + // Writes first, so that a caller can tell whether output survives a program + // that never gets to exit + if (command == "terminate") { + write_all(stdout, "partial"); + std::raise(SIGKILL); + } +#endif + + return 2; +} diff --git a/test/process/process_spawn_and_capture_test.cc b/test/process/process_spawn_and_capture_test.cc new file mode 100644 index 0000000000..dcddde8e61 --- /dev/null +++ b/test/process/process_spawn_and_capture_test.cc @@ -0,0 +1,330 @@ +#include +#include + +#include // std::size_t +#include // std::filesystem::path +#include // std::map +#include // std::string +#include // std::string_view +#include // std::thread +#include // std::vector + +static const std::string HELPER{HELPER_BINARY}; + +// Enough to overrun the pipe buffer of every platform this builds on, so that a +// transfer that does not interleave reading and writing cannot complete +static constexpr std::size_t FLOOD_SIZE{1024 * 1024}; + +// Mirrors what the helper program writes, so that a truncated or reordered +// transfer cannot pass by accident +static auto payload_of(const std::size_t size) -> std::string { + std::string result; + result.reserve(size); + for (std::size_t index = 0; index < size; ++index) { + result.push_back(static_cast('a' + (index % 26))); + } + + return result; +} + +TEST(exit_code_zero_is_reported) { + const auto result{sourcemeta::core::spawn_and_capture(HELPER, {"exit", "0"})}; + EXPECT_TRUE(result.exit_code.has_value()); + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, ""); + EXPECT_EQ(result.standard_error, ""); +} + +TEST(exit_code_non_zero_is_reported) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"exit", "42"})}; + EXPECT_TRUE(result.exit_code.has_value()); + EXPECT_EQ(result.exit_code.value(), 42); +} + +TEST(exit_code_one_is_reported) { + const auto result{sourcemeta::core::spawn_and_capture(HELPER, {"exit", "1"})}; + EXPECT_TRUE(result.exit_code.has_value()); + EXPECT_EQ(result.exit_code.value(), 1); +} + +TEST(standard_output_is_captured) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"stdout", "hello"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "hello"); + EXPECT_EQ(result.standard_error, ""); +} + +TEST(standard_error_is_captured) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"stderr", "problem"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, ""); + EXPECT_EQ(result.standard_error, "problem"); +} + +TEST(both_streams_are_captured_separately) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"both", "out", "err"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "out"); + EXPECT_EQ(result.standard_error, "err"); +} + +TEST(a_silent_program_captures_nothing) { + const auto result{sourcemeta::core::spawn_and_capture(HELPER, {"silent"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, ""); + EXPECT_EQ(result.standard_error, ""); + EXPECT_TRUE(result.standard_output.empty()); + EXPECT_TRUE(result.standard_error.empty()); +} + +TEST(output_without_a_trailing_newline_is_preserved) { + const auto result{sourcemeta::core::spawn_and_capture( + HELPER, {"stdout", "no newline here"})}; + EXPECT_EQ(result.standard_output, "no newline here"); + EXPECT_EQ(result.standard_output.size(), 15); +} + +TEST(line_endings_in_output_are_not_rewritten) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"stdout", "one\r\ntwo\n"})}; + EXPECT_EQ(result.standard_output, "one\r\ntwo\n"); + EXPECT_EQ(result.standard_output.size(), 9); +} + +TEST(arguments_are_passed_in_order) { + const auto result{sourcemeta::core::spawn_and_capture( + HELPER, {"arguments", "alpha", "beta", "gamma"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "alpha\nbeta\ngamma\n"); +} + +TEST(an_argument_with_spaces_stays_a_single_argument) { + const auto result{sourcemeta::core::spawn_and_capture( + HELPER, {"arguments", "alpha beta gamma"})}; + EXPECT_EQ(result.standard_output, "alpha beta gamma\n"); +} + +TEST(an_argument_with_quotes_is_preserved) { + const auto result{sourcemeta::core::spawn_and_capture( + HELPER, {"arguments", "say \"hello\"", "back\\slash"})}; + EXPECT_EQ(result.standard_output, "say \"hello\"\nback\\slash\n"); +} + +TEST(an_empty_argument_is_preserved) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"arguments", "", "after"})}; + EXPECT_EQ(result.standard_output, "\nafter\n"); +} + +TEST(the_span_overload_accepts_a_dynamic_argument_list) { + const std::vector arguments{"arguments", "one", "two"}; + const auto result{sourcemeta::core::spawn_and_capture(HELPER, arguments)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "one\ntwo\n"); +} + +TEST(standard_input_is_delivered) { + const sourcemeta::core::ProcessInput input{.standard_input = "hello input"}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "hello input"); +} + +TEST(an_absent_standard_input_is_an_immediate_end_of_input) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"count-stdin"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "0"); +} + +TEST(an_empty_standard_input_is_an_immediate_end_of_input) { + const sourcemeta::core::ProcessInput input{.standard_input = ""}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"count-stdin"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "0"); +} + +TEST(standard_input_carrying_null_bytes_round_trips) { + const std::string payload{std::string{"before"} + std::string(1, '\0') + + std::string{"after"}}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output.size(), 12); + EXPECT_EQ(result.standard_output, payload); +} + +TEST(a_large_standard_input_round_trips) { + const auto payload{payload_of(FLOOD_SIZE)}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output.size(), FLOOD_SIZE); + EXPECT_EQ(result.standard_output, payload); +} + +TEST(a_large_standard_error_does_not_deadlock_a_large_standard_input) { + const auto payload{payload_of(FLOOD_SIZE)}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const auto result{sourcemeta::core::spawn_and_capture( + HELPER, {"flood-stderr-then-cat", "1048576"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_error.size(), FLOOD_SIZE); + EXPECT_EQ(result.standard_error, payload); + EXPECT_EQ(result.standard_output.size(), FLOOD_SIZE); + EXPECT_EQ(result.standard_output, payload); +} + +TEST(both_streams_flooding_at_once_does_not_deadlock) { + const auto payload{payload_of(FLOOD_SIZE)}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"flood-both", "1048576"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output.size(), FLOOD_SIZE); + EXPECT_EQ(result.standard_output, payload); + EXPECT_EQ(result.standard_error.size(), FLOOD_SIZE); + EXPECT_EQ(result.standard_error, payload); +} + +TEST(a_program_that_never_reads_its_input_does_not_kill_the_caller) { + const auto payload{payload_of(FLOOD_SIZE)}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"ignore-stdin"}, input)}; + EXPECT_TRUE(result.exit_code.has_value()); + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, "done"); + EXPECT_EQ(result.standard_error, ""); +} + +TEST(the_environment_is_inherited_by_default) { + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"environment"})}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_FALSE(result.standard_output.empty()); +} + +TEST(an_explicit_environment_is_delivered) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_ALPHA", "one"}, {"SOURCEMETA_BETA", "two"}}}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"environment"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_TRUE(result.standard_output.find("SOURCEMETA_ALPHA=one\n") != + std::string::npos); + EXPECT_TRUE(result.standard_output.find("SOURCEMETA_BETA=two\n") != + std::string::npos); +} + +TEST(an_explicit_environment_with_an_empty_value_is_delivered) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_EMPTY", ""}}}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"environment"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_TRUE(result.standard_output.find("SOURCEMETA_EMPTY=\n") != + std::string::npos); +} + +TEST(the_working_directory_is_honoured) { + const auto directory{ + std::filesystem::canonical(std::filesystem::temp_directory_path())}; + const sourcemeta::core::ProcessInput input{.directory = directory}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"directory"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, directory.string()); +} + +// Every descriptor the call opens is marked close-on-exec, so a program started +// on one thread never inherits the pipes of a program started on another. A +// leaked write end would keep a stream from ever reaching its end +TEST(concurrent_calls_do_not_leak_streams_into_each_other) { + const auto first_payload{payload_of(FLOOD_SIZE)}; + const std::string second_payload{"second"}; + const std::string third_payload{"third"}; + sourcemeta::core::ProcessOutput first; + sourcemeta::core::ProcessOutput second; + sourcemeta::core::ProcessOutput third; + + std::thread first_thread{[&first, &first_payload] { + const sourcemeta::core::ProcessInput input{.standard_input = first_payload}; + first = sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input); + }}; + std::thread second_thread{[&second, &second_payload] { + const sourcemeta::core::ProcessInput input{.standard_input = + second_payload}; + second = sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input); + }}; + std::thread third_thread{[&third, &third_payload] { + const sourcemeta::core::ProcessInput input{.standard_input = third_payload}; + third = sourcemeta::core::spawn_and_capture(HELPER, {"cat"}, input); + }}; + + first_thread.join(); + second_thread.join(); + third_thread.join(); + + EXPECT_EQ(first.exit_code.value(), 0); + EXPECT_EQ(first.standard_output.size(), FLOOD_SIZE); + EXPECT_EQ(first.standard_output, first_payload); + EXPECT_EQ(second.exit_code.value(), 0); + EXPECT_EQ(second.standard_output, second_payload); + EXPECT_EQ(third.exit_code.value(), 0); + EXPECT_EQ(third.standard_output, third_payload); +} + +TEST(a_missing_program_reports_the_program_it_could_not_find) { + const std::string program{"this_program_definitely_does_not_exist"}; + try { + const auto result{sourcemeta::core::spawn_and_capture(program, {})}; + FAIL(); + } catch (const sourcemeta::core::ProcessProgramNotFoundError &error) { + EXPECT_EQ(error.program(), program); + } +} + +#if !defined(_WIN32) + +TEST(a_program_killed_by_a_signal_reports_no_exit_code) { + const auto result{sourcemeta::core::spawn_and_capture(HELPER, {"terminate"})}; + EXPECT_FALSE(result.exit_code.has_value()); +} + +TEST(a_program_killed_by_a_signal_still_yields_its_output) { + const auto result{sourcemeta::core::spawn_and_capture(HELPER, {"terminate"})}; + EXPECT_EQ(result.standard_output, "partial"); + EXPECT_EQ(result.standard_error, ""); +} + +TEST(an_explicit_environment_replaces_rather_than_extends) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_ALPHA", "one"}, {"SOURCEMETA_BETA", "two"}}}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"environment"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, + "SOURCEMETA_ALPHA=one\nSOURCEMETA_BETA=two\n"); +} + +TEST(an_empty_explicit_environment_isolates_completely) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{}}; + const auto result{ + sourcemeta::core::spawn_and_capture(HELPER, {"environment"}, input)}; + EXPECT_EQ(result.exit_code.value(), 0); + EXPECT_EQ(result.standard_output, ""); +} + +#endif diff --git a/test/process/process_spawn_input_test.cc b/test/process/process_spawn_input_test.cc new file mode 100644 index 0000000000..da7ed58c80 --- /dev/null +++ b/test/process/process_spawn_input_test.cc @@ -0,0 +1,117 @@ +#include +#include + +#include // std::size_t +#include // std::filesystem::canonical, std::filesystem::temp_directory_path +#include // std::map +#include // std::string +#include // std::string_view + +static const std::string HELPER{HELPER_BINARY}; + +static auto payload_of(const std::size_t size) -> std::string { + std::string result; + result.reserve(size); + for (std::size_t index = 0; index < size; ++index) { + result.push_back(static_cast('a' + (index % 26))); + } + + return result; +} + +TEST(standard_input_is_delivered_without_capturing) { + const sourcemeta::core::ProcessInput input{.standard_input = "the payload"}; + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-stdin", "the payload"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +TEST(a_mismatched_standard_input_is_visible_to_the_program) { + const sourcemeta::core::ProcessInput input{.standard_input = + "something else"}; + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-stdin", "the payload"}, input)}; + EXPECT_EQ(exit_code, 1); +} + +TEST(an_absent_standard_input_leaves_the_stream_alone) { + const int exit_code{sourcemeta::core::spawn(HELPER, {"exit", "7"})}; + EXPECT_EQ(exit_code, 7); +} + +TEST(a_large_standard_input_is_delivered_without_capturing) { + const auto payload{payload_of(1024 * 1024)}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-stdin-size", "1048576"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +TEST(a_short_standard_input_is_measured_exactly_without_capturing) { + const sourcemeta::core::ProcessInput input{.standard_input = "12345"}; + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-stdin-size", "5"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +TEST(a_program_that_never_reads_a_large_input_does_not_kill_the_caller) { + const auto payload{payload_of(1024 * 1024)}; + const sourcemeta::core::ProcessInput input{.standard_input = payload}; + const int exit_code{sourcemeta::core::spawn(HELPER, {"exit", "3"}, input)}; + EXPECT_EQ(exit_code, 3); +} + +TEST(an_explicit_environment_is_delivered_without_capturing) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_ONE", "1"}}}; + const int exit_code{sourcemeta::core::spawn( + HELPER, {"expect-environment", "SOURCEMETA_ONE", "1"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +TEST(an_explicit_environment_with_a_different_value_is_visible) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_ONE", "2"}}}; + const int exit_code{sourcemeta::core::spawn( + HELPER, {"expect-environment", "SOURCEMETA_ONE", "1"}, input)}; + EXPECT_EQ(exit_code, 1); +} + +TEST(the_environment_is_inherited_by_default_without_capturing) { + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-no-environment", "PATH"})}; + EXPECT_EQ(exit_code, 1); +} + +TEST(the_working_directory_is_honoured_without_capturing) { + const auto directory{ + std::filesystem::canonical(std::filesystem::temp_directory_path())}; + const sourcemeta::core::ProcessInput input{.directory = directory}; + const int exit_code{sourcemeta::core::spawn(HELPER, {"silent"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +TEST(the_environment_and_the_standard_input_combine) { + const sourcemeta::core::ProcessInput input{ + .environment = + std::map{{"SOURCEMETA_ONE", "1"}}, + .standard_input = "ignored"}; + const int exit_code{sourcemeta::core::spawn( + HELPER, {"expect-environment", "SOURCEMETA_ONE", "1"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +#if !defined(_WIN32) + +TEST(an_explicit_environment_replaces_rather_than_extends_without_capturing) { + const sourcemeta::core::ProcessInput input{ + .environment = std::map{ + {"SOURCEMETA_ONE", "1"}}}; + const int exit_code{sourcemeta::core::spawn( + HELPER, {"expect-no-environment", "PATH"}, input)}; + EXPECT_EQ(exit_code, 0); +} + +#endif diff --git a/test/process/process_spawn_main.cc b/test/process/process_spawn_main.cc index edd97acb90..d4b87e6572 100644 --- a/test/process/process_spawn_main.cc +++ b/test/process/process_spawn_main.cc @@ -14,5 +14,5 @@ auto main(int argc, char *argv[]) -> int { arguments.emplace_back(argv[index]); } - return sourcemeta::core::spawn(program, arguments, directory); + return sourcemeta::core::spawn(program, arguments, {.directory = directory}); } diff --git a/test/process/process_spawn_test_unix.cc b/test/process/process_spawn_test_unix.cc index b6b8d5ac9d..a8759e69e9 100644 --- a/test/process/process_spawn_test_unix.cc +++ b/test/process/process_spawn_test_unix.cc @@ -69,8 +69,8 @@ TEST(empty_argument_is_preserved) { } TEST(pwd_with_custom_directory) { - const int exit_code{ - sourcemeta::core::spawn("pwd", {}, std::filesystem::path{"/tmp"})}; + const int exit_code{sourcemeta::core::spawn( + "pwd", {}, {.directory = std::filesystem::path{"/tmp"}})}; EXPECT_EQ(exit_code, 0); } diff --git a/test/process/process_spawn_test_windows.cc b/test/process/process_spawn_test_windows.cc index d0ec4aed1a..f22a38beaf 100644 --- a/test/process/process_spawn_test_windows.cc +++ b/test/process/process_spawn_test_windows.cc @@ -55,8 +55,8 @@ TEST(cmd_echo_with_arguments) { TEST(cmd_cd_with_custom_directory) { // Get the Windows temp directory const auto temp_dir = std::filesystem::temp_directory_path(); - const int exit_code{ - sourcemeta::core::spawn("cmd.exe", {"/c", "cd"}, temp_dir)}; + const int exit_code{sourcemeta::core::spawn("cmd.exe", {"/c", "cd"}, + {.directory = temp_dir})}; EXPECT_EQ(exit_code, 0); } From 936d3c9bb98dc39131569bdd4c21e5ef1659117d Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 13 Aug 2026 10:00:17 -0300 Subject: [PATCH 2/4] Fix Signed-off-by: Juan Cruz Viotti --- .../process/include/sourcemeta/core/process.h | 14 ++- src/lang/process/spawn.cc | 107 +++++++++++++++--- .../process/process_spawn_and_capture_main.cc | 28 ++++- test/process/process_spawn_input_test.cc | 12 +- 4 files changed, 136 insertions(+), 25 deletions(-) diff --git a/src/lang/process/include/sourcemeta/core/process.h b/src/lang/process/include/sourcemeta/core/process.h index 3b861e424f..ac79359285 100644 --- a/src/lang/process/include/sourcemeta/core/process.h +++ b/src/lang/process/include/sourcemeta/core/process.h @@ -30,6 +30,9 @@ namespace sourcemeta::core { /// @ingroup process /// The settings for running a program. +/// +/// Every member carries a default so that naming only the ones that matter +/// stays free of missing-initializer warnings. struct ProcessInput { /// The working directory of the program. It must be an absolute path to an /// existing directory @@ -38,10 +41,11 @@ struct ProcessInput { /// environment of the caller. Without a value, the caller's environment is /// inherited as it stands. The referenced names and values must outlive the /// call - std::optional> environment; + std::optional> environment{ + std::nullopt}; /// The bytes to feed the program on its standard input. The referenced buffer /// must outlive the call - std::string_view standard_input; + std::string_view standard_input{}; }; /// @ingroup process @@ -85,11 +89,11 @@ auto spawn(const std::string &program, struct ProcessOutput { /// The code the program exited with, or no value if it terminated abnormally, /// such as by a signal - std::optional exit_code; + std::optional exit_code{std::nullopt}; /// Everything the program wrote to its standard output - std::string standard_output; + std::string standard_output{}; /// Everything the program wrote to its standard error - std::string standard_error; + std::string standard_error{}; }; /// @ingroup process diff --git a/src/lang/process/spawn.cc b/src/lang/process/spawn.cc index d1dd11bb70..e42f2ea55e 100644 --- a/src/lang/process/spawn.cc +++ b/src/lang/process/spawn.cc @@ -128,6 +128,27 @@ auto make_pipe(Handle &read_end, Handle &write_end, const bool inherit_read) return SetHandleInformation(parent_end, HANDLE_FLAG_INHERIT, 0) != 0; } +// A standard handle the caller owns may be absent, as when there is no console, +// or may not be inheritable. Naming any handle means naming all three, so the +// ones this call does not replace are handed over as inheritable duplicates it +// owns and closes +auto inheritable_standard_handle(const DWORD stream, Handle &storage) + -> HANDLE { + const HANDLE original{GetStdHandle(stream)}; + if (original == nullptr || original == INVALID_HANDLE_VALUE) { + return INVALID_HANDLE_VALUE; + } + + HANDLE duplicate{nullptr}; + if (!DuplicateHandle(GetCurrentProcess(), original, GetCurrentProcess(), + &duplicate, 0, TRUE, DUPLICATE_SAME_ACCESS)) { + return INVALID_HANDLE_VALUE; + } + + storage = Handle{duplicate}; + return duplicate; +} + auto read_handle_to_string(HANDLE handle, std::string &destination) -> void { std::array buffer{}; DWORD count{0}; @@ -163,6 +184,12 @@ auto to_environment_block( block.push_back(L'\0'); } + // The block closes with an empty entry, so an environment with nothing in it + // is still two nulls rather than one + if (entries.empty()) { + block.push_back(L'\0'); + } + block.push_back(L'\0'); return block; } @@ -256,30 +283,71 @@ class SignalGuard { bool active_{false}; }; +#if !defined(__linux__) && !defined(__FreeBSD__) auto set_close_on_exec(const int descriptor) -> bool { const int flags{fcntl(descriptor, F_GETFD)}; return flags != -1 && fcntl(descriptor, F_SETFD, flags | FD_CLOEXEC) != -1; } +#endif auto set_non_blocking(const int descriptor) -> bool { const int flags{fcntl(descriptor, F_GETFL)}; return flags != -1 && fcntl(descriptor, F_SETFL, flags | O_NONBLOCK) != -1; } +// Duplicating a descriptor onto itself is defined to leave its flags alone, so +// an endpoint that landed on one of the standard descriptors, which happens +// when the caller closed one of its own, would keep close-on-exec and be shut +// rather than handed over. Moving it out of the way keeps every duplication a +// real one +auto relocate_above_standard(Descriptor &descriptor) -> bool { + if (descriptor.get() > STDERR_FILENO) { + return true; + } + + const int moved{fcntl(descriptor.get(), F_DUPFD_CLOEXEC, STDERR_FILENO + 1)}; + if (moved == -1) { + return false; + } + + descriptor = Descriptor{moved}; + return true; +} + // Both ends are marked close-on-exec so that a spawn running concurrently on // another thread cannot leak them into its own child. The descriptors this call // hands to its child are duplicated onto the standard ones, and duplication // clears that flag on the copy auto make_pipe(Descriptor &read_end, Descriptor &write_end) -> bool { std::array descriptors{}; + +#if defined(__linux__) || defined(__FreeBSD__) + // Setting the flag as part of the creation leaves no window in which another + // thread can spawn a program that inherits these + if (::pipe2(descriptors.data(), O_CLOEXEC) != 0) { + return false; + } + + read_end = Descriptor{descriptors[0]}; + write_end = Descriptor{descriptors[1]}; +#else + // Without an atomic creation the flag has to be set afterwards, which leaves + // a window that only matters to a program spawned by another thread in + // between if (::pipe(descriptors.data()) != 0) { return false; } read_end = Descriptor{descriptors[0]}; write_end = Descriptor{descriptors[1]}; - return set_close_on_exec(read_end.get()) && - set_close_on_exec(write_end.get()); + if (!set_close_on_exec(read_end.get()) || + !set_close_on_exec(write_end.get())) { + return false; + } +#endif + + return relocate_above_standard(read_end) && + relocate_above_standard(write_end); } // On every platform this builds for, a would-block error shares its value with @@ -393,6 +461,8 @@ auto transfer(Descriptor &input_descriptor, Descriptor &output_descriptor, #endif +namespace sourcemeta::core { + namespace { // The two entry points differ only in what they pipe. Capturing always replaces @@ -401,11 +471,7 @@ namespace { // Without capturing, a stream is only replaced when there is input to deliver auto execute(const std::string &program, std::span arguments, - const sourcemeta::core::ProcessInput &input, const bool capture) - -> sourcemeta::core::ProcessOutput { - using sourcemeta::core::ProcessProgramNotFoundError; - using sourcemeta::core::ProcessSpawnError; - + const ProcessInput &input, const bool capture) -> ProcessOutput { assert(input.directory.is_absolute()); assert(std::filesystem::exists(input.directory)); assert(std::filesystem::is_directory(input.directory)); @@ -447,18 +513,25 @@ auto execute(const std::string &program, environment_block = to_environment_block(input.environment.value()); } + Handle inherited_input; + Handle inherited_output; + Handle inherited_error; STARTUPINFOW startup_info{}; startup_info.cb = sizeof(startup_info); - // Naming any handle means naming all three, so the ones left alone are filled - // with whatever the caller already has if (input_piped || output_piped) { startup_info.dwFlags = STARTF_USESTDHANDLES; startup_info.hStdInput = - input_piped ? input_read.get() : GetStdHandle(STD_INPUT_HANDLE); + input_piped + ? input_read.get() + : inheritable_standard_handle(STD_INPUT_HANDLE, inherited_input); startup_info.hStdOutput = - output_piped ? output_write.get() : GetStdHandle(STD_OUTPUT_HANDLE); + output_piped + ? output_write.get() + : inheritable_standard_handle(STD_OUTPUT_HANDLE, inherited_output); startup_info.hStdError = - output_piped ? error_write.get() : GetStdHandle(STD_ERROR_HANDLE); + output_piped + ? error_write.get() + : inheritable_standard_handle(STD_ERROR_HANDLE, inherited_error); } PROCESS_INFORMATION process_info{}; @@ -503,10 +576,14 @@ auto execute(const std::string &program, std::size_t offset{0}; while (input_piped && offset < input.standard_input.size()) { + // Bounded so that an input of four gibibytes or more cannot wrap on its way + // into the smaller count this call takes + const std::size_t remaining{input.standard_input.size() - offset}; + const DWORD chunk{static_cast( + remaining < TRANSFER_BUFFER_SIZE ? remaining : TRANSFER_BUFFER_SIZE)}; DWORD written{0}; if (!WriteFile(input_write.get(), input.standard_input.data() + offset, - static_cast(input.standard_input.size() - offset), - &written, nullptr) || + chunk, &written, nullptr) || written == 0) { break; } @@ -691,8 +768,6 @@ auto execute(const std::string &program, } // namespace -namespace sourcemeta::core { - auto spawn(const std::string &program, std::span arguments, const ProcessInput &input) -> int { diff --git a/test/process/process_spawn_and_capture_main.cc b/test/process/process_spawn_and_capture_main.cc index d983a51d03..88ea7295f2 100644 --- a/test/process/process_spawn_and_capture_main.cc +++ b/test/process/process_spawn_and_capture_main.cc @@ -1,6 +1,6 @@ #include // std::size_t #include // std::fread, std::fwrite, std::fflush, std::FILE -#include // std::atoi, std::atoll, std::getenv +#include // std::atoi, std::atoll #include // std::filesystem::current_path #include // std::string, std::to_string #include // std::string_view @@ -24,6 +24,22 @@ auto environment_entries() -> char ** { #endif } +// Scanning the block avoids the lookup the Microsoft runtime deprecates, and +// keeps one way of reading the environment across every platform +auto find_environment(const std::string_view name) -> const char * { + for (char **entry = environment_entries(); entry != nullptr && *entry; + ++entry) { + const std::string_view current{*entry}; + const auto separator{current.find('=')}; + if (separator != std::string_view::npos && + current.substr(0, separator) == name) { + return *entry + separator + 1; + } + } + + return nullptr; +} + auto write_all(std::FILE *stream, const std::string_view payload) -> void { if (!payload.empty()) { std::fwrite(payload.data(), 1, payload.size(), stream); @@ -156,12 +172,18 @@ auto main(int argc, char *argv[]) -> int { } if (command == "expect-environment") { - const char *value{std::getenv(argv[2])}; + const char *value{find_environment(argv[2])}; return value != nullptr && std::string_view{value} == argv[3] ? 0 : 1; } if (command == "expect-no-environment") { - return std::getenv(argv[2]) == nullptr ? 0 : 1; + return find_environment(argv[2]) == nullptr ? 0 : 1; + } + + if (command == "expect-directory") { + return std::filesystem::current_path() == std::filesystem::path{argv[2]} + ? 0 + : 1; } if (command == "directory") { diff --git a/test/process/process_spawn_input_test.cc b/test/process/process_spawn_input_test.cc index da7ed58c80..7e7723ae68 100644 --- a/test/process/process_spawn_input_test.cc +++ b/test/process/process_spawn_input_test.cc @@ -89,10 +89,20 @@ TEST(the_working_directory_is_honoured_without_capturing) { const auto directory{ std::filesystem::canonical(std::filesystem::temp_directory_path())}; const sourcemeta::core::ProcessInput input{.directory = directory}; - const int exit_code{sourcemeta::core::spawn(HELPER, {"silent"}, input)}; + const int exit_code{sourcemeta::core::spawn( + HELPER, {"expect-directory", directory.string().c_str()}, input)}; EXPECT_EQ(exit_code, 0); } +TEST(a_program_reports_a_working_directory_that_does_not_match) { + const auto directory{ + std::filesystem::canonical(std::filesystem::temp_directory_path())}; + const sourcemeta::core::ProcessInput input{.directory = directory}; + const int exit_code{ + sourcemeta::core::spawn(HELPER, {"expect-directory", "/nowhere"}, input)}; + EXPECT_EQ(exit_code, 1); +} + TEST(the_environment_and_the_standard_input_combine) { const sourcemeta::core::ProcessInput input{ .environment = From 7499f892c51c9092445993630656f87fea1644f7 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 13 Aug 2026 10:44:08 -0300 Subject: [PATCH 3/4] More Signed-off-by: Juan Cruz Viotti --- src/lang/process/spawn.cc | 29 +++++++++++++------ test/process/CMakeLists.txt | 2 ++ .../process/process_spawn_and_capture_main.cc | 15 +++++++++- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/lang/process/spawn.cc b/src/lang/process/spawn.cc index e42f2ea55e..e9b7b8b978 100644 --- a/src/lang/process/spawn.cc +++ b/src/lang/process/spawn.cc @@ -26,7 +26,7 @@ #include // std::ref #include // std::map #include // std::thread -#include // CreateProcessW, CreatePipe, ReadFile, WriteFile, SetHandleInformation, STARTUPINFOW, PROCESS_INFORMATION, WaitForSingleObject, GetExitCodeProcess, MultiByteToWideChar, CloseHandle, WAIT_FAILED +#include // CreateProcessW, CreatePipe, CreateFileW, DuplicateHandle, GetStdHandle, ReadFile, WriteFile, SetHandleInformation, STARTUPINFOW, PROCESS_INFORMATION, WaitForSingleObject, GetExitCodeProcess, MultiByteToWideChar, CloseHandle, WAIT_FAILED #else #include // sigset_t, sigemptyset, sigaddset, sigismember, sigpending, sigwait, SIGPIPE, SIG_BLOCK, SIG_SETMASK #include // fcntl, FD_CLOEXEC, F_GETFD, F_SETFD, F_GETFL, F_SETFL, O_NONBLOCK @@ -131,22 +131,33 @@ auto make_pipe(Handle &read_end, Handle &write_end, const bool inherit_read) // A standard handle the caller owns may be absent, as when there is no console, // or may not be inheritable. Naming any handle means naming all three, so the // ones this call does not replace are handed over as inheritable duplicates it -// owns and closes +// owns and closes, falling back to the null device so that the program always +// receives something it can use auto inheritable_standard_handle(const DWORD stream, Handle &storage) -> HANDLE { const HANDLE original{GetStdHandle(stream)}; - if (original == nullptr || original == INVALID_HANDLE_VALUE) { - return INVALID_HANDLE_VALUE; + if (original != nullptr && original != INVALID_HANDLE_VALUE) { + HANDLE duplicate{nullptr}; + if (DuplicateHandle(GetCurrentProcess(), original, GetCurrentProcess(), + &duplicate, 0, TRUE, DUPLICATE_SAME_ACCESS)) { + storage = Handle{duplicate}; + return duplicate; + } } - HANDLE duplicate{nullptr}; - if (!DuplicateHandle(GetCurrentProcess(), original, GetCurrentProcess(), - &duplicate, 0, TRUE, DUPLICATE_SAME_ACCESS)) { + SECURITY_ATTRIBUTES attributes{}; + attributes.nLength = sizeof(attributes); + attributes.lpSecurityDescriptor = nullptr; + attributes.bInheritHandle = TRUE; + const HANDLE null_device{CreateFileW( + L"NUL", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + &attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)}; + if (null_device == INVALID_HANDLE_VALUE) { return INVALID_HANDLE_VALUE; } - storage = Handle{duplicate}; - return duplicate; + storage = Handle{null_device}; + return null_device; } auto read_handle_to_string(HANDLE handle, std::string &destination) -> void { diff --git a/test/process/CMakeLists.txt b/test/process/CMakeLists.txt index 5ee7b8cb2e..335280a3e9 100644 --- a/test/process/CMakeLists.txt +++ b/test/process/CMakeLists.txt @@ -24,6 +24,8 @@ target_link_libraries(sourcemeta_core_process_unit_spawn_main add_executable(sourcemeta_core_process_unit_capture_main process_spawn_and_capture_main.cc) sourcemeta_add_default_options(PRIVATE sourcemeta_core_process_unit_capture_main) +target_link_libraries(sourcemeta_core_process_unit_capture_main + PRIVATE sourcemeta::core::text) add_dependencies(sourcemeta_core_process_unit sourcemeta_core_process_unit_capture_main) diff --git a/test/process/process_spawn_and_capture_main.cc b/test/process/process_spawn_and_capture_main.cc index 88ea7295f2..0df133bdd8 100644 --- a/test/process/process_spawn_and_capture_main.cc +++ b/test/process/process_spawn_and_capture_main.cc @@ -1,3 +1,5 @@ +#include + #include // std::size_t #include // std::fread, std::fwrite, std::fflush, std::FILE #include // std::atoi, std::atoll @@ -24,6 +26,17 @@ auto environment_entries() -> char ** { #endif } +auto environment_name_matches(const std::string_view left, + const std::string_view right) -> bool { +#if defined(_WIN32) + // Windows compares environment variable names without regard to case, and + // spells the search path "Path" rather than "PATH" + return sourcemeta::core::equals_ignore_case(left, right); +#else + return left == right; +#endif +} + // Scanning the block avoids the lookup the Microsoft runtime deprecates, and // keeps one way of reading the environment across every platform auto find_environment(const std::string_view name) -> const char * { @@ -32,7 +45,7 @@ auto find_environment(const std::string_view name) -> const char * { const std::string_view current{*entry}; const auto separator{current.find('=')}; if (separator != std::string_view::npos && - current.substr(0, separator) == name) { + environment_name_matches(current.substr(0, separator), name)) { return *entry + separator + 1; } } From 4880185063a2daee283a5ad7e5c792c72345ac76 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 13 Aug 2026 11:05:44 -0300 Subject: [PATCH 4/4] Fix Signed-off-by: Juan Cruz Viotti --- config.cmake.in | 1 + src/lang/process/CMakeLists.txt | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/config.cmake.in b/config.cmake.in index 973d86cc8a..88329a8721 100644 --- a/config.cmake.in +++ b/config.cmake.in @@ -53,6 +53,7 @@ foreach(component ${SOURCEMETA_CORE_COMPONENTS}) elseif(component STREQUAL "io") include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_io.cmake") elseif(component STREQUAL "process") + include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_text.cmake") include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_process.cmake") elseif(component STREQUAL "parallel") find_dependency(Threads) diff --git a/src/lang/process/CMakeLists.txt b/src/lang/process/CMakeLists.txt index 93380e06ac..58ea5c1e0d 100644 --- a/src/lang/process/CMakeLists.txt +++ b/src/lang/process/CMakeLists.txt @@ -6,6 +6,4 @@ if(SOURCEMETA_CORE_INSTALL) sourcemeta_library_install(NAMESPACE sourcemeta PROJECT core NAME process) endif() -if(WIN32) - target_link_libraries(sourcemeta_core_process PRIVATE sourcemeta::core::text) -endif() +target_link_libraries(sourcemeta_core_process PRIVATE sourcemeta::core::text)