From 8270bcec7d272c7dd092cc51628213702713843f Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Jul 2026 07:28:07 -0400 Subject: [PATCH 1/8] Improve SpawnProcess API and documentation Remove recently introduced SpawnConnectInfo and SpawnConnectInfoToArgsFn type aliases since they are the same on all platforms and might obscure the fact that connect info should be treated as an opaque string. Co-authored-by: Sjors Provoost --- example/example.cpp | 5 +++-- include/mp/util.h | 28 +++++++++++++--------------- src/mp/util.cpp | 6 +++--- test/mp/test/spawn_tests.cpp | 2 +- 4 files changed, 20 insertions(+), 21 deletions(-) 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..8e78ee2f 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -292,22 +292,20 @@ 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&)>; - //! 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, and the +//! child process can call StartSpawned to parse it. +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. diff --git a/src/mp/util.cpp b/src/mp/util.cpp index f524ab23..242b4e05 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -129,7 +129,7 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) return result; } -std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args) +std::tuple SpawnProcess(const std::function(std::string)>& spawn_argv) { auto fds{SocketPair()}; @@ -139,7 +139,7 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ // 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(); @@ -188,7 +188,7 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ return {pid, fds[1]}; } -SocketId StartSpawned(const SpawnConnectInfo& connect_info) +SocketId StartSpawned(const std::string& connect_info) { try { return std::stoi(connect_info); diff --git a/test/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index bdc9ef54..d1b4df8f 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -90,7 +90,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); From dc27ce435be9814141e6a13a42e7fda363ec958e Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 22 Jun 2026 12:47:49 -0400 Subject: [PATCH 2/8] util: Add Windows CommandLineFromArgv escaping function Co-authored-by: Sjors Provoost --- include/mp/util.h | 7 +++ src/mp/util.cpp | 50 ++++++++++++++++ test/CMakeLists.txt | 1 + test/mp/test/util_tests.cpp | 112 ++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 test/mp/test/util_tests.cpp diff --git a/include/mp/util.h b/include/mp/util.h index 8e78ee2f..9e8311ca 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -286,6 +286,13 @@ 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; using ProcessId = int; diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 242b4e05..095b19eb 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -129,6 +129,56 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) return result; } +//! 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()}; 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/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 From 52e20dd364433f4989968d9c8c497dddd5fd0c7a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 22 Jun 2026 12:47:49 -0400 Subject: [PATCH 3/8] util: Add Windows support Add Windows-specific code to support building and running on Windows: - util.h: Guard ProcessId/SocketId/SocketError type aliases with WIN32 ifdefs so they use SOCKET/uintptr_t on Windows and int on Unix. Add winsock2.h include on Windows. - util.cpp: Guard Unix-specific system headers with WIN32 ifdefs. Add Windows-specific includes (windows.h, winsock2.h). Guard MaxFd() with #ifndef WIN32. Add GetCurrentThreadId() branch in ThreadName(). Add win32Socketpair() forward-declare. Add Windows branch in SocketPair() using win32Socketpair(). Add CommandLineFromArgv() helper needed to construct CreateProcess command lines. Add Windows branch in SpawnProcess() using named pipes and WSADuplicateSocket to pass socket to child. Add Windows branch in StartSpawned() reading socket from named pipe. Add Windows branch in WaitProcess() using WaitForSingleObject/GetExitCodeProcess. - proxy.cpp: Add SocketOutputStream class on Windows (analogous to FdOutputStream but using SOCKET/send()). Add Windows branch in EventLoop constructor to create m_post_writer using SocketOutputStream. Co-Authored-By: ViniciusCestarii --- include/mp/util.h | 30 +++++++++- src/mp/proxy.cpp | 38 ++++++++++++ src/mp/util.cpp | 113 +++++++++++++++++++++++++++++++++-- test/mp/test/spawn_tests.cpp | 16 ++++- 4 files changed, 186 insertions(+), 11 deletions(-) diff --git a/include/mp/util.h b/include/mp/util.h index 9e8311ca..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. @@ -295,17 +306,29 @@ 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}; +#endif //! Spawn a new process that communicates with the current process over a socket //! 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, and the -//! child process can call StartSpawned to parse it. +//! 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 @@ -318,6 +341,9 @@ SocketId StartSpawned(const std::string& connect_info); //! 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/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 095b19eb..7dbb54eb 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -7,24 +7,31 @@ #include #include -#include #include #include #include #include #include #include -#include -#include -#include -#include -#include #include #include // NOLINT(misc-include-cleaner) // IWYU pragma: keep #include #include #include +#ifdef WIN32 +#include +#include +#include +#else +#include +#include +#include +#include +#include +#include +#endif + #ifdef __linux__ #include #endif @@ -33,11 +40,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 +84,7 @@ template (void)written; _exit(126); } +#endif } // namespace @@ -92,6 +106,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); @@ -183,6 +199,7 @@ std::tuple SpawnProcess(const std::function SpawnProcess(const std::function 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, 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)"); + + // Send socket to the child via the pipe + KJ_WIN32(ConnectNamedPipe(pipe, nullptr) || GetLastError() == ERROR_PIPE_CONNECTED, "ConnectNamedPipe 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]); + DWORD wr; + KJ_WIN32(WriteFile(pipe, &info, sizeof(info), &wr, nullptr) && wr == sizeof(info), "WriteFile(pipe) failed"); + KJ_WIN32(CloseHandle(pipe), "CloseHandle(pipe)"); + + return {pi.hProcess, fds[1]}; +#endif } 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/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index d1b4df8f..6f27eefe 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -9,21 +9,25 @@ #include #include #include -#include #include #include #include -#include #include #include -#include #include #include +#ifndef WIN32 +#include +#include +#include +#endif + namespace mp { namespace test { namespace { +#ifndef WIN32 constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30}; // Poll for child process exit using waitpid(..., WNOHANG) until the child exits @@ -44,14 +48,19 @@ static bool WaitPidWithTimeout(ProcessId pid, std::chrono::milliseconds timeout, } return false; } +#endif // !WIN32 } // namespace +#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; @@ -113,5 +122,6 @@ 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 test } // namespace mp From ef7a7a2f1617621538ce0a01647f8843d9eaa1fb Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 4 Aug 2026 19:53:44 -0400 Subject: [PATCH 4/8] SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (https://github.com/bitcoin-core/libmultiprocess/pull/231#discussion_r3706021950) Co-Authored-By: ViniciusCestarii Co-Authored-By: Claude Sonnet 4.6 --- src/mp/util.cpp | 46 ++++++++++++++++++++++++++++++++---- test/mp/test/spawn_tests.cpp | 43 +++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 7dbb54eb..c0d3662a 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -255,9 +255,12 @@ std::tuple SpawnProcess(const std::function 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, PIPE_TYPE_MESSAGE | PIPE_WAIT, /*nMaxInstances=*/1, /*nOutBufferSize=*/0, /*nInBufferSize=*/0, /*nDefaultTimeOut=*/0, /*lpSecurityAttributes=*/nullptr)}; + 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, @@ -272,8 +275,37 @@ std::tuple SpawnProcess(const std::function(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)"); - // Send socket to the child via the pipe - KJ_WIN32(ConnectNamedPipe(pipe, nullptr) || GetLastError() == ERROR_PIPE_CONNECTED, "ConnectNamedPipe failed"); + // 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"); @@ -282,8 +314,14 @@ std::tuple SpawnProcess(const std::function #include #include +#include #include #include #include @@ -17,7 +18,9 @@ #include #include -#ifndef WIN32 +#ifdef WIN32 +#include +#else #include #include #include @@ -27,9 +30,10 @@ namespace mp { namespace test { namespace { -#ifndef WIN32 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. @@ -52,6 +56,40 @@ static bool WaitPidWithTimeout(ProcessId pid, std::chrono::milliseconds timeout, } // 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") { @@ -123,5 +161,6 @@ KJ_TEST("SpawnProcess does not run callback in child") KJ_EXPECT(WIFEXITED(status) && WEXITSTATUS(status) == 0); } #endif // !WIN32 +} // namespace } // namespace test } // namespace mp From 08450131fb02bc5ecacd2205773f112b7702fb07 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Apr 2026 11:17:39 -0400 Subject: [PATCH 5/8] util: make pthreads optional on Windows to enable MSVC builds Allow code to compile without pthreads available, as required for MSVC compatibility. Avoid unconditional POSIX calls (fork, posix_spawn, pthread_getname_np) by moving them into #ifndef WIN32 or HAVE_PTHREAD_* guards. When pthreads is available on Windows (detected via cmake HAVE_PTHREAD_* checks), still use it for thread name reporting since it provides useful information at low cost. Also add Threads::Threads as an explicit dependency of the multiprocess library. proxy.cpp directly uses thread_local, std::this_thread, and std::thread, and the dependency was previously satisfied only through transitive linkage from CapnProto::kj-async. Co-Authored-By: Claude Sonnet 4.6 --- CMakeLists.txt | 1 + src/mp/util.cpp | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) 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/src/mp/util.cpp b/src/mp/util.cpp index c0d3662a..df51fb9b 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -10,17 +10,16 @@ #include #include #include -#include #include #include #include #include // NOLINT(misc-include-cleaner) // IWYU pragma: keep -#include #include #include #ifdef WIN32 #include +#include #include #include #else @@ -30,6 +29,12 @@ #include #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__ @@ -96,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 << "-"; From fc543c53ad8abdecc567ba8982cedc46c1d8efab Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Jul 2026 09:59:40 -0400 Subject: [PATCH 6/8] test: fix listen_tests to compile and run on Windows Replace POSIX-only headers (sys/socket.h, sys/un.h, unistd.h) with Windows equivalents (afunix.h via util.h), guard them with #ifdef WIN32, use TCP sockets instead of Unix sockets for Wine compatibility, replace mkdtemp/unlink/rmdir with std::filesystem equivalents, and use SocketId/SocketError types instead of int/-1 for socket handles so the file compiles and works with MinGW. Co-Authored-By: Claude Sonnet 4.6 --- test/mp/test/listen_tests.cpp | 138 ++++++++++++++++++++++++---------- 1 file changed, 98 insertions(+), 40 deletions(-) diff --git a/test/mp/test/listen_tests.cpp b/test/mp/test/listen_tests.cpp index a9d4dca2..3d394434 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,19 @@ #include #include #include +#include +#include +#include + +#ifdef WIN32 +#include +#include +#else +#include +#include #include #include -#include -#include +#endif namespace mp { namespace test { @@ -38,63 +46,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() { - 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"; + // 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); + } - m_fd = socket(AF_UNIX, SOCK_STREAM, 0); - KJ_REQUIRE(m_fd >= 0); + ~SocketListener() + { + 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); + } + } - 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 release() + { + assert(m_fd != SocketError); + SocketId fd = m_fd; + m_fd = SocketError; + return fd; + } + + 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 +161,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 +263,7 @@ class ListenSetup KJ_REQUIRE(matched); } - UnixListener listener; + SocketListener listener; std::promise ready_promise; std::optional m_loop_ref; Mutex counter_mutex; @@ -314,8 +372,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. From fa056ce761522608ad2931c0ad0bf0104ae0e161 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Jul 2026 04:56:32 -0400 Subject: [PATCH 7/8] test: Initialize Winsock in listen_tests.cpp on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listen_tests.cpp tests fail on Windows with: "expected m_fd != SocketError [18446744073709551615 != 18446744073709551615]" socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) returns INVALID_SOCKET because WSAStartup() has not been called. The mp library calls WSAStartup() only inside ConnectSocketToProcess(), which listen_tests.cpp never reaches — it creates sockets directly using the BSD API. Fix: add a static initializer that calls WSAStartup(MAKEWORD(2,2), ...) at program startup before any test runs. Co-Authored-By: Claude Sonnet 4.6 --- test/mp/test/listen_tests.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/mp/test/listen_tests.cpp b/test/mp/test/listen_tests.cpp index 3d394434..10ee29c5 100644 --- a/test/mp/test/listen_tests.cpp +++ b/test/mp/test/listen_tests.cpp @@ -40,6 +40,19 @@ #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 { namespace { From 0fcfa78d90759aec5a71646d38bbea124942a9e9 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 21 Apr 2026 23:26:39 -0400 Subject: [PATCH 8/8] ci: add Windows cross-compilation config using MinGW and Wine - shell.nix: add `windows` parameter that selects pkgs.pkgsCross.mingwW64 as the cross target; also change crossPkgs default from import{} to null (cleaner API). When windows=true, add native pkgs.capnproto to nativeBuildInputs so capnp/capnpc-c++ are in PATH for cmake code generation, and add wine64Packages.staging so ctest can run mptest.exe via wine. Change llvmBase to always use pkgs (native) instead of crossPkgs. - ci/configs/windows.bash: new config that cross-compiles with mingw, sets CMAKE_SYSTEM_NAME=Windows, CMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER (so cmake finds native capnp from PATH), CMAKE_CROSSCOMPILING_EMULATOR=wine (so ctest runs mptest.exe via wine), and sets MPGEN_PRE_BUILD=1. - ci/scripts/ci.sh: add MPGEN_PRE_BUILD support: when set, build native mpgen in $CI_DIR-native before the main cross build, then inject -DMPGEN_EXECUTABLE into CMAKE_ARGS. This is needed because cmake's add_custom_command does not use CMAKE_CROSSCOMPILING_EMULATOR, so the cross-compiled mpgen.exe cannot be used as a code generator directly. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 2 +- ci/README.md | 1 + ci/configs/windows.bash | 21 ++++++++++ ci/scripts/ci.sh | 52 ++++++++++++++++++++++++ shell.nix | 86 +++++++++++++++++++++++++++++++++++++--- 5 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 ci/configs/windows.bash 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/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/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; }