From 7aaad5e03e9fa4e3b62fd63fec0f4e67bcc6096d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=80=9A=E6=B4=B2?= Date: Mon, 10 Nov 2025 10:33:14 +0800 Subject: [PATCH] Processing: refactors process execution to separate spawning and reading --- src/common/processing.h | 25 ++++++-- src/common/processing_linux.c | 31 ++++++---- src/common/processing_windows.c | 103 +++++++++++++++++++------------- 3 files changed, 105 insertions(+), 54 deletions(-) diff --git a/src/common/processing.h b/src/common/processing.h index 7208214db..fa9f8e503 100644 --- a/src/common/processing.h +++ b/src/common/processing.h @@ -2,13 +2,26 @@ #include "util/FFstrbuf.h" -#include +typedef struct FFProcessHandle { + #if _WIN32 + void* pid; // HANDLE + void* pipeRead; // HANDLE + #else + pid_t pid; + int pipeRead; + #endif +} FFProcessHandle; -const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr); +const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle); +const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer); // Destroys handle internally static inline const char* ffProcessAppendStdOut(FFstrbuf* buffer, char* const argv[]) { - const char* error = ffProcessAppendOutput(buffer, argv, false); + FFProcessHandle handle; + const char* error = ffProcessSpawn(argv, false, &handle); + if (error) return error; + + error = ffProcessReadOutput(&handle, buffer); if (!error) ffStrbufTrimRightSpace(buffer); return error; @@ -16,7 +29,11 @@ static inline const char* ffProcessAppendStdOut(FFstrbuf* buffer, char* const ar static inline const char* ffProcessAppendStdErr(FFstrbuf* buffer, char* const argv[]) { - const char* error = ffProcessAppendOutput(buffer, argv, true); + FFProcessHandle handle; + const char* error = ffProcessSpawn(argv, true, &handle); + if (error) return error; + + error = ffProcessReadOutput(&handle, buffer); if (!error) ffStrbufTrimRightSpace(buffer); return error; diff --git a/src/common/processing_linux.c b/src/common/processing_linux.c index fc9def0dd..c4139ff4b 100644 --- a/src/common/processing_linux.c +++ b/src/common/processing_linux.c @@ -58,11 +58,9 @@ static inline int ffPipe2(int* fds, int flags) // Not thread-safe -const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr) +const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle) { int pipes[2]; - const int timeout = instance.config.general.processingTimeout; - if(ffPipe2(pipes, O_CLOEXEC) == -1) return "pipe() failed"; @@ -146,7 +144,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use if(childPid == 0) { - //Child + // Child process dup2(pipes[1], useStdErr ? STDERR_FILENO : STDOUT_FILENO); dup2(nullFile, useStdErr ? STDOUT_FILENO : STDERR_FILENO); putenv("LANG=C.UTF-8"); @@ -157,11 +155,24 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use #endif close(pipes[1]); + outHandle->pid = childPid; + outHandle->pipeRead = pipes[0]; + return NULL; +} - int FF_AUTO_CLOSE_FD childPipeFd = pipes[0]; +const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer) +{ + assert(handle->pipeRead != -1); + assert(handle->pid != -1); + + const int32_t timeout = instance.config.general.processingTimeout; + FF_AUTO_CLOSE_FD int childPipeFd = handle->pipeRead; + pid_t childPid = handle->pid; + handle->pipeRead = -1; + handle->pid = -1; char str[FF_PIPE_BUFSIZ]; - while(1) + for (;;) { if (timeout >= 0) { @@ -189,14 +200,14 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use { kill(childPid, SIGTERM); waitpid(childPid, NULL, 0); - return "poll(&pollfd, 1, timeout) error"; + return "poll(&pollfd, 1, timeout) error: not EINTR"; } } else if (pollfd.revents & POLLERR) { kill(childPid, SIGTERM); waitpid(childPid, NULL, 0); - return "poll(&pollfd, 1, timeout) error"; + return "poll(&pollfd, 1, timeout) error: POLLERR"; } } @@ -211,7 +222,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use if (!WIFEXITED(stat_loc)) return "child process exited abnormally"; if (WEXITSTATUS(stat_loc) == 127) - return "command was not found"; + return "command not found"; // We only handle 127 as an error. See `getTerminalVersionUrxvt` in `terminalshell.c` return NULL; } @@ -224,7 +235,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use else break; } - }; + } return "read(childPipeFd, str, FF_PIPE_BUFSIZ) failed"; } diff --git a/src/common/processing_windows.c b/src/common/processing_windows.c index f414ac7a1..51042a625 100644 --- a/src/common/processing_windows.c +++ b/src/common/processing_windows.c @@ -45,12 +45,13 @@ static void argvToCmdline(char* const argv[], FFstrbuf* result) } } -const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr) +const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle) { - int timeout = instance.config.general.processingTimeout; + const int32_t timeout = instance.config.general.processingTimeout; wchar_t pipeName[32]; - swprintf(pipeName, ARRAY_SIZE(pipeName), L"\\\\.\\pipe\\FASTFETCH-%u", GetCurrentProcessId()); + static unsigned pidCounter = 0; + swprintf(pipeName, ARRAY_SIZE(pipeName), L"\\\\.\\pipe\\FASTFETCH-%u-%u", GetCurrentProcessId(), ++pidCounter); FF_AUTO_CLOSE_FD HANDLE hChildPipeRead = CreateNamedPipeW( pipeName, @@ -82,52 +83,67 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use return "CreateFileW(L\"\\\\.\\pipe\\FASTFETCH-$(PID)\") failed"; PROCESS_INFORMATION piProcInfo = {}; - - BOOL success; - + STARTUPINFOA siStartInfo = { + .cb = sizeof(siStartInfo), + .dwFlags = STARTF_USESTDHANDLES, + }; + if (useStdErr) { - STARTUPINFOA siStartInfo = { - .cb = sizeof(siStartInfo), - .dwFlags = STARTF_USESTDHANDLES, - }; - if (useStdErr) - { - siStartInfo.hStdOutput = ffGetNullFD(); - siStartInfo.hStdError = hChildPipeWrite; - } - else - { - siStartInfo.hStdOutput = hChildPipeWrite; - siStartInfo.hStdError = ffGetNullFD(); - } - - FF_STRBUF_AUTO_DESTROY cmdline = ffStrbufCreate(); - argvToCmdline(argv, &cmdline); - - success = CreateProcessA( - NULL, // application name - cmdline.chars, // command line - NULL, // process security attributes - NULL, // primary thread security attributes - TRUE, // handles are inherited - 0, // creation flags - NULL, // use parent's environment - NULL, // use parent's current directory - &siStartInfo, // STARTUPINFO pointer - &piProcInfo // receives PROCESS_INFORMATION - ); + siStartInfo.hStdOutput = ffGetNullFD(); + siStartInfo.hStdError = hChildPipeWrite; } + else + { + siStartInfo.hStdOutput = hChildPipeWrite; + siStartInfo.hStdError = ffGetNullFD(); + } + + FF_STRBUF_AUTO_DESTROY cmdline = ffStrbufCreate(); + argvToCmdline(argv, &cmdline); + + BOOL success = CreateProcessA( + NULL, // application name + cmdline.chars, // command line + NULL, // process security attributes + NULL, // primary thread security attributes + TRUE, // handles are inherited + 0, // creation flags + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFO pointer + &piProcInfo // receives PROCESS_INFORMATION + ); CloseHandle(hChildPipeWrite); if(!success) + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + return "command not found"; return "CreateProcessA() failed"; + } - FF_AUTO_CLOSE_FD HANDLE hProcess = piProcInfo.hProcess; - FF_MAYBE_UNUSED FF_AUTO_CLOSE_FD HANDLE hThread = piProcInfo.hThread; + CloseHandle(piProcInfo.hThread); // we don't need the thread handle + outHandle->pid = piProcInfo.hProcess; + outHandle->pipeRead = hChildPipeRead; + hChildPipeRead = INVALID_HANDLE_VALUE; // ownership transferred, don't close it + + return NULL; +} + +const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer) +{ + assert(handle->pipeRead != INVALID_HANDLE_VALUE); + assert(handle->pid != INVALID_HANDLE_VALUE); + + int32_t timeout = instance.config.general.processingTimeout; + FF_AUTO_CLOSE_FD HANDLE hProcess = handle->pid; + FF_AUTO_CLOSE_FD HANDLE hChildPipeRead = handle->pipeRead; + handle->pid = INVALID_HANDLE_VALUE; + handle->pipeRead = INVALID_HANDLE_VALUE; char str[FF_PIPE_BUFSIZ]; DWORD nRead = 0; - OVERLAPPED overlapped = { }; + OVERLAPPED overlapped = {}; // ReadFile always completes synchronously if the pipe is not created with FILE_FLAG_OVERLAPPED do { @@ -164,7 +180,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use break; case ERROR_BROKEN_PIPE: - return NULL; + goto exit; default: CancelIo(hChildPipeRead); @@ -175,6 +191,13 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use ffStrbufAppendNS(buffer, nRead, str); } while (nRead > 0); +exit: + { + DWORD exitCode = 0; + if (GetExitCodeProcess(hProcess, &exitCode) && exitCode != STILL_ACTIVE && exitCode != 0) + return "Child process exited with an error"; + } + return NULL; }