diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a203ae9..71b20a8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,7 +148,7 @@ jobs: strategy: fail-fast: false matrix: - config: [default, llvm, gnu32, sanitize, olddeps] + config: [default, llvm, gnu32, sanitize, olddeps, windows] name: build • ${{ matrix.config }} diff --git a/CMakeLists.txt b/CMakeLists.txt index bf50018a..f59b5b97 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -191,6 +191,7 @@ target_link_libraries(multiprocess PUBLIC CapnProto::capnp) target_link_libraries(multiprocess PUBLIC CapnProto::capnp-rpc) target_link_libraries(multiprocess PUBLIC CapnProto::kj) target_link_libraries(multiprocess PUBLIC CapnProto::kj-async) +target_link_libraries(multiprocess PUBLIC Threads::Threads) set_target_properties(multiprocess PROPERTIES PUBLIC_HEADER "${MP_PUBLIC_HEADERS}") install(TARGETS multiprocess EXPORT LibTargets diff --git a/ci/README.md b/ci/README.md index fef1c022..5297172d 100644 --- a/ci/README.md +++ b/ci/README.md @@ -21,6 +21,7 @@ CI_CONFIG=ci/configs/llvm.bash ci/scripts/run.sh CI_CONFIG=ci/configs/gnu32.bash ci/scripts/run.sh CI_CONFIG=ci/configs/sanitize.bash ci/scripts/run.sh CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh +CI_CONFIG=ci/configs/windows.bash ci/scripts/run.sh ``` By default CI jobs will reuse their build directories. `CI_CLEAN=1` can be specified to delete them before running instead. diff --git a/ci/configs/windows.bash b/ci/configs/windows.bash new file mode 100644 index 00000000..0bf73a83 --- /dev/null +++ b/ci/configs/windows.bash @@ -0,0 +1,21 @@ +CI_DESC="CI job cross-compiling to Windows with MinGW, tested with Wine" +CI_DIR=build-windows +CI_CACHE_NIX_STORE=true +NIX_ARGS=( + --arg windows true + --arg minimal true +) +CMAKE_ARGS=( + -G Ninja + -DCMAKE_SYSTEM_NAME=Windows + -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER + -DCMAKE_CROSSCOMPILING_EMULATOR=wine + # -Wa,-mbig-obj: template-heavy C++ generates >32K COFF sections per .obj; + # BigCOFF format raises the limit. Must be passed to the assembler via -Wa. + "-DCMAKE_CXX_FLAGS=-Wa,-mbig-obj" +) +# CXX is set by the nix cross shell to the mingw g++ wrapper; cmake picks +# it up from the environment, so no CMAKE_CXX_COMPILER override needed. +BUILD_ARGS=(-k 0) +# Build native mpgen first (as a code generator for cross-compiled test/example targets). +MPGEN_PRE_BUILD=1 diff --git a/ci/scripts/ci.sh b/ci/scripts/ci.sh index d989e9f4..715d1c3c 100755 --- a/ci/scripts/ci.sh +++ b/ci/scripts/ci.sh @@ -22,6 +22,49 @@ cmake_ver=$(cmake --version | awk '/version/{print $3; exit}') ver_ge() { [ "$(printf '%s\n' "$2" "$1" | sort -V | head -n1)" = "$2" ]; } src_dir=$PWD + +# If cross-compiling, build native mpgen first so it can be used as a code +# generator when cmake invokes it from add_custom_command (which does not go +# through CMAKE_CROSSCOMPILING_EMULATOR, unlike add_test executables). +if [ -n "${MPGEN_PRE_BUILD-}" ]; then + native_dir="${src_dir}/${CI_DIR}-native" + [ -n "${CI_CLEAN-}" ] && rm -rf "$native_dir" + mkdir -p "$native_dir" + # Unset cross-compilation env vars so cmake uses the native compiler and + # does a native (not cross) build. Key vars to clear: + # CXX/CC/AR/RANLIB/LD - set to cross-compiler by the cross shell + # cmakeFlags - nix cross shell injects -DCMAKE_SYSTEM_NAME=Windows etc. + # Pass NATIVE_CAPNPROTO_PREFIX as CMAKE_PREFIX_PATH so cmake finds the + # native Cap'n Proto rather than the cross-compiled one. + native_cmake_args=() + if [ -n "${NATIVE_CAPNPROTO_PREFIX-}" ]; then + # Build a cmake prefix path with native capnproto and its dependencies + # (openssl, zlib) so find_package and find_dependency succeed. + native_prefix="${NATIVE_CAPNPROTO_PREFIX}" + [ -n "${NATIVE_OPENSSL_DEV-}" ] && native_prefix="${native_prefix};${NATIVE_OPENSSL_DEV}" + [ -n "${NATIVE_OPENSSL_LIB-}" ] && native_prefix="${native_prefix};${NATIVE_OPENSSL_LIB}" + [ -n "${NATIVE_ZLIB_DEV-}" ] && native_prefix="${native_prefix};${NATIVE_ZLIB_DEV}" + [ -n "${NATIVE_ZLIB_LIB-}" ] && native_prefix="${native_prefix};${NATIVE_ZLIB_LIB}" + native_cmake_args+=( + "-DCMAKE_PREFIX_PATH=${native_prefix}" + "-DCapnProto_DIR=${NATIVE_CAPNPROTO_PREFIX}/lib/cmake/CapnProto" + ) + fi + # -static-libstdc++ / -static-libgcc: the native mpgen must run inside the + # cross nix shell where the native libstdc++.so may not be in LD_LIBRARY_PATH. + native_cmake_args+=("-DCMAKE_EXE_LINKER_FLAGS=-static-libstdc++ -static-libgcc") + (cd "$native_dir" && env -u CXX -u CC -u AR -u RANLIB -u LD -u cmakeFlags cmake "$src_dir" "${native_cmake_args[@]+${native_cmake_args[@]}}" && cmake --build . -t mpgen) + CMAKE_ARGS+=("-DMPGEN_EXECUTABLE=${native_dir}/mpgen") + + # Override capnp tool executables: the cross capnproto cmake config sets + # CAPNP_EXECUTABLE to capnp.exe (Windows binary), which can't run on Linux. + # Use the native capnp/capnpc-c++ binaries from pkgs.capnproto in nativeBuildInputs. + _capnp=$(command -v capnp 2>/dev/null || true) + _capnpc=$(command -v capnpc-c++ 2>/dev/null || true) + [ -n "$_capnp" ] && CMAKE_ARGS+=("-DCAPNP_EXECUTABLE=$_capnp") + [ -n "$_capnpc" ] && CMAKE_ARGS+=("-DCAPNPC_CXX_EXECUTABLE=$_capnpc") +fi + mkdir -p "$CI_DIR" cd "$CI_DIR" cmake "$src_dir" "${CMAKE_ARGS[@]+"${CMAKE_ARGS[@]}"}" @@ -34,4 +77,13 @@ else cmake --build . --target "$t" -- "${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}" done fi +# When cross-compiling for Windows, copy GCC and MCF thread runtime DLLs +# alongside the test executables so wine can find them (wine DLL loading +# checks the executable's directory first, before any search-path logic). +if [ -n "${WIN_RUNTIME_DLLS-}" ]; then + IFS=: read -ra _dll_dirs <<< "$WIN_RUNTIME_DLLS" + for _dir in "${_dll_dirs[@]}"; do + find "$_dir" -maxdepth 1 -name "*.dll" -exec cp -n {} test/ \; + done +fi ctest --output-on-failure diff --git a/example/example.cpp b/example/example.cpp index e4d159ab..9a128675 100644 --- a/example/example.cpp +++ b/example/example.cpp @@ -8,6 +8,7 @@ #include // IWYU pragma: keep #include #include +#include #include #include #include @@ -27,11 +28,11 @@ namespace fs = std::filesystem; static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name) { - const auto [pid, socket] = mp::SpawnProcess([&](mp::SpawnConnectInfo info) -> std::vector { + const auto [pid, socket] = mp::SpawnProcess([&](std::string connect_info) -> std::vector { fs::path path = process_argv0; path.remove_filename(); path.append(new_exe_name); - return {path.string(), std::move(info)}; + return {path.string(), std::move(connect_info)}; }); return std::make_tuple(mp::ConnectStream(loop, mp::MakeStream(loop, socket)), pid); } diff --git a/include/mp/util.h b/include/mp/util.h index eac655be..8e06e676 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -29,6 +29,17 @@ #include #endif +#ifdef WIN32 +// WIN32_LEAN_AND_MEAN excludes commdlg.h which defines `#define INTERFACE +// IPrintDialogServices` — this conflicts with capnp::Kind::INTERFACE used +// in CAPNP_DECLARE_INTERFACE_HEADER. Must be defined before winsock2.h. +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#include +#endif + namespace mp { //! Generic utility functions used by capnp code. @@ -286,33 +297,53 @@ std::string ThreadName(const char* exe_name); //! errors in python unit tests. std::string LogEscape(const kj::StringTree& string, size_t max_size); +//! Convert an argument vector into a single command line string suitable for +//! CreateProcess, following the quoting rules of CommandLineToArgvW, which +//! executables use to split the command line back into arguments. Declared +//! unconditionally (not just on windows) so it can be unit tested on any +//! platform. +std::string CommandLineFromArgv(const std::vector& argv); + using Stream = kj::Own; +#ifdef WIN32 +// On Windows, ProcessId is defined to be the local process HANDLE rather than +// global process ID, because handles are more useful for controlling and +// waiting for processes. It it possible to obtain the actual process ID from +// handles by calling the GetProcessId API. +using ProcessId = HANDLE; +using SocketId = SOCKET; +constexpr SocketId SocketError{INVALID_SOCKET}; +#else using ProcessId = int; using SocketId = int; constexpr SocketId SocketError{-1}; - -//! Information about parent process passed to child process as a command-line -//! argument. On unix this is the child socket fd number formatted as a string. -using SpawnConnectInfo = std::string; - -//! Callback type used by SpawnProcess below. -using SpawnConnectInfoToArgsFn = std::function(const SpawnConnectInfo&)>; +#endif //! Spawn a new process that communicates with the current process over a socket -//! pair. Calls connect_info_to_args callback with a connection string that -//! needs to be passed to the child process, and executes the argv command line -//! it returns. Returns child process id and socket id. -std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args); - -//! Initialize spawned child process using the SpawnConnectInfo string passed to it, -//! returning a socket id for communicating with the parent process. -SocketId StartSpawned(const SpawnConnectInfo& connect_info); +//! pair. Calls spawn_argv callback with a connection string that needs to be +//! passed to the child process, and executes the argv command line it returns. +//! Returns child process id and socket id. +//! +//! The connection string is just a file descriptor number on unix. On windows, +//! it is a path to a named pipe the parent process will write +//! WSADuplicateSocket info to. In both cases, the child process can call +//! StartSpawned to get a socket handle from the connection string. +std::tuple SpawnProcess(const std::function(std::string)>& spawn_argv); + +//! Initialize spawned child process. The connect_info argument is the +//! connection string SpawnProcess generated in the parent process and passed +//! to the child on its command line. Returns socket id for communicating with +//! the parent process. +SocketId StartSpawned(const std::string& connect_info); //! Create a socket pair that can be used to communicate within a process or //! between parent and child processes. std::array SocketPair(); +//! Close a socket, throwing a KJ exception on failure. +void CloseSocket(SocketId fd); + //! Start a process and return its process id. Caller should call WaitProcess //! on the returned id. ProcessId StartProcess(const std::vector& args); diff --git a/shell.nix b/shell.nix index 2d115fea..9d6cdd49 100644 --- a/shell.nix +++ b/shell.nix @@ -1,5 +1,6 @@ { pkgs ? import {} -, crossPkgs ? import {} +, crossPkgs ? null # null means same as pkgs; overrides windows when set explicitly +, windows ? false # Cross-compile for Windows using MinGW; implies crossPkgs = pkgs.pkgsCross.mingwW64 , enableLibcxx ? false # Whether to use libc++ toolchain and libraries instead of libstdc++ , minimal ? false # Whether to create minimal shell without extra tools (faster when cross compiling) , capnprotoVersion ? null @@ -11,7 +12,11 @@ let lib = pkgs.lib; - llvmBase = crossPkgs.llvmPackages_21; + effectiveCrossPkgs = + if crossPkgs != null then crossPkgs + else if windows then pkgs.pkgsCross.mingwW64 + else pkgs; + llvmBase = pkgs.llvmPackages_21; llvm = llvmBase // lib.optionalAttrs (libcxxSanitizers != null) { libcxx = llvmBase.libcxx.override { devExtraCmakeFlags = [ "-DLLVM_USE_SANITIZER=${libcxxSanitizers}" ]; @@ -27,9 +32,9 @@ let "1.1.0" = "sha256-gxkko7LFyJNlxpTS+CWOd/p9x/778/kNIXfpDGiKM2A="; "1.2.0" = "sha256-aDcn4bLZGq8915/NPPQsN5Jv8FRWd8cAspkG3078psc="; }; - capnprotoBase = if capnprotoVersion == null then crossPkgs.capnproto else crossPkgs.capnproto.overrideAttrs (old: { + capnprotoBase = if capnprotoVersion == null then effectiveCrossPkgs.capnproto else effectiveCrossPkgs.capnproto.overrideAttrs (old: { version = capnprotoVersion; - src = crossPkgs.fetchFromGitHub { + src = effectiveCrossPkgs.fetchFromGitHub { owner = "capnproto"; repo = "capnproto"; rev = "v${capnprotoVersion}"; @@ -50,7 +55,50 @@ let "-g" ]; }; - })).override (lib.optionalAttrs enableLibcxx { clangStdenv = llvm.libcxxStdenv; }); + } // lib.optionalAttrs windows { + # Two CXXFLAGS additions needed for the MinGW cross-build: + # - _WIN32_WINNT=0x0601: mcfgthread/fwd.h hard-errors if this isn't defined + # to at least Windows 7; it's pulled in transitively via → gthr.h. + # - Wno-class-memaccess: GCC promotes this to an error on the memset(&addr,0,…) + # call in kj/async-io-win32.c++; the memset intentionally zeroes a network + # address struct, so the warning is a false positive here. + env = (old.env or { }) // { + CXXFLAGS = "${old.env.CXXFLAGS or ""} -D_WIN32_WINNT=0x0601 -Wno-class-memaccess"; + }; + # Cross-compiling capnproto for Windows with the nixpkgs llvm-mingw toolchain + # requires several cmake/nix workarounds: + # + # - BUILD_SHARED_LIBS=FALSE: static libs mean mptest.exe is self-contained, + # simplifying wine execution (no DLL search path needed). + # - WITH_FIBERS=FALSE: kj fiber support on Windows/MinGW may not build. + # + # Two nix environment issues also need fixing: + # 1. The clang wrapper uses GCC's C++ headers (libstdc++), which on this + # GCC version use the MCF thread model. MCF headers are in a separate + # package (windows.mcfgthreads.dev) not in capnproto's default buildInputs. + # 2. The clang wrapper's cc-ldflags is missing the GCC target lib directory + # that contains libgcc_s.a. We add it in preConfigure. + buildInputs = (old.buildInputs or []) ++ [ + effectiveCrossPkgs.windows.mcfgthreads.dev # provides mcfgthread/gthr.h + effectiveCrossPkgs.windows.mcfgthreads # provides libmcfgthread.a for linking + ]; + preConfigure = (old.preConfigure or "") + '' + # The nixpkgs clang-mingw wrapper omits the GCC target lib directory + # ($gcc/x86_64-w64-mingw32/lib) from its search path, so the linker + # can't find libgcc_s.a even though the file exists. Add it explicitly. + # Must use 'export' so child processes (cmake, linker) inherit the value. + export NIX_LDFLAGS_x86_64_w64_mingw32="''${NIX_LDFLAGS_x86_64_w64_mingw32:-} -L${effectiveCrossPkgs.buildPackages.gcc.cc}/x86_64-w64-mingw32/lib" + ''; + cmakeFlags = (old.cmakeFlags or []) ++ [ + "-DBUILD_SHARED_LIBS=FALSE" + "-DWITH_FIBERS=FALSE" + ]; + })).override (lib.optionalAttrs enableLibcxx { clangStdenv = llvm.libcxxStdenv; } + # Switch capnproto's cross-build to effectiveCrossPkgs.stdenv (GCC) instead of + # the default clangStdenv. Clang with GCC's libstdc++.a causes MCF thread + # symbol errors (_MCF_mutex_lock_slow etc.) because clang doesn't automatically + # link GCC's MCF runtime. GCC knows its own runtime and links it correctly. + // lib.optionalAttrs windows { clangStdenv = effectiveCrossPkgs.stdenv; }); clang = if enableLibcxx then llvm.libcxxClang else llvm.clang; clang-tools = llvm.clang-tools.override { inherit enableLibcxx; }; cmakeHashes = { @@ -65,7 +113,7 @@ let }; patches = []; })).override { isMinimalBuild = true; }; -in crossPkgs.mkShell { +in effectiveCrossPkgs.mkShell { buildInputs = [ capnproto ]; @@ -76,6 +124,14 @@ in crossPkgs.mkShell { ] ++ lib.optional (gcc != null) gcc ++ lib.optionals (!minimal) [ clang clang-tools + ] ++ lib.optionals windows [ + pkgs.capnproto # native capnp + capnpc-c++ in PATH for cmake code generation + pkgs.wine64Packages.staging # run cross-compiled mptest.exe in ctest + pkgs.gcc # native C++ compiler for the native mpgen pre-build + pkgs.openssl.dev # native capnproto cmake config: find_dependency(OpenSSL) headers + pkgs.openssl.out # native capnproto cmake config: find_dependency(OpenSSL) libs + pkgs.zlib.dev # native capnproto cmake config: find_dependency(ZLIB) headers + pkgs.zlib # native capnproto cmake config: find_dependency(ZLIB) libs ]; CC = if gcc == null then null else "${gcc}/bin/gcc"; @@ -83,4 +139,22 @@ in crossPkgs.mkShell { # Tell IWYU where its libc++ mapping lives IWYU_MAPPING_FILE = if enableLibcxx then "${llvm.libcxx.dev}/include/c++/v1/libcxx.imp" else null; + + # When cross-compiling, expose native package prefixes so ci.sh can point + # the native mpgen pre-build's CMAKE_PREFIX_PATH at them. CMAKE_PREFIX_PATH + # in the cross shell points at the Windows packages, not the native ones. + NATIVE_CAPNPROTO_PREFIX = if windows then "${pkgs.capnproto}" else null; + # OpenSSL and zlib are dependencies of the native capnproto cmake config + # (find_dependency calls); the native cmake build needs to find them too. + # FindOpenSSL.cmake needs both the dev output (headers) and out (libs). + NATIVE_OPENSSL_DEV = if windows then "${pkgs.openssl.dev}" else null; + NATIVE_OPENSSL_LIB = if windows then "${pkgs.openssl.out}" else null; + NATIVE_ZLIB_DEV = if windows then "${pkgs.zlib.dev}" else null; + NATIVE_ZLIB_LIB = if windows then "${pkgs.zlib}" else null; + # GCC and MCF thread runtime DLL directories needed by wine to run mptest.exe. + # libstdc++-6.dll and libgcc_s_seh-1.dll are in the GCC cross-compiler lib dir; + # libmcfgthread-2.dll is in the mcfgthreads package's bin dir. + WIN_RUNTIME_DLLS = if windows then + "${effectiveCrossPkgs.buildPackages.gcc.cc.lib}/x86_64-w64-mingw32/lib:${effectiveCrossPkgs.windows.mcfgthreads}/bin" + else null; } diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 4c7f7666..dade9c2f 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -231,6 +231,40 @@ void Connection::removeSyncCleanup(CleanupIt it) m_sync_cleanup_fns.erase(it); } +#ifdef WIN32 +//! Synchronous socket output stream. Cap'n Proto library only provides limited +//! support for synchronous IO. It provides `FdOutputStream` which wraps unix +//! file descriptors and calls write() internally, and `HandleOutStream` which +//! wraps windows HANDLE values and calls WriteFile() internally. This class +//! just provides analogous functionality wrapping SOCKET values and calls +//! send() internally. +class SocketOutputStream : public kj::OutputStream { +public: + explicit SocketOutputStream(SOCKET socket) : m_socket(socket) {} + + void write(const void* buffer, size_t size) override; + +private: + SOCKET m_socket; +}; + +static constexpr size_t WRITE_CLAMP_SIZE = 1u << 30; // 1GB clamp for Windows, like FdOutputStream + +void SocketOutputStream::write(const void* buffer, size_t size) { + const char* pos = reinterpret_cast(buffer); + + while (size > 0) { + int n = send(m_socket, pos, static_cast(kj::min(size, WRITE_CLAMP_SIZE)), 0); + + KJ_WIN32(n != SOCKET_ERROR, "send() failed"); + KJ_ASSERT(n > 0, "send() returned zero."); + + pos += n; + size -= n; + } +} +#endif + void EventLoop::addAsyncCleanup(std::function fn) { const Lock lock(m_mutex); @@ -266,6 +300,10 @@ EventLoop::EventLoop(const char* exe_name, LogOptions log_opts, void* context) m_post_stream = kj::mv(pipe.ends[1]); KJ_IF_MAYBE(fd, m_post_stream->getFd()) { m_post_writer = kj::heap(*fd); +#ifdef WIN32 + } else KJ_IF_MAYBE(handle, m_post_stream->getWin32Handle()) { + m_post_writer = kj::heap(reinterpret_cast(*handle)); +#endif } else { throw std::logic_error("Could not get file descriptor for new pipe."); } diff --git a/src/mp/util.cpp b/src/mp/util.cpp index f524ab23..df51fb9b 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -7,23 +7,35 @@ #include #include -#include #include #include #include -#include #include #include -#include +#include +#include // NOLINT(misc-include-cleaner) // IWYU pragma: keep +#include +#include + +#ifdef WIN32 +#include +#include +#include +#include +#else +#include #include #include #include +#include #include -#include -#include // NOLINT(misc-include-cleaner) // IWYU pragma: keep #include -#include -#include +#define _getpid getpid +#endif + +#if !defined(WIN32) || defined(HAVE_PTHREAD_GETNAME_NP) || defined(HAVE_PTHREAD_THREADID_NP) || defined(HAVE_PTHREAD_GETTHREADID_NP) +#include +#endif #ifdef __linux__ #include @@ -33,11 +45,17 @@ #include #endif // HAVE_PTHREAD_GETTHREADID_NP +#ifndef WIN32 extern "C" char **environ; // NOLINT(readability-redundant-declaration) +#else +// Forward-declare internal capnp function. +namespace kj { namespace _ { int win32Socketpair(SOCKET socks[2]); } } +#endif namespace mp { namespace { +#ifndef WIN32 std::vector MakeArgv(const std::vector& args) { std::vector argv; @@ -71,6 +89,7 @@ template (void)written; _exit(126); } +#endif } // namespace @@ -82,7 +101,7 @@ std::string ThreadName(const char* exe_name) #endif // HAVE_PTHREAD_GETNAME_NP std::ostringstream buffer; - buffer << (exe_name ? exe_name : "") << "-" << getpid() << "/"; + buffer << (exe_name ? exe_name : "") << "-" << _getpid() << "/"; if (thread_name[0] != '\0') { buffer << thread_name << "-"; @@ -92,6 +111,8 @@ std::string ThreadName(const char* exe_name) // the former are shorter and are the same as what gdb prints "LWP ...". #ifdef __linux__ buffer << syscall(SYS_gettid); +#elif defined(WIN32) + buffer << GetCurrentThreadId(); #elif defined(HAVE_PTHREAD_THREADID_NP) uint64_t tid = 0; pthread_threadid_np(nullptr, &tid); @@ -129,17 +150,68 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) return result; } -std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args) +//! Generate command line that the executable being invoked will split up using +//! the CommandLineToArgvW function, which expects arguments with spaces to be +//! quoted, quote characters to be backslash-escaped, and backslashes to also be +//! backslash-escaped, but only if they precede a quote character. +std::string CommandLineFromArgv(const std::vector& argv) +{ + std::string out; + for (const auto& arg : argv) { + if (!out.empty()) out += " "; + if (!arg.empty() && arg.find_first_of(" \t\"") == std::string::npos) { + // Argument has no quotes or spaces so escaping not necessary. + out += arg; + } else { + out += '"'; // Start with a quote + for (size_t i = 0; i < arg.size(); ++i) { + if (arg[i] == '\\') { + // Count consecutive backslashes + size_t backslash_count = 0; + while (i < arg.size() && arg[i] == '\\') { + ++backslash_count; + ++i; + } + if (i < arg.size() && arg[i] == '"') { + // Backslashes before a quote need to be doubled + out.append(backslash_count * 2 + 1, '\\'); + out.push_back('"'); + } else if (i == arg.size()) { + // Backslashes at the end of the argument precede the + // closing quote added below, so also need to be doubled + out.append(backslash_count * 2, '\\'); + --i; // Compensate for the outer loop's increment + } else { + // Otherwise, backslashes remain as-is + out.append(backslash_count, '\\'); + --i; // Compensate for the outer loop's increment + } + } else if (arg[i] == '"') { + // Escape double quotes with a backslash + out.push_back('\\'); + out.push_back('"'); + } else { + out.push_back(arg[i]); + } + } + out += '"'; // End with a quote + } + } + return out; +} + +std::tuple SpawnProcess(const std::function(std::string)>& spawn_argv) { auto fds{SocketPair()}; +#ifndef WIN32 // Evaluate the callback and build the argv array before forking. // // The parent process may be multi-threaded and holding internal library // locks at fork time. In that case, running code that allocates memory or // takes locks in the child between fork() and exec() can deadlock // indefinitely. Precomputing arguments in the parent avoids this. - const std::vector args{connect_info_to_args(std::to_string(fds[0]))}; + const std::vector args{spawn_argv(std::to_string(fds[0]))}; const std::vector argv{MakeArgv(args)}; ProcessId pid = fork(); @@ -186,44 +258,166 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ _exit(127); } return {pid, fds[1]}; +#else + // Create windows pipe to send socket over to child process. + // FILE_FLAG_OVERLAPPED lets ConnectNamedPipe be used with + // WaitForMultipleObjects so the parent detects child death instead of + // blocking forever if the child exits without opening the pipe. + static std::atomic counter{1}; + std::string pipe_path{R"(\\.\pipe\mp-)" + std::to_string(GetCurrentProcessId()) + "-" + std::to_string(counter.fetch_add(1))}; + HANDLE pipe{CreateNamedPipeA(pipe_path.c_str(), PIPE_ACCESS_OUTBOUND | FILE_FLAG_OVERLAPPED, PIPE_TYPE_MESSAGE | PIPE_WAIT, /*nMaxInstances=*/1, /*nOutBufferSize=*/0, /*nInBufferSize=*/0, /*nDefaultTimeOut=*/0, /*lpSecurityAttributes=*/nullptr)}; + KJ_WIN32(pipe != INVALID_HANDLE_VALUE, "CreateNamedPipe failed"); + + // TODO: Would be good to add more exception safety here. Resources (pipe, + // fds[1], pi.hProcess) may leak if any call below throws, and the child + // process will be orphaned. + + // Start child process + std::string cmd{CommandLineFromArgv(spawn_argv(pipe_path))}; + STARTUPINFOA si{}; + si.cb = sizeof(si); + PROCESS_INFORMATION pi{}; + KJ_WIN32(CreateProcessA(/*lpApplicationName=*/nullptr, const_cast(cmd.c_str()), /*lpProcessAttributes=*/nullptr, /*lpThreadAttributes=*/nullptr, /*bInheritHandles=*/FALSE, /*dwCreationFlags=*/0, /*lpEnvironment=*/nullptr, /*lpCurrentDirectory=*/nullptr, &si, &pi), "CreateProcess failed"); + KJ_WIN32(CloseHandle(pi.hThread), "CloseHandle(hThread)"); + + // Wait for child to connect to the pipe. WaitForMultipleObjects on both + // the connect event and the child process handle lets us fail cleanly if + // the child exits without opening the pipe. + HANDLE event{CreateEvent(/*lpEventAttributes=*/nullptr, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, /*lpName=*/nullptr)}; + KJ_WIN32(event != nullptr, "CreateEvent failed"); + OVERLAPPED ov{}; + ov.hEvent = event; + if (!ConnectNamedPipe(pipe, &ov)) { + DWORD err{GetLastError()}; + if (err == ERROR_IO_PENDING) { + HANDLE objects[2]{event, pi.hProcess}; + DWORD result{WaitForMultipleObjects(2, objects, /*bWaitAll=*/FALSE, /*dwMilliseconds=*/INFINITE)}; + KJ_WIN32(result != WAIT_FAILED, "WaitForMultipleObjects failed"); + if (result != WAIT_OBJECT_0) { + CloseHandle(event); + CloseHandle(pipe); + KJ_FAIL_REQUIRE("child process exited before connecting to named pipe"); + } + DWORD unused; + KJ_WIN32(GetOverlappedResult(pipe, &ov, &unused, /*bWait=*/FALSE), "ConnectNamedPipe failed"); + } else if (err != ERROR_PIPE_CONNECTED) { + CloseHandle(event); + KJ_FAIL_WIN32("ConnectNamedPipe", err); + } + } + KJ_WIN32(CloseHandle(event), "CloseHandle(event)"); + + // Send socket to child. Use overlapped I/O since pipe is FILE_FLAG_OVERLAPPED. + OVERLAPPED write_ov{}; + write_ov.hEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr); + KJ_WIN32(write_ov.hEvent != nullptr, "CreateEvent failed"); + // Duplicate socket for the child using its PID. + WSAPROTOCOL_INFO info{}; + KJ_WINSOCK(WSADuplicateSocket(fds[0], pi.dwProcessId, &info), "WSADuplicateSocket failed"); + // Close the parent's copy of the child's socket end. Without this, the + // parent holds fds[0] open indefinitely, so the peer socket (fds[1]) never + // sees a disconnection when the child exits, resulting in hangs reading or + // writing to fds[1]. + CloseSocket(fds[0]); + if (!WriteFile(pipe, &info, sizeof(info), nullptr, &write_ov)) { + DWORD write_err{GetLastError()}; + if (write_err != ERROR_IO_PENDING) KJ_FAIL_WIN32("WriteFile(pipe)", write_err); + } + DWORD wr; + KJ_WIN32(GetOverlappedResult(pipe, &write_ov, &wr, /*bWait=*/TRUE), "WriteFile(pipe) failed"); + KJ_REQUIRE(wr == sizeof(info), "WriteFile(pipe): short write"); + KJ_WIN32(CloseHandle(write_ov.hEvent), "CloseHandle(write_event)"); + KJ_WIN32(CloseHandle(pipe), "CloseHandle(pipe)"); + + return {pi.hProcess, fds[1]}; +#endif } -SocketId StartSpawned(const SpawnConnectInfo& connect_info) +SocketId StartSpawned(const std::string& connect_info) { +#ifndef WIN32 try { return std::stoi(connect_info); } catch (const std::exception&) { throw std::system_error(EINVAL, std::system_category(), std::string("StartSpawned: invalid connect_info '") + connect_info + "'"); } +#else + HANDLE pipe = CreateFileA(connect_info.c_str(), /*dwDesiredAccess=*/GENERIC_READ, /*dwShareMode=*/0, /*lpSecurityAttributes=*/nullptr, /*dwCreationDisposition=*/OPEN_EXISTING, /*dwFlagsAndAttributes=*/0, /*hTemplateFile=*/nullptr); + KJ_WIN32(pipe != INVALID_HANDLE_VALUE, "CreateFile(pipe) failed"); + + WSAPROTOCOL_INFO info{}; + DWORD rd; + KJ_WIN32(ReadFile(pipe, &info, sizeof(info), &rd, nullptr) && rd == sizeof(info), "ReadFile(pipe) failed"); + KJ_WIN32(CloseHandle(pipe), "CloseHandle(pipe)"); + + WSADATA dontcare; + if (int wsaErr = WSAStartup(MAKEWORD(2, 2), &dontcare)) KJ_FAIL_WIN32("WSAStartup()", wsaErr); + + SOCKET socket{WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, &info, 0, WSA_FLAG_OVERLAPPED | WSA_FLAG_NO_HANDLE_INHERIT)}; + KJ_WINSOCK(socket, "WSASocket(FROM_PROTOCOL_INFO) failed"); + return socket; +#endif } std::array SocketPair() { +#ifdef WIN32 + SOCKET pair[2]; + KJ_WINSOCK(kj::_::win32Socketpair(pair)); +#else int pair[2]; KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, pair)); KJ_SYSCALL(fcntl(pair[0], F_SETFD, FD_CLOEXEC)); KJ_SYSCALL(fcntl(pair[1], F_SETFD, FD_CLOEXEC)); +#endif return {pair[0], pair[1]}; } +void CloseSocket(SocketId fd) +{ +#ifdef WIN32 + KJ_WINSOCK(closesocket(fd)); +#else + KJ_SYSCALL(close(fd)); +#endif +} + ProcessId StartProcess(const std::vector& args) { +#ifndef WIN32 const std::vector argv{MakeArgv(args)}; ProcessId pid; if (int err = posix_spawnp(&pid, argv[0], nullptr, nullptr, argv.data(), ::environ)) { KJ_FAIL_SYSCALL("posix_spawnp", err, args.front()); } return pid; +#else + std::string cmd{CommandLineFromArgv(args)}; + STARTUPINFOA si{}; + si.cb = sizeof(si); + PROCESS_INFORMATION pi{}; + KJ_WIN32(CreateProcessA(/*lpApplicationName=*/nullptr, const_cast(cmd.c_str()), /*lpProcessAttributes=*/nullptr, /*lpThreadAttributes=*/nullptr, /*bInheritHandles=*/FALSE, /*dwCreationFlags=*/0, /*lpEnvironment=*/nullptr, /*lpCurrentDirectory=*/nullptr, &si, &pi), "CreateProcess"); + KJ_WIN32(CloseHandle(pi.hThread), "CloseHandle(hThread)"); + return pi.hProcess; +#endif } int WaitProcess(ProcessId pid) { +#ifndef WIN32 int status; if (::waitpid(pid, &status, /*options=*/0) != pid) { throw std::system_error(errno, std::system_category(), "waitpid"); } return status; +#else + DWORD result{WaitForSingleObject(pid, /*dwMilliseconds=*/INFINITE)}; + if (result != WAIT_OBJECT_0) KJ_FAIL_WIN32("WaitForSingleObject(child)", GetLastError()); + KJ_WIN32(GetExitCodeProcess(pid, &result), "GetExitCodeProcess"); + KJ_WIN32(CloseHandle(pid), "CloseHandle(process)"); + return result; +#endif } } // namespace mp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 13246293..c3c181af 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -29,6 +29,7 @@ if(BUILD_TESTING AND TARGET CapnProto::kj-test) mp/test/listen_tests.cpp mp/test/spawn_tests.cpp mp/test/test.cpp + mp/test/util_tests.cpp ) include(${PROJECT_SOURCE_DIR}/cmake/TargetCapnpSources.cmake) target_capnp_sources(mptest ${CMAKE_CURRENT_SOURCE_DIR} mp/test/foo.capnp) diff --git a/test/mp/test/listen_tests.cpp b/test/mp/test/listen_tests.cpp index a9d4dca2..10ee29c5 100644 --- a/test/mp/test/listen_tests.cpp +++ b/test/mp/test/listen_tests.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -27,10 +26,32 @@ #include #include #include +#include +#include +#include + +#ifdef WIN32 +#include +#include +#else +#include +#include #include #include -#include -#include +#endif + +#ifdef WIN32 +// Call WSAStartup before any test runs. Winsock requires WSAStartup before any +// socket call; the mp library calls it inside StartSpawned(), but +// listen_tests.cpp creates sockets directly and never reaches that code path. +// WSACleanup is intentionally omitted: the OS reclaims Winsock state on exit. +// TODO: check the return value of WSAStartup and fail fast if it returns an error. +namespace { +struct WsaInit { + WsaInit() { WSADATA data; WSAStartup(MAKEWORD(2, 2), &data); } +} g_wsa_init; +} // namespace +#endif namespace mp { namespace test { @@ -38,63 +59,113 @@ namespace { constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30}; -//! Owns a temporary Unix-domain listening socket used by ListenSetup. Tests call +//! Owns a temporary listening socket used by ListenSetup. Tests call //! Connect() to create client socket FDs and release() to transfer the listening //! FD to ListenConnections(). -class UnixListener +class SocketListener { public: - UnixListener() + SocketListener() + { + // Use TCP on Windows to work around Wine's incompatibility with + // AF_UNIX: Wine does not support the AcceptEx extension used by KJ's + // AF_UNIX listener + // (https://gitlab.winehq.org/wine/wine/-/merge_requests/7650). + // AF_UNIX sockets work fine on real Windows. It could make sense + // later to test TCP connections on Unix as well. +#ifdef WIN32 + m_addr.emplace(); +#else + m_addr.emplace(); +#endif + std::visit([this](auto& addr) { Init(addr); }, m_addr); + } + + ~SocketListener() { - std::string dir_template = (std::filesystem::temp_directory_path() / "mptest-listener-XXXXXX").string(); - char* dir = mkdtemp(dir_template.data()); - KJ_REQUIRE(dir != nullptr); - m_dir = dir; - m_path = m_dir + "/socket"; + if (m_fd != SocketError) mp::CloseSocket(m_fd); + if (auto* un = std::get_if(&m_addr)) { + std::error_code ec; + if (un->sun_path[0]) std::filesystem::remove(un->sun_path, ec); + if (!m_dir.empty()) std::filesystem::remove(m_dir, ec); + } + } - m_fd = socket(AF_UNIX, SOCK_STREAM, 0); - KJ_REQUIRE(m_fd >= 0); + SocketId release() + { + assert(m_fd != SocketError); + SocketId fd = m_fd; + m_fd = SocketError; + return fd; + } - sockaddr_un addr{}; - addr.sun_family = AF_UNIX; - KJ_REQUIRE(m_path.size() < sizeof(addr.sun_path)); - std::strncpy(addr.sun_path, m_path.c_str(), sizeof(addr.sun_path) - 1); + SocketId MakeConnectedSocket() const + { + return std::visit([](const auto& addr) { return Connect(addr); }, m_addr); + } + +private: + void Init(sockaddr_in& addr) + { + m_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + KJ_REQUIRE(m_fd != SocketError); + + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.sin_port = 0; KJ_REQUIRE(bind(m_fd, reinterpret_cast(&addr), sizeof(addr)) == 0); KJ_REQUIRE(listen(m_fd, SOMAXCONN) == 0); + + socklen_t len = sizeof(addr); + KJ_REQUIRE(getsockname(m_fd, reinterpret_cast(&addr), &len) == 0); } - ~UnixListener() + void Init(sockaddr_un& addr) { - if (m_fd >= 0) close(m_fd); - if (!m_path.empty()) unlink(m_path.c_str()); - if (!m_dir.empty()) rmdir(m_dir.c_str()); + auto base = std::filesystem::temp_directory_path(); + auto now = std::chrono::steady_clock::now().time_since_epoch().count(); + for (unsigned attempt = 0; ; ++attempt) { + auto path = base / ("mptest-listener-" + std::to_string(now) + std::to_string(attempt)); + if (std::filesystem::create_directory(path)) { + m_dir = path.string(); + break; + } + } + std::string path = m_dir + "/socket"; + + m_fd = socket(AF_UNIX, SOCK_STREAM, 0); + KJ_REQUIRE(m_fd != SocketError); + + addr.sun_family = AF_UNIX; + KJ_REQUIRE(path.size() < sizeof(addr.sun_path)); + std::strncpy(addr.sun_path, path.c_str(), sizeof(addr.sun_path) - 1); + KJ_REQUIRE(bind(m_fd, reinterpret_cast(&addr), sizeof(addr)) == 0); + KJ_REQUIRE(listen(m_fd, SOMAXCONN) == 0); } - int release() + static SocketId Connect(const sockaddr_in& addr) { - assert(m_fd >= 0); - int fd = m_fd; - m_fd = -1; + SocketId fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + KJ_REQUIRE(fd != SocketError); + + sockaddr_in a = addr; + KJ_REQUIRE(connect(fd, reinterpret_cast(&a), sizeof(a)) == 0); return fd; } - int MakeConnectedSocket() const + static SocketId Connect(const sockaddr_un& addr) { - int fd = socket(AF_UNIX, SOCK_STREAM, 0); - KJ_REQUIRE(fd >= 0); + SocketId fd = socket(AF_UNIX, SOCK_STREAM, 0); + KJ_REQUIRE(fd != SocketError); - sockaddr_un addr{}; - addr.sun_family = AF_UNIX; - KJ_REQUIRE(m_path.size() < sizeof(addr.sun_path)); - std::strncpy(addr.sun_path, m_path.c_str(), sizeof(addr.sun_path) - 1); - KJ_REQUIRE(connect(fd, reinterpret_cast(&addr), sizeof(addr)) == 0); + sockaddr_un a = addr; + KJ_REQUIRE(connect(fd, reinterpret_cast(&a), sizeof(a)) == 0); return fd; } -private: - int m_fd{-1}; + SocketId m_fd{SocketError}; std::string m_dir; - std::string m_path; + std::variant m_addr; }; //! Runs a client EventLoop on its own thread and connects one socket FD to the @@ -103,7 +174,7 @@ class UnixListener class ClientSetup { public: - explicit ClientSetup(int fd) + explicit ClientSetup(SocketId fd) : thread([this, fd] { EventLoop loop("mptest-client", [](mp::LogMessage log) { KJ_LOG(INFO, log.level, log.message); @@ -205,7 +276,7 @@ class ListenSetup KJ_REQUIRE(matched); } - UnixListener listener; + SocketListener listener; std::promise ready_promise; std::optional m_loop_ref; Mutex counter_mutex; @@ -314,8 +385,8 @@ KJ_TEST("ListenConnections handles a client that disconnects before being accept // This is racy, if the close does not happen before accept(), // the connection is accepted normally. - int fd = server.listener.MakeConnectedSocket(); - KJ_SYSCALL(close(fd)); + mp::SocketId fd = server.listener.MakeConnectedSocket(); + mp::CloseSocket(fd); // Wait for the connection to either be accepted and disconnected, or fail // to be accepted and log the error above. diff --git a/test/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index bdc9ef54..ed680642 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -9,23 +9,31 @@ #include #include #include -#include #include +#include #include #include -#include #include #include -#include #include #include +#ifdef WIN32 +#include +#else +#include +#include +#include +#endif + namespace mp { namespace test { namespace { constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30}; +#ifndef WIN32 + // Poll for child process exit using waitpid(..., WNOHANG) until the child exits // or timeout expires. Returns true if the child exited and status_out was set. // Returns false on timeout or error. @@ -44,14 +52,53 @@ static bool WaitPidWithTimeout(ProcessId pid, std::chrono::milliseconds timeout, } return false; } +#endif // !WIN32 } // namespace +namespace { +#ifdef WIN32 +KJ_TEST("SpawnProcess does not hang if child never connects to named pipe") +{ + // Without FILE_FLAG_OVERLAPPED on the named pipe, ConnectNamedPipe blocks + // forever if the child exits without opening the pipe. Verify SpawnProcess + // detects child exit and throws instead of hanging. + // + // Run in a detached thread so the test suite times out and reports a failure + // instead of hanging indefinitely if the bug is reintroduced. + std::atomic done{false}; + std::thread t([&done] { + try { + auto [process, socket]{SpawnProcess([](std::string) -> std::vector { + // A child that exits immediately without opening the named pipe. + return {"cmd.exe", "/c", "exit 0"}; + })}; + CloseHandle(process); + CloseSocket(socket); + } catch (...) { + // Throwing is the expected outcome; blocking forever is not. + } + done.store(true, std::memory_order_relaxed); + }); + t.detach(); + + const auto deadline{std::chrono::steady_clock::now() + FAILURE_TIMEOUT}; + while (!done.load(std::memory_order_relaxed) && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds{10}); + } + KJ_EXPECT(done.load(std::memory_order_relaxed), "SpawnProcess hung waiting for child that never connected"); +} +#endif // WIN32 + +#ifndef WIN32 KJ_TEST("SpawnProcess does not run callback in child") { // This test is designed to fail deterministically if fd_to_args is invoked // in the post-fork child: a mutex held by another parent thread at fork // time appears locked forever in the child. + // + // This test is Unix-only: Windows uses CreateProcess (not fork), so the + // inherited-locked-mutex hazard does not apply there. std::mutex target_mutex; std::mutex control_mutex; std::condition_variable control_cv; @@ -90,7 +137,7 @@ KJ_TEST("SpawnProcess does not run callback in child") control_cv.notify_one(); }); - const auto [pid, socket]{SpawnProcess([&](SpawnConnectInfo connect_info) -> std::vector { + const auto [pid, socket]{SpawnProcess([&](std::string connect_info) -> std::vector { // If this callback runs in the post-fork child, target_mutex appears // locked forever (the owning thread does not exist), so this deadlocks. std::lock_guard g(target_mutex); @@ -113,5 +160,7 @@ KJ_TEST("SpawnProcess does not run callback in child") KJ_EXPECT(exited, "Timeout waiting for child process to exit"); KJ_EXPECT(WIFEXITED(status) && WEXITSTATUS(status) == 0); } +#endif // !WIN32 +} // namespace } // namespace test } // namespace mp diff --git a/test/mp/test/util_tests.cpp b/test/mp/test/util_tests.cpp new file mode 100644 index 00000000..ea8f2e75 --- /dev/null +++ b/test/mp/test/util_tests.cpp @@ -0,0 +1,112 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include + +#ifdef WIN32 +#include +#include +#include +#endif + +namespace mp { +namespace test { +namespace { + +KJ_TEST("CommandLineFromArgv quoting") +{ + // Arguments without spaces, tabs, or quotes pass through unquoted, even if + // they contain backslashes. + KJ_EXPECT(CommandLineFromArgv({}) == ""); + KJ_EXPECT(CommandLineFromArgv({"simple"}) == "simple"); + KJ_EXPECT(CommandLineFromArgv({"a", "b", "c"}) == "a b c"); + KJ_EXPECT(CommandLineFromArgv({R"(C:\a\b)"}) == R"(C:\a\b)"); + KJ_EXPECT(CommandLineFromArgv({R"(\\.\pipe\mp-1234-1)"}) == R"(\\.\pipe\mp-1234-1)"); + KJ_EXPECT(CommandLineFromArgv({R"(\)"}) == R"(\)"); + + // Empty arguments must be quoted so they are not dropped. + KJ_EXPECT(CommandLineFromArgv({""}) == R"("")"); + KJ_EXPECT(CommandLineFromArgv({"a", "", "b"}) == R"(a "" b)"); + + // Arguments with spaces or tabs are quoted. + KJ_EXPECT(CommandLineFromArgv({"has space"}) == R"("has space")"); + KJ_EXPECT(CommandLineFromArgv({"has\ttab"}) == "\"has\ttab\""); + KJ_EXPECT(CommandLineFromArgv({R"(C:\Program Files\bitcoin\bitcoin-node.exe)", "-ipcfd", "4"}) == + R"("C:\Program Files\bitcoin\bitcoin-node.exe" -ipcfd 4)"); + + // Embedded quotes are backslash-escaped. + KJ_EXPECT(CommandLineFromArgv({R"(say "hi")"}) == R"("say \"hi\"")"); + KJ_EXPECT(CommandLineFromArgv({R"(")"}) == R"("\"")"); + + // Backslashes preceding a quote are doubled; other backslashes are not. + KJ_EXPECT(CommandLineFromArgv({R"(back\\"slash quote)"}) == R"("back\\\\\"slash quote")"); + + // Backslashes at the end of a quoted argument precede the closing quote, + // so they must be doubled too, or the closing quote would be read as an + // escaped literal quote and the argument would swallow the rest of the + // command line. + KJ_EXPECT(CommandLineFromArgv({R"(trailing backslash\)"}) == R"("trailing backslash\\")"); + KJ_EXPECT(CommandLineFromArgv({R"(trailing backslashes\\)"}) == R"("trailing backslashes\\\\")"); + KJ_EXPECT(CommandLineFromArgv({R"(mix \" of \\" things\)"}) == R"("mix \\\" of \\\\\" things\\")"); +} + +#ifdef WIN32 +KJ_TEST("CommandLineFromArgv round-trips through CommandLineToArgvW") +{ + //! Argument vectors covering the CommandLineToArgvW quoting rules: plain + //! arguments, spaces and tabs, embedded quotes, backslashes in various + //! positions, and realistic Windows paths. + const std::vector> quoting_cases{ + {"simple"}, + {"a", "b", "c"}, + {""}, + {"a", "", "b"}, + {"has space"}, + {"has\ttab"}, + {R"(say "hi")"}, + {R"(")"}, + {R"(\)"}, + {R"(C:\a\b)"}, + {R"(C:\Program Files\bitcoin\bitcoin-node.exe)", "-ipcfd", "4"}, + {R"(\\.\pipe\mp-1234-1)"}, + {R"(trailing backslash\)"}, + {R"(trailing backslashes\\)"}, + {R"(back\\"slash quote)"}, + {R"(mix \" of \\" things\)"}, + }; + + for (const auto& argv : quoting_cases) { + // Prepend a plain program name: CommandLineToArgvW parses the first + // token with simpler rules (no backslash escaping), so only the + // remaining arguments exercise the quoting logic under test. + std::vector args{"prog"}; + args.insert(args.end(), argv.begin(), argv.end()); + + const std::string cmd{CommandLineFromArgv(args)}; + // Test arguments are ASCII, so widening by casting is fine. + const std::wstring wcmd{cmd.begin(), cmd.end()}; + + int argc{0}; + LPWSTR* wargv{CommandLineToArgvW(wcmd.c_str(), &argc)}; + KJ_ASSERT(wargv != nullptr, cmd); + KJ_EXPECT(argc == static_cast(args.size()), cmd, argc); + for (int i = 0; i < argc && i < static_cast(args.size()); ++i) { + const std::wstring warg{wargv[i]}; + const std::string arg{warg.begin(), warg.end()}; + KJ_EXPECT(arg == args[i], cmd, i, arg); + } + LocalFree(wargv); + } +} +#endif + +} // namespace +} // namespace test +} // namespace mp