From 5c6b3f97201b27efe2c36ae31fb078f8360ef076 Mon Sep 17 00:00:00 2001 From: sb123sb123 <152394158+sb123sb123@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:56:00 +0800 Subject: [PATCH] Command (macOS): returns output after daemonized child exits --- CHANGELOG.md | 5 ++ CMakeLists.txt | 13 +++ src/common/impl/processing_linux.c | 121 ++++++++++++++++++------- tests/processing.c | 137 +++++++++++++++++++++++++++++ 4 files changed, 242 insertions(+), 34 deletions(-) create mode 100644 tests/processing.c diff --git a/CHANGELOG.md b/CHANGELOG.md index 609c5f1ae4..2e58b71ad9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# Unreleased + +Bugfixes: +* Fixed the Command module timing out when a command exits after spawning a background process that keeps its output pipe open. (Command, macOS) + # 2.70.0 Changes: diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e30560ecd..500cb5e994 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2263,6 +2263,15 @@ endif() ################### if (BUILD_TESTS) + if (NOT WIN32) + add_executable(fastfetch-test-processing + tests/processing.c + ) + target_link_libraries(fastfetch-test-processing + PRIVATE libfastfetch Threads::Threads + ) + endif() + add_executable(fastfetch-test-cache tests/cache.c ) @@ -2390,6 +2399,10 @@ if (BUILD_TESTS) ) enable_testing() + if (NOT WIN32) + add_test(NAME test-processing COMMAND fastfetch-test-processing) + set_tests_properties(test-processing PROPERTIES TIMEOUT 5) + endif() add_test(NAME test-cache COMMAND fastfetch-test-cache) add_test(NAME test-logo COMMAND fastfetch-test-logo) add_test(NAME test-strbuf COMMAND fastfetch-test-strbuf) diff --git a/src/common/impl/processing_linux.c b/src/common/impl/processing_linux.c index e9989acc6a..3969731ec2 100644 --- a/src/common/impl/processing_linux.c +++ b/src/common/impl/processing_linux.c @@ -4,6 +4,7 @@ #include "common/io.h" #include "common/strutil.h" #include "common/mallocHelper.h" +#include "common/time.h" #include #include @@ -167,6 +168,18 @@ const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFNativeFD stdinF return nullptr; } +static const char* ffProcessCheckExitStatus(int stat_loc) { + if (!WIFEXITED(stat_loc)) { + return "child process exited abnormally"; + } + if (WEXITSTATUS(stat_loc) == 127) { + FF_DEBUG("command not found"); + return "command not found"; + } + // We only handle 127 as an error. See `getTerminalVersionUrxvt` in `terminalshell.c` + return nullptr; +} + const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer) { assert(handle->pipeRead != -1); assert(handle->pid != -1); @@ -177,49 +190,89 @@ const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer) { handle->pipeRead = -1; handle->pid = -1; char str[FF_PIPE_BUFSIZ]; + double lastOutputTick = ffTimeGetTick(); + bool childExited = false; + int stat_loc = 0; + enum { FF_PROCESS_POLL_INTERVAL = 50 }; for (;;) { - if (timeout >= 0) { - struct pollfd pollfd = { childPipeFd, POLLIN, 0 }; - int pollret = poll(&pollfd, 1, timeout); - if (pollret == 0) { - FF_DEBUG("poll(&pollfd, 1, timeout) timeout (try increasing --processing-timeout)"); - kill(childPid, SIGTERM); - waitpid(childPid, nullptr, 0); - return "poll(&pollfd, 1, timeout) timeout (try increasing --processing-timeout)"; - } else if (pollret < 0 || (pollfd.revents & POLLERR)) { - kill(childPid, SIGTERM); - waitpid(childPid, nullptr, 0); - return pollret < 0 - ? "poll(&pollfd, 1, timeout) error: pollret < 0" - : "poll(&pollfd, 1, timeout) error: pollfd.revents & POLLERR"; + // poll() has no portable way to watch a child process on every supported POSIX system. + // Use short slices so a child that exits while a daemonized descendant still holds the + // output pipe open can be observed without process-global signal handling. + int pollTimeout = FF_PROCESS_POLL_INTERVAL; + if (childExited) { + pollTimeout = 0; + } else if (timeout >= 0) { + double remaining = (double) timeout - (ffTimeGetTick() - lastOutputTick); + if (remaining <= 0) { + pollTimeout = 0; + } else if (remaining < FF_PROCESS_POLL_INTERVAL) { + pollTimeout = (int) remaining; } } - ssize_t nRead = read(childPipeFd, str, FF_PIPE_BUFSIZ); - if (nRead > 0) { - ffStrbufAppendNS(buffer, (uint32_t) nRead, str); - } else if (nRead == 0) { - int stat_loc = 0; - if (childPid > 0 && waitpid(childPid, &stat_loc, 0) == childPid) { - if (!WIFEXITED(stat_loc)) { - return "child process exited abnormally"; - } - if (WEXITSTATUS(stat_loc) == 127) { - FF_DEBUG("command not found"); - return "command not found"; + struct pollfd pollfd = { childPipeFd, POLLIN, 0 }; + int pollret = poll(&pollfd, 1, pollTimeout); + bool pollInterrupted = pollret < 0 && errno == EINTR; + if (pollret < 0 && !pollInterrupted) { + kill(childPid, SIGTERM); + while (waitpid(childPid, nullptr, 0) < 0 && errno == EINTR) {} + return "poll(&pollfd, 1, timeout) error: pollret < 0"; + } + if (pollret > 0 && (pollfd.revents & (POLLERR | POLLNVAL))) { + kill(childPid, SIGTERM); + while (waitpid(childPid, nullptr, 0) < 0 && errno == EINTR) {} + return "poll(&pollfd, 1, timeout) error: pollfd.revents & POLLERR"; + } + + if (!childExited) { + pid_t waitResult; + do { + waitResult = waitpid(childPid, &stat_loc, WNOHANG); + } while (waitResult < 0 && errno == EINTR); + if (waitResult == childPid || (waitResult < 0 && errno == ECHILD)) { + childExited = true; + } else if (waitResult < 0) { + return "waitpid(childPid, &stat_loc, WNOHANG) failed"; + } + } + + if (pollInterrupted) { + continue; + } + + if (pollret > 0 && (pollfd.revents & (POLLIN | POLLHUP))) { + ssize_t nRead = read(childPipeFd, str, FF_PIPE_BUFSIZ); + if (nRead > 0) { + ffStrbufAppendNS(buffer, (uint32_t) nRead, str); + lastOutputTick = ffTimeGetTick(); + continue; + } else if (nRead == 0) { + if (!childExited && childPid > 0) { + pid_t waitResult; + do { + waitResult = waitpid(childPid, &stat_loc, 0); + } while (waitResult < 0 && errno == EINTR); + childExited = waitResult == childPid; } - // We only handle 127 as an error. See `getTerminalVersionUrxvt` in `terminalshell.c` - return nullptr; + return childExited ? ffProcessCheckExitStatus(stat_loc) : nullptr; + } else if (errno != EINTR && errno != EAGAIN && errno != EWOULDBLOCK) { + FF_DEBUG("read(childPipeFd, str, FF_PIPE_BUFSIZ) failed: %s", strerror(errno)); + return "read(childPipeFd, str, FF_PIPE_BUFSIZ) failed"; } - return nullptr; - } else if (nRead < 0) { - break; } - } - FF_DEBUG("read(childPipeFd, str, FF_PIPE_BUFSIZ) failed: %s", strerror(errno)); - return "read(childPipeFd, str, FF_PIPE_BUFSIZ) failed"; + if (childExited) { + return ffProcessCheckExitStatus(stat_loc); + } + + if (timeout >= 0 && ffTimeGetTick() - lastOutputTick >= (double) timeout) { + FF_DEBUG("poll(&pollfd, 1, timeout) timeout (try increasing --processing-timeout)"); + kill(childPid, SIGTERM); + while (waitpid(childPid, nullptr, 0) < 0 && errno == EINTR) {} + return "poll(&pollfd, 1, timeout) timeout (try increasing --processing-timeout)"; + } + } } void ffProcessGetInfoLinux(pid_t pid, FFstrbuf* processName, FFstrbuf* exe, const char** exeName, FFstrbuf* exePath) { diff --git a/tests/processing.c b/tests/processing.c new file mode 100644 index 0000000000..11520bdac2 --- /dev/null +++ b/tests/processing.c @@ -0,0 +1,137 @@ +#include "common/processing.h" +#include "fastfetch.h" + +#include +#include +#include +#include +#include + +typedef struct ProcessReadContext { + FFProcessHandle handle; + FFstrbuf output; + const char* error; +} ProcessReadContext; + +static void* readOutput(void* data) { + ProcessReadContext* context = data; + context->error = ffProcessReadOutput(&context->handle, &context->output); + return nullptr; +} + +static pid_t parseDescendantPid(const FFstrbuf* output) { + if (!output->chars || output->length == 0) { + return -1; + } + + char* end = nullptr; + long pid = strtol(output->chars, &end, 10); + if (end == output->chars || pid <= 0) { + return -1; + } + + return (pid_t) pid; +} + +static void stopDescendant(const FFstrbuf* output) { + pid_t pid = parseDescendantPid(output); + if (pid > 0) { + kill(pid, SIGKILL); + } +} + +int main(void) { + // The direct shell exits immediately, while its background sleep keeps the output pipe open. + char* const successArgv[] = { + "/bin/sh", + "-c", + "sleep 30 & printf '%s\\n' \"$!\"", + nullptr, + }; + char* const failureArgv[] = { + "/bin/sh", + "-c", + "sleep 30 & printf '%s\\n' \"$!\"; exit 127", + nullptr, + }; + const char* const expectedErrors[] = { nullptr, "command not found" }; + + instance.config.general.processingTimeout = 1000; + + ProcessReadContext contexts[2] = {}; + size_t spawned = 0; + bool spawnFailed = false; + for (; spawned < 2;) { + contexts[spawned].output = ffStrbufCreate(); + char* const* argv = spawned == 0 ? successArgv : failureArgv; + const char* error = ffProcessSpawn(argv, false, ffGetNullFD(), &contexts[spawned].handle); + if (error) { + fprintf(stderr, "ffProcessSpawn failed: %s\n", error); + ffStrbufDestroy(&contexts[spawned].output); + spawnFailed = true; + break; + } + spawned++; + } + + pthread_t threads[2]; + size_t started = 0; + if (!spawnFailed) { + for (; started < 2; started++) { + if (pthread_create(&threads[started], nullptr, readOutput, &contexts[started]) != 0) { + fprintf(stderr, "pthread_create failed\n"); + break; + } + } + } + + for (size_t i = started; i < spawned; i++) { + contexts[i].error = ffProcessReadOutput(&contexts[i].handle, &contexts[i].output); + } + for (size_t i = 0; i < started; i++) { + pthread_join(threads[i], nullptr); + } + + bool passed = !spawnFailed && started == 2; + for (size_t i = 0; i < spawned; i++) { + stopDescendant(&contexts[i].output); + bool errorMatches = expectedErrors[i] == nullptr + ? contexts[i].error == nullptr + : contexts[i].error && strcmp(contexts[i].error, expectedErrors[i]) == 0; + if (!errorMatches) { + if (contexts[i].error) { + fprintf(stderr, "ffProcessReadOutput returned an unexpected error: %s\n", contexts[i].error); + } else { + fprintf(stderr, "ffProcessReadOutput returned success unexpectedly\n"); + } + passed = false; + } + if (parseDescendantPid(&contexts[i].output) <= 0) { + fprintf(stderr, "child PID was not captured from command output\n"); + passed = false; + } + ffStrbufDestroy(&contexts[i].output); + } + + instance.config.general.processingTimeout = 100; + FFstrbuf timeoutOutput = ffStrbufCreate(); + FFProcessHandle timeoutHandle; + char* const timeoutArgv[] = { "/bin/sleep", "30", nullptr }; + const char* timeoutError = ffProcessSpawn(timeoutArgv, false, ffGetNullFD(), &timeoutHandle); + if (timeoutError == nullptr) { + timeoutError = ffProcessReadOutput(&timeoutHandle, &timeoutOutput); + } + if (!timeoutError + || strcmp(timeoutError, "poll(&pollfd, 1, timeout) timeout (try increasing --processing-timeout)") != 0) { + fprintf(stderr, "processing timeout was not reported\n"); + passed = false; + } + ffStrbufDestroy(&timeoutOutput); + + if (!passed) { + return 1; + } + + puts("All processing tests passed!"); + return 0; +}