From dfb5cfd688c17954f6978e64268d32a7d42ca5a9 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 15 Jul 2025 07:45:16 +0100 Subject: [PATCH] Revert "AsyncProcess: improve, fix & test fd close loop (#223)" This reverts commit 3f4a6a800414010da0057985f80b2aba5dd4fc60. --- Sources/CProcessSpawnSync/internal-helpers.h | 51 +------------- Sources/CProcessSpawnSync/spawner.c | 2 +- .../AsyncProcessTests/IntegrationTests.swift | 70 ------------------- 3 files changed, 2 insertions(+), 121 deletions(-) diff --git a/Sources/CProcessSpawnSync/internal-helpers.h b/Sources/CProcessSpawnSync/internal-helpers.h index a8e9e7e..b57f0f6 100644 --- a/Sources/CProcessSpawnSync/internal-helpers.h +++ b/Sources/CProcessSpawnSync/internal-helpers.h @@ -13,8 +13,6 @@ #ifndef INTERNAL_HELPERS_H #define INTERNAL_HELPERS_H #include -#include -#include static int positive_int_parse(const char *str) { int out = 0; @@ -51,53 +49,6 @@ static int highest_possibly_open_fd_dir(const char *fd_dir) { return highest_fd_so_far; } -#if defined(__linux__) -// Linux-specific version that uses syscalls directly and doesn't allocate heap memory. -// Safe to use after vfork() and before execve() -static int highest_possibly_open_fd_dir_linux(const char *fd_dir) { - int highest_fd_so_far = 0; - int dir_fd = open(fd_dir, O_RDONLY); - if (dir_fd < 0) { - // errno set by `open`. - return -1; - } - - // Buffer for directory entries - allocated on stack, no heap allocation - char buffer[4096] = {0}; - long bytes_read = -1; - - while ((bytes_read = getdents64(dir_fd, (struct dirent64 *)buffer, sizeof(buffer))) > 0) { - if (bytes_read < 0) { - if (errno == EINTR) { - continue; - } else { - // `errno` set by getdents64. - highest_fd_so_far = -1; - goto error; - } - } - long offset = 0; - while (offset < bytes_read) { - struct dirent64 *entry = (struct dirent64 *)(buffer + offset); - - // Skip "." and ".." entries - if (entry->d_name[0] != '.') { - int number = positive_int_parse(entry->d_name); - if (number > highest_fd_so_far) { - highest_fd_so_far = number; - } - } - - offset += entry->d_reclen; - } - } - -error: - close(dir_fd); - return highest_fd_so_far; -} -#endif - static int highest_possibly_open_fd(void) { #if defined(__APPLE__) int hi = highest_possibly_open_fd_dir("/dev/fd"); @@ -105,7 +56,7 @@ static int highest_possibly_open_fd(void) { hi = getdtablesize(); } #elif defined(__linux__) - int hi = highest_possibly_open_fd_dir_linux("/proc/self/fd"); + int hi = highest_possibly_open_fd_dir("/proc/self/fd"); if (hi < 0) { hi = getdtablesize(); } diff --git a/Sources/CProcessSpawnSync/spawner.c b/Sources/CProcessSpawnSync/spawner.c index ce20e79..315bb75 100644 --- a/Sources/CProcessSpawnSync/spawner.c +++ b/Sources/CProcessSpawnSync/spawner.c @@ -169,7 +169,7 @@ static void setup_and_execve_child(ps_process_configuration *config, int error_p #endif if (close_range_err) { // close_range failed (or doesn't exist), let's fall back onto this - for (int i=config->psc_fd_setup_count; i<=highest_possibly_open_fd(); i++) { + for (int i=config->psc_fd_setup_count; i= candidate - let fd = fcntl(devNullFD, F_DUPFD, candidate) - if fd == -1 { - // Failed to allocate FD >= candidate, try next power of 2 - self.logger.debug( - "already unavailable, skipping", - metadata: ["candidate": "\(candidate)", "errno": "\(errno)"] - ) - continue - } else { - openedFDs.append(fd) - self.logger.debug("Opened FD in parent", metadata: ["fd": "\(fd)"]) - } - } - - defer { - for fd in openedFDs { - let closeResult = close(fd) - XCTAssertEqual(0, closeResult, "Failed to close FD \(fd)") - } - } - - // Create shell script that checks each FD passed as arguments - let shellScript = """ - for fd in "$@"; do - if [ -e "/proc/self/fd/$fd" ] || [ -e "/dev/fd/$fd" ]; then - echo "- fd: $fd: OPEN" - else - echo "- fd: $fd: CLOSED" - fi - done - """ - - var arguments = ["-c", shellScript, "--"] - arguments.append(contentsOf: openedFDs.map { "\($0)" }) - - let result = try await ProcessExecutor.runCollectingOutput( - group: self.group, - executable: "/bin/sh", - arguments, - standardInput: EOFSequence(), - collectStandardOutput: true, - collectStandardError: true, - logger: self.logger - ) - try result.exitReason.throwIfNonZero() - - // Assert stderr is empty - XCTAssertEqual("", String(buffer: result.standardError!)) - - // Assert stdout contains exactly the expected output (all FDs closed) - let expectedOutput = openedFDs.map { "- fd: \($0): CLOSED" }.joined(separator: "\n") + "\n" - XCTAssertEqual(expectedOutput, String(buffer: result.standardOutput!)) - } - // MARK: - Setup/teardown override func setUp() async throws { self.group = MultiThreadedEventLoopGroup(numberOfThreads: 3)