Skip to content

Commit f396a28

Browse files
authored
Revert "AsyncProcess: improve, fix & test fd close loop" (#226)
Reverting #223 due to failure on Amazon Linux 2: https://ci.swift.org/job/swift-PR-toolchain-amazonlinux2/82/console ``` 05:24:57 In file included from /home/build-user/swift-sdk-generator/Sources/CProcessSpawnSync/spawner.c:31: 05:24:57 /home/build-user/swift-sdk-generator/Sources/CProcessSpawnSync/internal-helpers.h:69:26: error: call to undeclared function 'getdents64'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 05:24:57 while ((bytes_read = getdents64(dir_fd, (struct dirent64 *)buffer, sizeof(buffer))) > 0) { 05:24:57 ^ 05:24:57 1 error generated. ```
1 parent 5f8246b commit f396a28

File tree

3 files changed

+2
-121
lines changed

3 files changed

+2
-121
lines changed

Sources/CProcessSpawnSync/internal-helpers.h

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
#ifndef INTERNAL_HELPERS_H
1414
#define INTERNAL_HELPERS_H
1515
#include <signal.h>
16-
#include <unistd.h>
17-
#include <fcntl.h>
1816

1917
static int positive_int_parse(const char *str) {
2018
int out = 0;
@@ -51,61 +49,14 @@ static int highest_possibly_open_fd_dir(const char *fd_dir) {
5149
return highest_fd_so_far;
5250
}
5351

54-
#if defined(__linux__)
55-
// Linux-specific version that uses syscalls directly and doesn't allocate heap memory.
56-
// Safe to use after vfork() and before execve()
57-
static int highest_possibly_open_fd_dir_linux(const char *fd_dir) {
58-
int highest_fd_so_far = 0;
59-
int dir_fd = open(fd_dir, O_RDONLY);
60-
if (dir_fd < 0) {
61-
// errno set by `open`.
62-
return -1;
63-
}
64-
65-
// Buffer for directory entries - allocated on stack, no heap allocation
66-
char buffer[4096] = {0};
67-
long bytes_read = -1;
68-
69-
while ((bytes_read = getdents64(dir_fd, (struct dirent64 *)buffer, sizeof(buffer))) > 0) {
70-
if (bytes_read < 0) {
71-
if (errno == EINTR) {
72-
continue;
73-
} else {
74-
// `errno` set by getdents64.
75-
highest_fd_so_far = -1;
76-
goto error;
77-
}
78-
}
79-
long offset = 0;
80-
while (offset < bytes_read) {
81-
struct dirent64 *entry = (struct dirent64 *)(buffer + offset);
82-
83-
// Skip "." and ".." entries
84-
if (entry->d_name[0] != '.') {
85-
int number = positive_int_parse(entry->d_name);
86-
if (number > highest_fd_so_far) {
87-
highest_fd_so_far = number;
88-
}
89-
}
90-
91-
offset += entry->d_reclen;
92-
}
93-
}
94-
95-
error:
96-
close(dir_fd);
97-
return highest_fd_so_far;
98-
}
99-
#endif
100-
10152
static int highest_possibly_open_fd(void) {
10253
#if defined(__APPLE__)
10354
int hi = highest_possibly_open_fd_dir("/dev/fd");
10455
if (hi < 0) {
10556
hi = getdtablesize();
10657
}
10758
#elif defined(__linux__)
108-
int hi = highest_possibly_open_fd_dir_linux("/proc/self/fd");
59+
int hi = highest_possibly_open_fd_dir("/proc/self/fd");
10960
if (hi < 0) {
11061
hi = getdtablesize();
11162
}

Sources/CProcessSpawnSync/spawner.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ static void setup_and_execve_child(ps_process_configuration *config, int error_p
169169
#endif
170170
if (close_range_err) {
171171
// close_range failed (or doesn't exist), let's fall back onto this
172-
for (int i=config->psc_fd_setup_count; i<=highest_possibly_open_fd(); i++) {
172+
for (int i=config->psc_fd_setup_count; i<highest_possibly_open_fd(); i++) {
173173
if (i != error_pipe) {
174174
close(i);
175175
}

Tests/AsyncProcessTests/IntegrationTests.swift

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,76 +1333,6 @@ final class IntegrationTests: XCTestCase {
13331333
}
13341334
}
13351335

1336-
func testVeryHighFDs() async throws {
1337-
var openedFDs: [CInt] = []
1338-
1339-
// Open /dev/null to use as source for duplication
1340-
let devNullFD = open("/dev/null", O_RDONLY)
1341-
guard devNullFD != -1 else {
1342-
XCTFail("Failed to open /dev/null")
1343-
return
1344-
}
1345-
defer {
1346-
let closeResult = close(devNullFD)
1347-
XCTAssertEqual(0, closeResult, "Failed to close /dev/null FD")
1348-
}
1349-
1350-
for candidate in sequence(first: CInt(1), next: { $0 <= CInt.max / 2 ? $0 * 2 : nil }) {
1351-
// Use fcntl with F_DUPFD to find next available FD >= candidate
1352-
let fd = fcntl(devNullFD, F_DUPFD, candidate)
1353-
if fd == -1 {
1354-
// Failed to allocate FD >= candidate, try next power of 2
1355-
self.logger.debug(
1356-
"already unavailable, skipping",
1357-
metadata: ["candidate": "\(candidate)", "errno": "\(errno)"]
1358-
)
1359-
continue
1360-
} else {
1361-
openedFDs.append(fd)
1362-
self.logger.debug("Opened FD in parent", metadata: ["fd": "\(fd)"])
1363-
}
1364-
}
1365-
1366-
defer {
1367-
for fd in openedFDs {
1368-
let closeResult = close(fd)
1369-
XCTAssertEqual(0, closeResult, "Failed to close FD \(fd)")
1370-
}
1371-
}
1372-
1373-
// Create shell script that checks each FD passed as arguments
1374-
let shellScript = """
1375-
for fd in "$@"; do
1376-
if [ -e "/proc/self/fd/$fd" ] || [ -e "/dev/fd/$fd" ]; then
1377-
echo "- fd: $fd: OPEN"
1378-
else
1379-
echo "- fd: $fd: CLOSED"
1380-
fi
1381-
done
1382-
"""
1383-
1384-
var arguments = ["-c", shellScript, "--"]
1385-
arguments.append(contentsOf: openedFDs.map { "\($0)" })
1386-
1387-
let result = try await ProcessExecutor.runCollectingOutput(
1388-
group: self.group,
1389-
executable: "/bin/sh",
1390-
arguments,
1391-
standardInput: EOFSequence(),
1392-
collectStandardOutput: true,
1393-
collectStandardError: true,
1394-
logger: self.logger
1395-
)
1396-
try result.exitReason.throwIfNonZero()
1397-
1398-
// Assert stderr is empty
1399-
XCTAssertEqual("", String(buffer: result.standardError!))
1400-
1401-
// Assert stdout contains exactly the expected output (all FDs closed)
1402-
let expectedOutput = openedFDs.map { "- fd: \($0): CLOSED" }.joined(separator: "\n") + "\n"
1403-
XCTAssertEqual(expectedOutput, String(buffer: result.standardOutput!))
1404-
}
1405-
14061336
// MARK: - Setup/teardown
14071337
override func setUp() async throws {
14081338
self.group = MultiThreadedEventLoopGroup(numberOfThreads: 3)

0 commit comments

Comments
 (0)