From 7c4d98f09e1129ed3f7e188fdc31c305f919b2c5 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Wed, 1 Apr 2026 14:05:11 +0200 Subject: fix fork() issues on linux and MacOS (#910) - Improvement: Hub child process spawning on macOS now uses `posix_spawn` in line with Apple recommendations - Bugfix: Hub child process spawning on Linux now uses `vfork` instead of `fork`, preventing ENOMEM failures on systems with strict memory overcommit (`vm.overcommit_memory=2`) - Bugfix: Fixed process group management on POSIX; child processes were not placed into the correct process group, breaking group-wide signal delivery --- src/zencore/process.cpp | 170 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 158 insertions(+), 12 deletions(-) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index 9cbbfa56a..ee821944a 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -37,7 +37,9 @@ ZEN_THIRD_PARTY_INCLUDES_START #endif #if ZEN_PLATFORM_MAC +# include # include +# include # include # include #endif @@ -135,8 +137,68 @@ IsZombieProcess(int pid, std::error_code& OutEc) } return false; } + +static char** +GetEnviron() +{ + return *_NSGetEnviron(); +} #endif // ZEN_PLATFORM_MAC +#if ZEN_PLATFORM_LINUX +static char** +GetEnviron() +{ + return environ; +} +#endif // ZEN_PLATFORM_LINUX + +#if ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC +// Holds a null-terminated envp array built by merging the current process environment with +// a set of overrides. When Overrides is empty, Data points directly to environ (no allocation). +// Must outlive any posix_spawn / execve call that receives Data. +struct EnvpHolder +{ + char** Data = GetEnviron(); + + explicit EnvpHolder(const std::vector>& Overrides) + { + if (Overrides.empty()) + { + return; + } + std::map EnvMap; + for (char** E = GetEnviron(); *E; ++E) + { + std::string_view Entry(*E); + const size_t EqPos = Entry.find('='); + if (EqPos != std::string_view::npos) + { + EnvMap[std::string(Entry.substr(0, EqPos))] = std::string(Entry.substr(EqPos + 1)); + } + } + for (const auto& [Key, Value] : Overrides) + { + EnvMap[Key] = Value; + } + for (const auto& [Key, Value] : EnvMap) + { + m_Strings.push_back(Key + "=" + Value); + } + for (std::string& S : m_Strings) + { + m_Ptrs.push_back(S.data()); + } + m_Ptrs.push_back(nullptr); + Data = m_Ptrs.data(); + } + +private: + std::vector m_Strings; + std::vector m_Ptrs; +}; +#endif // ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC + ////////////////////////////////////////////////////////////////////////// // Pipe creation for child process stdout capture @@ -691,6 +753,7 @@ BuildArgV(std::vector& Out, char* CommandLine) ++Cursor; } } + #endif // !WINDOWS || TESTS #if ZEN_PLATFORM_WINDOWS @@ -766,7 +829,7 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma { CreationFlags |= CREATE_NO_WINDOW; } - if (Options.Flags & CreateProcOptions::Flag_Windows_NewProcessGroup) + if (Options.Flags & CreateProcOptions::Flag_NewProcessGroup) { CreationFlags |= CREATE_NEW_PROCESS_GROUP; } @@ -1070,23 +1133,30 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine } return CreateProcNormal(Executable, CommandLine, Options); -#else +#elif ZEN_PLATFORM_LINUX + // vfork uses CLONE_VM|CLONE_VFORK: the child shares the parent's address space and the + // parent is suspended until the child calls exec or _exit. This avoids page-table duplication + // and the ENOMEM that fork() produces on systems with strict overcommit (vm.overcommit_memory=2). + // All child-side setup uses only syscalls that do not modify user-space memory. + // Environment overrides are merged into envp before vfork so that setenv() is never called + // from the child (which would corrupt the shared address space). std::vector ArgV; std::string CommandLineZ(CommandLine); BuildArgV(ArgV, CommandLineZ.data()); ArgV.push_back(nullptr); - int ChildPid = fork(); + EnvpHolder Envp(Options.Environment); + + int ChildPid = vfork(); if (ChildPid < 0) { - ThrowLastError("Failed to fork a new child process"); + ThrowLastError("Failed to vfork a new child process"); } else if (ChildPid == 0) { if (Options.WorkingDirectory != nullptr) { - int Result = chdir(Options.WorkingDirectory->c_str()); - ZEN_UNUSED(Result); + chdir(Options.WorkingDirectory->c_str()); } if (Options.StdoutPipe != nullptr && Options.StdoutPipe->WriteFd >= 0) @@ -1118,23 +1188,99 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine } } - if (Options.ProcessGroupId > 0) + if (Options.Flags & CreateProcOptions::Flag_NewProcessGroup) + { + setpgid(0, 0); + } + else if (Options.ProcessGroupId > 0) { setpgid(0, Options.ProcessGroupId); } - for (const auto& [Key, Value] : Options.Environment) + execve(Executable.c_str(), ArgV.data(), Envp.Data); + _exit(127); + } + + return ChildPid; +#else // macOS + std::vector ArgV; + std::string CommandLineZ(CommandLine); + BuildArgV(ArgV, CommandLineZ.data()); + ArgV.push_back(nullptr); + + posix_spawn_file_actions_t FileActions; + posix_spawnattr_t Attr; + + int Err = posix_spawn_file_actions_init(&FileActions); + if (Err != 0) + { + ThrowSystemError(Err, "posix_spawn_file_actions_init failed"); + } + auto FileActionsGuard = MakeGuard([&] { posix_spawn_file_actions_destroy(&FileActions); }); + + Err = posix_spawnattr_init(&Attr); + if (Err != 0) + { + ThrowSystemError(Err, "posix_spawnattr_init failed"); + } + auto AttrGuard = MakeGuard([&] { posix_spawnattr_destroy(&Attr); }); + + if (Options.WorkingDirectory != nullptr) + { + Err = posix_spawn_file_actions_addchdir_np(&FileActions, Options.WorkingDirectory->c_str()); + if (Err != 0) { - setenv(Key.c_str(), Value.c_str(), 1); + ThrowSystemError(Err, "posix_spawn_file_actions_addchdir_np failed"); } + } + + if (Options.StdoutPipe != nullptr && Options.StdoutPipe->WriteFd >= 0) + { + const int StdoutWriteFd = Options.StdoutPipe->WriteFd; + ZEN_ASSERT(StdoutWriteFd > STDERR_FILENO); + posix_spawn_file_actions_adddup2(&FileActions, StdoutWriteFd, STDOUT_FILENO); - if (execv(Executable.c_str(), ArgV.data()) < 0) + if (Options.StderrPipe != nullptr && Options.StderrPipe->WriteFd >= 0) { - ThrowLastError("Failed to exec() a new process image"); + const int StderrWriteFd = Options.StderrPipe->WriteFd; + ZEN_ASSERT(StderrWriteFd > STDERR_FILENO && StderrWriteFd != StdoutWriteFd); + posix_spawn_file_actions_adddup2(&FileActions, StderrWriteFd, STDERR_FILENO); + posix_spawn_file_actions_addclose(&FileActions, StderrWriteFd); } + else + { + posix_spawn_file_actions_adddup2(&FileActions, StdoutWriteFd, STDERR_FILENO); + } + + posix_spawn_file_actions_addclose(&FileActions, StdoutWriteFd); + } + else if (!Options.StdoutFile.empty()) + { + posix_spawn_file_actions_addopen(&FileActions, STDOUT_FILENO, Options.StdoutFile.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644); + posix_spawn_file_actions_adddup2(&FileActions, STDOUT_FILENO, STDERR_FILENO); } - return ChildPid; + if (Options.Flags & CreateProcOptions::Flag_NewProcessGroup) + { + posix_spawnattr_setflags(&Attr, POSIX_SPAWN_SETPGROUP); + posix_spawnattr_setpgroup(&Attr, 0); + } + else if (Options.ProcessGroupId > 0) + { + posix_spawnattr_setflags(&Attr, POSIX_SPAWN_SETPGROUP); + posix_spawnattr_setpgroup(&Attr, Options.ProcessGroupId); + } + + EnvpHolder Envp(Options.Environment); + + pid_t ChildPid = 0; + Err = posix_spawn(&ChildPid, Executable.c_str(), &FileActions, &Attr, ArgV.data(), Envp.Data); + if (Err != 0) + { + ThrowSystemError(Err, "Failed to posix_spawn a new child process"); + } + + return int(ChildPid); #endif } -- cgit v1.2.3 From c49e5b15e0f86080d7d33e4e31aecfb701f8f96f Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 13 Apr 2026 14:05:03 +0200 Subject: Some minor polish from tourist branch (#949) - Replace per-type fmt::formatter specializations (StringBuilderBase, NiceBase) with a single generic formatter using a HasStringViewConversion concept - Add ThousandsNum for comma-separated integer formatting (e.g. "1,234,567") - Thread naming now accepts a sort hint for trace ordering - Fix main thread trace registration to use actual thread ID and sort first - Add ExpandEnvironmentVariables() for expanding %VAR% references in strings, with tests - Add ParseHexBytes() overload with expected byte count validation - Add Flag_BelowNormalPriority to CreateProcOptions (BELOW_NORMAL_PRIORITY_CLASS on Windows, setpriority on POSIX) - Add PrettyScroll progress bar mode that pins the status line to the bottom of the terminal using scroll regions, with signal handler cleanup for Ctrl+C/SIGTERM --- src/zencore/process.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index ee821944a..aa41c82ff 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -28,6 +28,7 @@ ZEN_THIRD_PARTY_INCLUDES_START # include # include # include +# include # include # include # include @@ -833,6 +834,10 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma { CreationFlags |= CREATE_NEW_PROCESS_GROUP; } + if (Options.Flags & CreateProcOptions::Flag_BelowNormalPriority) + { + CreationFlags |= BELOW_NORMAL_PRIORITY_CLASS; + } if (AssignToJob) { CreationFlags |= CREATE_SUSPENDED; @@ -1043,6 +1048,10 @@ CreateProcUnelevated(const std::filesystem::path& Executable, std::string_view C { CreateProcFlags |= CREATE_NO_WINDOW; } + if (Options.Flags & CreateProcOptions::Flag_BelowNormalPriority) + { + CreateProcFlags |= BELOW_NORMAL_PRIORITY_CLASS; + } if (AssignToJob) { CreateProcFlags |= CREATE_SUSPENDED; @@ -1201,6 +1210,11 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine _exit(127); } + if (Options.Flags & CreateProcOptions::Flag_BelowNormalPriority) + { + setpriority(PRIO_PROCESS, ChildPid, 5); + } + return ChildPid; #else // macOS std::vector ArgV; @@ -1280,6 +1294,11 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine ThrowSystemError(Err, "Failed to posix_spawn a new child process"); } + if (Options.Flags & CreateProcOptions::Flag_BelowNormalPriority) + { + setpriority(PRIO_PROCESS, ChildPid, 5); + } + return int(ChildPid); #endif } -- cgit v1.2.3 From 3d59b5d7036c35fe484d052ff32dbdc9d0a75cf7 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Mon, 13 Apr 2026 19:17:09 +0200 Subject: fix utf characters in source code (#953) --- src/zencore/process.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index aa41c82ff..66062df4d 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -507,7 +507,7 @@ ProcessHandle::Kill() std::error_code Ec; if (!Wait(5000, Ec)) { - // Graceful shutdown timed out — force-kill + // Graceful shutdown timed out - force-kill kill(pid_t(m_Pid), SIGKILL); Wait(1000, Ec); } @@ -1755,7 +1755,7 @@ GetProcessCommandLine(int Pid, std::error_code& OutEc) ++p; // skip null terminator of argv[0] } - // Build result: remaining entries joined by spaces (inter-arg nulls → spaces) + // Build result: remaining entries joined by spaces (inter-arg nulls -> spaces) std::string Result; Result.reserve(static_cast(End - p)); for (const char* q = p; q < End; ++q) @@ -2093,7 +2093,7 @@ GetProcessMetrics(const ProcessHandle& Handle, ProcessMetrics& OutMetrics) { Buf[Len] = '\0'; - // Skip past "pid (name) " — find last ')' to handle names containing spaces or parens + // Skip past "pid (name) " - find last ')' to handle names containing spaces or parens const char* P = strrchr(Buf, ')'); if (P) { -- cgit v1.2.3 From 38abebcb6ff417faf431dcaa103bb7f173c4b3f7 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 20 Apr 2026 10:59:41 +0200 Subject: zencore: CreateProc stdin pipes + BuildArgV quote stripping (#983) Two related improvements to `CreateProc`: ### 1. Stdin pipe support - Adds `StdinPipeHandles` + `CreateStdinPipe` alongside the existing `StdoutPipeHandles`, letting callers feed data into a child process's stdin. - Platform-agnostic RAII (Windows `HANDLE` pair / POSIX `pipe()` fd pair) with the same semantics as the stdout pipe: the inherited end goes to the child, the non-inherited end stays with the parent, destructor closes both. - `CreateProcOptions` gains a `StdinPipe*` field. - On Windows, `CreateProcNormal` is reworked so stdin/stdout redirection handles all combinations (stdin + stdout, each alone, neither) uniformly. POSIX already supported arbitrary fd redirection and just needed to honor the new option. - `zentest-appstub` gains a `-stdin_echo` mode that reads stdin to EOF and echoes it back (switching to binary mode on Windows so CRLF translation doesn't mangle bytes). - `zenserver-test` gets a `server.process` / `stdin_pipe.*` test group that exercises launching a child with a stdin pipe, writing, closing the write end, and reading back the echoed data. ### 2. Shell-style quote stripping in `BuildArgV` - Callers that build a single command-line string for `CreateProc` commonly wrap spacey paths in double quotes (e.g. `--tracefile="$path"`). The old `BuildArgV` only used quotes to suppress space-splitting and left the characters in the resulting argv element, so the spawned process saw literal `--tracefile="..."` and the value parser failed to open the quoted path. - `BuildArgV` now compacts in place, dropping quote chars as it goes, matching shell semantics for paired double quotes. --- src/zencore/process.cpp | 344 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 269 insertions(+), 75 deletions(-) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index 66062df4d..b95e706d5 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -272,6 +272,79 @@ CreateStdoutPipe(StdoutPipeHandles& OutPipe) return true; } +StdinPipeHandles::~StdinPipeHandles() +{ + Close(); +} + +StdinPipeHandles::StdinPipeHandles(StdinPipeHandles&& Other) noexcept +: ReadHandle(std::exchange(Other.ReadHandle, nullptr)) +, WriteHandle(std::exchange(Other.WriteHandle, nullptr)) +{ +} + +StdinPipeHandles& +StdinPipeHandles::operator=(StdinPipeHandles&& Other) noexcept +{ + if (this != &Other) + { + Close(); + ReadHandle = std::exchange(Other.ReadHandle, nullptr); + WriteHandle = std::exchange(Other.WriteHandle, nullptr); + } + return *this; +} + +void +StdinPipeHandles::CloseReadEnd() +{ + if (ReadHandle) + { + CloseHandle(ReadHandle); + ReadHandle = nullptr; + } +} + +void +StdinPipeHandles::CloseWriteEnd() +{ + if (WriteHandle) + { + CloseHandle(WriteHandle); + WriteHandle = nullptr; + } +} + +void +StdinPipeHandles::Close() +{ + CloseReadEnd(); + CloseWriteEnd(); +} + +bool +CreateStdinPipe(StdinPipeHandles& OutPipe) +{ + SECURITY_ATTRIBUTES Sa; + Sa.nLength = sizeof(Sa); + Sa.lpSecurityDescriptor = nullptr; + Sa.bInheritHandle = TRUE; + + HANDLE ReadHandle = nullptr; + HANDLE WriteHandle = nullptr; + if (!::CreatePipe(&ReadHandle, &WriteHandle, &Sa, 0)) + { + return false; + } + + // The write end should not be inherited by the child + SetHandleInformation(WriteHandle, HANDLE_FLAG_INHERIT, 0); + + OutPipe.ReadHandle = ReadHandle; + OutPipe.WriteHandle = WriteHandle; + return true; +} + #else StdoutPipeHandles::~StdoutPipeHandles() @@ -334,6 +407,72 @@ CreateStdoutPipe(StdoutPipeHandles& OutPipe) return true; } +StdinPipeHandles::~StdinPipeHandles() +{ + Close(); +} + +StdinPipeHandles::StdinPipeHandles(StdinPipeHandles&& Other) noexcept +: ReadFd(std::exchange(Other.ReadFd, -1)) +, WriteFd(std::exchange(Other.WriteFd, -1)) +{ +} + +StdinPipeHandles& +StdinPipeHandles::operator=(StdinPipeHandles&& Other) noexcept +{ + if (this != &Other) + { + Close(); + ReadFd = std::exchange(Other.ReadFd, -1); + WriteFd = std::exchange(Other.WriteFd, -1); + } + return *this; +} + +void +StdinPipeHandles::CloseReadEnd() +{ + if (ReadFd >= 0) + { + close(ReadFd); + ReadFd = -1; + } +} + +void +StdinPipeHandles::CloseWriteEnd() +{ + if (WriteFd >= 0) + { + close(WriteFd); + WriteFd = -1; + } +} + +void +StdinPipeHandles::Close() +{ + CloseReadEnd(); + CloseWriteEnd(); +} + +bool +CreateStdinPipe(StdinPipeHandles& OutPipe) +{ + int Fds[2]; + if (pipe(Fds) != 0) + { + return false; + } + OutPipe.ReadFd = Fds[0]; + OutPipe.WriteFd = Fds[1]; + + // Set close-on-exec on the write end so the child doesn't inherit it + fcntl(OutPipe.WriteFd, F_SETFD, FD_CLOEXEC); + return true; +} + #endif ////////////////////////////////////////////////////////////////////////// @@ -715,43 +854,53 @@ ProcessHandle::WaitExitCode() ////////////////////////////////////////////////////////////////////////// #if !ZEN_PLATFORM_WINDOWS || ZEN_WITH_TESTS +// Splits an in-process command-line string into argv, in place. Double quotes +// suppress space-splitting and are themselves stripped out (shell-style), so +// `foo "a b" c` -> {"foo", "a b", "c"} and `--k="a b"` -> {"--k=a b"}. +// Quotes do not have to wrap the whole token; pairs anywhere in a token are +// removed. There is no escape mechanism — `\"` is not recognised. static void BuildArgV(std::vector& Out, char* CommandLine) { - char* Cursor = CommandLine; + char* Read = CommandLine; while (true) { - // Skip leading whitespace - for (; *Cursor == ' '; ++Cursor) + // Skip leading whitespace between tokens + for (; *Read == ' '; ++Read) ; - // Check for nullp terminator - if (*Cursor == '\0') + if (*Read == '\0') { break; } - Out.push_back(Cursor); + // Compact in place: Write trails Read, omitting quote chars + char* Write = Read; + Out.push_back(Write); - // Extract word - int QuoteCount = 0; - do + bool InQuotes = false; + while (*Read != '\0') { - QuoteCount += (*Cursor == '\"'); - if (*Cursor == ' ' && !(QuoteCount & 1)) + if (*Read == '\"') + { + InQuotes = !InQuotes; + ++Read; + continue; + } + if (*Read == ' ' && !InQuotes) { break; } - ++Cursor; - } while (*Cursor != '\0'); + *Write++ = *Read++; + } - if (*Cursor == '\0') + const bool AtEnd = (*Read == '\0'); + *Write = '\0'; + if (AtEnd) { break; } - - *Cursor = '\0'; - ++Cursor; + ++Read; } } @@ -852,19 +1001,25 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma ExtendableWideStringBuilder<256> CommandLineZ; CommandLineZ << CommandLine; - bool DuplicatedStdErr = false; + bool DuplicatedStdErr = false; + bool CreatedStdOutFile = false; + bool UseStdHandles = false; + + if (Options.StdinPipe != nullptr && Options.StdinPipe->ReadHandle != nullptr) + { + StartupInfo.hStdInput = (HANDLE)Options.StdinPipe->ReadHandle; + UseStdHandles = true; + } if (Options.StdoutPipe != nullptr && Options.StdoutPipe->WriteHandle != nullptr) { - StartupInfo.hStdInput = nullptr; StartupInfo.hStdOutput = (HANDLE)Options.StdoutPipe->WriteHandle; + UseStdHandles = true; if (Options.StderrPipe != nullptr && Options.StderrPipe->WriteHandle != nullptr) { // Use separate pipe for stderr StartupInfo.hStdError = (HANDLE)Options.StderrPipe->WriteHandle; - StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - InheritHandles = true; } else { @@ -880,8 +1035,6 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma if (DupSuccess) { DuplicatedStdErr = true; - StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - InheritHandles = true; } } } @@ -892,7 +1045,6 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma sa.lpSecurityDescriptor = nullptr; sa.bInheritHandle = TRUE; - StartupInfo.hStdInput = nullptr; StartupInfo.hStdOutput = CreateFileW(Options.StdoutFile.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, @@ -901,25 +1053,54 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma FILE_ATTRIBUTE_NORMAL, nullptr); - const BOOL Success = DuplicateHandle(GetCurrentProcess(), - StartupInfo.hStdOutput, - GetCurrentProcess(), - &StartupInfo.hStdError, - 0, - TRUE, - DUPLICATE_SAME_ACCESS); + if (StartupInfo.hStdOutput != INVALID_HANDLE_VALUE) + { + CreatedStdOutFile = true; + UseStdHandles = true; + + const BOOL Success = DuplicateHandle(GetCurrentProcess(), + StartupInfo.hStdOutput, + GetCurrentProcess(), + &StartupInfo.hStdError, + 0, + TRUE, + DUPLICATE_SAME_ACCESS); + + if (Success) + { + DuplicatedStdErr = true; + } + else + { + CloseHandle(StartupInfo.hStdOutput); + StartupInfo.hStdOutput = 0; + CreatedStdOutFile = false; + UseStdHandles = (Options.StdinPipe != nullptr && Options.StdinPipe->ReadHandle != nullptr); + } + } + } - if (Success) + if (UseStdHandles) + { + // When STARTF_USESTDHANDLES is set, Windows requires all three handles to be + // specified. Fall back to the parent's current std handles for any that the + // caller didn't supply. This is best-effort: GetStdHandle may return handles + // that are not inheritable in a headless parent, in which case the child will + // see closed handles on those streams. + if (StartupInfo.hStdInput == nullptr) { - DuplicatedStdErr = true; - StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - InheritHandles = true; + StartupInfo.hStdInput = GetStdHandle(STD_INPUT_HANDLE); } - else + if (StartupInfo.hStdOutput == nullptr) { - CloseHandle(StartupInfo.hStdOutput); - StartupInfo.hStdOutput = 0; + StartupInfo.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); + } + if (StartupInfo.hStdError == nullptr) + { + StartupInfo.hStdError = GetStdHandle(STD_ERROR_HANDLE); } + StartupInfo.dwFlags |= STARTF_USESTDHANDLES; + InheritHandles = true; } BOOL Success = CreateProcessW(Executable.c_str(), @@ -941,7 +1122,7 @@ CreateProcNormal(const std::filesystem::path& Executable, std::string_view Comma CloseHandle(StartupInfo.hStdError); } // Only close hStdOutput if it was a file handle we created (not a pipe handle owned by caller) - if (Options.StdoutPipe == nullptr || Options.StdoutPipe->WriteHandle == nullptr) + if (CreatedStdOutFile) { CloseHandle(StartupInfo.hStdOutput); } @@ -1168,6 +1349,13 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine chdir(Options.WorkingDirectory->c_str()); } + if (Options.StdinPipe != nullptr && Options.StdinPipe->ReadFd >= 0) + { + dup2(Options.StdinPipe->ReadFd, STDIN_FILENO); + close(Options.StdinPipe->ReadFd); + // WriteFd has FD_CLOEXEC so it's auto-closed on exec + } + if (Options.StdoutPipe != nullptr && Options.StdoutPipe->WriteFd >= 0) { dup2(Options.StdoutPipe->WriteFd, STDOUT_FILENO); @@ -1248,6 +1436,15 @@ CreateProc(const std::filesystem::path& Executable, std::string_view CommandLine } } + if (Options.StdinPipe != nullptr && Options.StdinPipe->ReadFd >= 0) + { + const int StdinReadFd = Options.StdinPipe->ReadFd; + ZEN_ASSERT(StdinReadFd > STDERR_FILENO); + posix_spawn_file_actions_adddup2(&FileActions, StdinReadFd, STDIN_FILENO); + posix_spawn_file_actions_addclose(&FileActions, StdinReadFd); + // WriteFd has FD_CLOEXEC so it's auto-closed on exec + } + if (Options.StdoutPipe != nullptr && Options.StdoutPipe->WriteFd >= 0) { const int StdoutWriteFd = Options.StdoutPipe->WriteFd; @@ -2263,52 +2460,49 @@ TEST_CASE("GetProcessMetrics") TEST_CASE("BuildArgV") { - const char* Words[] = {"one", "two", "three", "four", "five"}; - struct - { - int WordCount; - const char* Input; - } Cases[] = { - {0, ""}, - {0, " "}, - {1, "one"}, - {1, " one"}, - {1, "one "}, - {2, "one two"}, - {2, " one two"}, - {2, "one two "}, - {2, " one two"}, - {2, "one two "}, - {2, "one two "}, - {3, "one two three"}, - {3, "\"one\" two \"three\""}, - {5, "one two three four five"}, + struct Case + { + std::initializer_list Expected; + const char* Input; + }; + const Case Cases[] = { + {{}, ""}, + {{}, " "}, + {{"one"}, "one"}, + {{"one"}, " one"}, + {{"one"}, "one "}, + {{"one", "two"}, "one two"}, + {{"one", "two"}, " one two"}, + {{"one", "two"}, "one two "}, + {{"one", "two", "three"}, "one two three"}, + {{"one", "two", "three", "four", "five"}, "one two three four five"}, + + // Quotes are stripped (shell-style) and suppress space-splitting + {{"one", "two", "three"}, "\"one\" two \"three\""}, + {{"hello world"}, "\"hello world\""}, + {{"--key=hello world"}, "--key=\"hello world\""}, + {{"--key=hello world"}, "\"--key=hello world\""}, + {{"a b", "c d"}, "\"a b\" \"c d\""}, + {{"abc"}, "a\"b\"c"}, + {{""}, "\"\""}, + {{"foo", "bar baz", "qux"}, "foo \"bar baz\" qux"}, }; for (const auto& Case : Cases) { std::vector OutArgs; - StringBuilder<64> Mutable; + StringBuilder<128> Mutable; Mutable << Case.Input; BuildArgV(OutArgs, Mutable.Data()); - CHECK_EQ(OutArgs.size(), Case.WordCount); + REQUIRE_EQ(OutArgs.size(), Case.Expected.size()); - for (int i = 0, n = int(OutArgs.size()); i < n; ++i) + size_t i = 0; + for (const char* Truth : Case.Expected) { - const char* Truth = Words[i]; - size_t TruthLen = strlen(Truth); - - const char* Candidate = OutArgs[i]; - bool bQuoted = (Candidate[0] == '\"'); - Candidate += bQuoted; - - CHECK(strncmp(Truth, Candidate, TruthLen) == 0); - - if (bQuoted) - { - CHECK_EQ(Candidate[TruthLen], '\"'); - } + CHECK_MESSAGE(std::string_view(OutArgs[i]) == std::string_view(Truth), + fmt::format("input='{}' arg[{}]='{}' expected='{}'", Case.Input, i, OutArgs[i], Truth)); + ++i; } } } -- cgit v1.2.3 From 015db1c80865eabd28f98378610a9eca6798a7d4 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Mon, 20 Apr 2026 14:04:57 +0200 Subject: add --pid och --executable till zen down command (#988) --- src/zencore/process.cpp | 136 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 116 insertions(+), 20 deletions(-) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index b95e706d5..4708f15cf 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -2044,9 +2044,41 @@ GetProcessCommandLine(int Pid, std::error_code& OutEc) #endif } +#if ZEN_PLATFORM_WINDOWS +static const wchar_t* +StripExtendedLengthPrefix(const wchar_t* S) +{ + // "\\?\C:\foo" -> "C:\foo"; "\\?\UNC\srv\share" -> "UNC\srv\share". + // UNC stripping is asymmetric vs a bare "\\srv\share" form, so this helper only normalizes + // the "\\?\" prefix itself. Comparing a "\\?\UNC\..." path to a bare "\\..." path is not + // expected for FindProcess inputs (zenserver installs are local paths). + if (S[0] == L'\\' && S[1] == L'\\' && S[2] == L'?' && S[3] == L'\\') + { + return S + 4; + } + return S; +} +#endif + +static bool +IsSameNativePath(const std::filesystem::path& A, const std::filesystem::path& B) +{ +#if ZEN_PLATFORM_WINDOWS + // Windows filesystem is case-insensitive; std::filesystem::path::operator== is not. + // CompareStringOrdinal is Microsoft's recommended API for filename/resource comparison: + // ordinal (no locale sensitivity) and case-insensitive when bIgnoreCase is TRUE. + // Strip "\\?\" extended-length prefix first so paths produced by MakeSafeAbsolutePath + // compare equal to paths from GetProcessExecutablePath which do not carry the prefix. + return CompareStringOrdinal(StripExtendedLengthPrefix(A.c_str()), -1, StripExtendedLengthPrefix(B.c_str()), -1, TRUE) == CSTR_EQUAL; +#else + return A == B; +#endif +} + std::error_code FindProcess(const std::filesystem::path& ExecutableImage, ProcessHandle& OutHandle, bool IncludeSelf) { + const bool MatchFullPath = ExecutableImage.has_parent_path(); #if ZEN_PLATFORM_WINDOWS HANDLE ProcessSnapshotHandle = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); if (ProcessSnapshotHandle == INVALID_HANDLE_VALUE) @@ -2063,29 +2095,41 @@ FindProcess(const std::filesystem::path& ExecutableImage, ProcessHandle& OutHand { do { - if ((IncludeSelf || (Entry.th32ProcessID != ThisProcessId)) && (ExecutableImage.filename() == Entry.szExeFile)) + if ((IncludeSelf || (Entry.th32ProcessID != ThisProcessId)) && IsSameNativePath(ExecutableImage.filename(), Entry.szExeFile)) { - std::error_code Ec; - std::filesystem::path EntryPath = GetProcessExecutablePath(Entry.th32ProcessID, Ec); - if (!Ec) + HANDLE Handle = OpenProcess(PROCESS_TERMINATE | SYNCHRONIZE | PROCESS_QUERY_INFORMATION, FALSE, Entry.th32ProcessID); + if (Handle == NULL) + { + // Skip processes we can't open (access denied, exited between snapshot and open, etc.) + continue; + } + // Close on all exits from this iteration unless ownership transfers to OutHandle (Handle set to NULL). + auto HandleGuard = MakeGuard([&]() { + if (Handle != NULL) + { + CloseHandle(Handle); + Handle = NULL; + } + }); + DWORD ExitCode = 0; + bool Match = false; + if (GetExitCodeProcess(Handle, &ExitCode) && ExitCode == STILL_ACTIVE) { - if (EntryPath == ExecutableImage) + // Re-verify executable path post-open so PID reuse between snapshot and open is caught. + std::error_code Ec; + std::filesystem::path EntryPath = GetProcessExecutablePath(Entry.th32ProcessID, Ec); + if (!Ec) { - HANDLE Handle = - OpenProcess(PROCESS_TERMINATE | SYNCHRONIZE | PROCESS_QUERY_INFORMATION, FALSE, Entry.th32ProcessID); - if (Handle == NULL) - { - return MakeErrorCodeFromLastError(); - } - DWORD ExitCode = 0; - GetExitCodeProcess(Handle, &ExitCode); - if (ExitCode == STILL_ACTIVE) - { - OutHandle.Initialize((void*)Handle); - return {}; - } + Match = MatchFullPath ? IsSameNativePath(EntryPath, ExecutableImage) + : IsSameNativePath(EntryPath.filename(), ExecutableImage.filename()); } } + if (Match) + { + OutHandle.Initialize((void*)Handle); + Handle = NULL; + return {}; + } } } while (::Process32Next(ProcessSnapshotHandle, (LPPROCESSENTRY32)&Entry)); return {}; @@ -2135,7 +2179,9 @@ FindProcess(const std::filesystem::path& ExecutableImage, ProcessHandle& OutHand std::filesystem::path EntryPath = GetProcessExecutablePath(Pid, Ec); if (!Ec) { - if (EntryPath == ExecutableImage) + const bool Match = MatchFullPath ? IsSameNativePath(EntryPath, ExecutableImage) + : IsSameNativePath(EntryPath.filename(), ExecutableImage.filename()); + if (Match) { if (Processes[ProcIndex].kp_proc.p_stat != SZOMB) { @@ -2173,7 +2219,9 @@ FindProcess(const std::filesystem::path& ExecutableImage, ProcessHandle& OutHand std::filesystem::path EntryPath = GetProcessExecutablePath((int)Pid, Ec); if (!Ec) { - if (EntryPath == ExecutableImage) + const bool Match = MatchFullPath ? IsSameNativePath(EntryPath, ExecutableImage) + : IsSameNativePath(EntryPath.filename(), ExecutableImage.filename()); + if (Match) { char Status = GetPidStatus(Pid, Ec); if (!Ec) @@ -2438,6 +2486,54 @@ TEST_CASE("FindProcess") CHECK(!Ec); CHECK(!Process.IsValid()); } + { + ProcessHandle Process; + std::filesystem::path BareName = GetRunningExecutablePath().filename(); + std::error_code Ec = FindProcess(BareName, Process, /*IncludeSelf*/ true); + CHECK(!Ec); + CHECK(Process.IsValid()); + } + { + ProcessHandle Process; + std::error_code Ec = FindProcess("this-executable-definitely-does-not-exist.xyz", Process, /*IncludeSelf*/ true); + CHECK(!Ec); + CHECK(!Process.IsValid()); + } + { + // Correct filename but wrong directory must not match when a parent path is supplied. + std::filesystem::path WrongPath = std::filesystem::path("nonexistent-dir-7f3a9c1e") / GetRunningExecutablePath().filename(); + ProcessHandle Process; + std::error_code Ec = FindProcess(WrongPath, Process, /*IncludeSelf*/ true); + CHECK(!Ec); + CHECK(!Process.IsValid()); + } +# if ZEN_PLATFORM_WINDOWS + { + // On Windows, filename match is case-insensitive (filesystem is case-insensitive). + std::filesystem::path::string_type Upper = GetRunningExecutablePath().filename().native(); + for (auto& Ch : Upper) + { + Ch = towupper(Ch); + } + ProcessHandle Process; + std::error_code Ec = FindProcess(std::filesystem::path(Upper), Process, /*IncludeSelf*/ true); + CHECK(!Ec); + CHECK(Process.IsValid()); + } + { + // "\\?\"-prefixed absolute path must still match a running process whose reported path is unprefixed. + std::filesystem::path AbsPath = MakeSafeAbsolutePath(GetRunningExecutablePath()); + std::filesystem::path::string_type Prefixed = AbsPath.native(); + if (Prefixed.rfind(L"\\\\?\\", 0) != 0) + { + Prefixed.insert(0, L"\\\\?\\"); + } + ProcessHandle Process; + std::error_code Ec = FindProcess(std::filesystem::path(Prefixed), Process, /*IncludeSelf*/ true); + CHECK(!Ec); + CHECK(Process.IsValid()); + } +# endif } TEST_CASE("GetProcessMetrics") -- cgit v1.2.3 From 28a61b12d302e9e0d37d52bf1aa5d19069f3411b Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Mon, 20 Apr 2026 15:53:22 +0200 Subject: zen history command (#987) - Feature: Per-user invocation history for `zen` and `zenserver`; each startup appends a record to a JSONL file capped at the most recent 100 entries. Location: `%LOCALAPPDATA%\Epic\Zen\History\invocations.jsonl` on Windows, `~/.zen/History/invocations.jsonl` on POSIX - `zen history` opens an interactive picker; selecting a zen row re-runs it inline and forwards the exit code, selecting a zenserver row spawns it detached - `zen history --list` (`-l`) prints the table to stdout instead of showing the picker - `zen history --filter zen|zenserver` restricts the listing to one executable - `zen history --print` prints the reconstructed command line of the selected row instead of launching it - `--enable-execution-history` global option on both binaries (default `true`) to opt out per invocation - The history file is attached to Sentry crash reports (alongside the existing zenserver log) --- src/zencore/process.cpp | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'src/zencore/process.cpp') diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp index 4708f15cf..5d37c3715 100644 --- a/src/zencore/process.cpp +++ b/src/zencore/process.cpp @@ -906,6 +906,56 @@ BuildArgV(std::vector& Out, char* CommandLine) #endif // !WINDOWS || TESTS +std::string +GetRawCommandLine() +{ +#if ZEN_PLATFORM_WINDOWS + LPWSTR Raw = ::GetCommandLineW(); + if (Raw == nullptr) + { + return {}; + } + return WideToUtf8(Raw); +#else + return {}; +#endif +} + +std::string +BuildCommandLine(std::span Argv) +{ + constexpr AsciiSet QuoteChars = " \t\""; + + std::string Result; + for (size_t I = 0; I < Argv.size(); ++I) + { + if (I > 0) + { + Result += ' '; + } + + const std::string& Arg = Argv[I]; + const bool NeedsQuotes = Arg.empty() || AsciiSet::HasAny(Arg.c_str(), QuoteChars); + if (!NeedsQuotes) + { + Result += Arg; + continue; + } + + Result += '"'; + for (char Ch : Arg) + { + if (Ch == '"') + { + Result += '\\'; + } + Result += Ch; + } + Result += '"'; + } + return Result; +} + #if ZEN_PLATFORM_WINDOWS static CreateProcResult CreateProcNormal(const std::filesystem::path& Executable, std::string_view CommandLine, const CreateProcOptions& Options) -- cgit v1.2.3