From 347ba44d6161f855c5869141fcb5123277163763 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 24 Apr 2024 10:33:27 -0400 Subject: [PATCH 1/4] Add a `Pipe` type. This PR adds a move-only wrapper for POSIX and Windows anonymous pipes (i.e. those created with `pipe(2)`.) We will need pipes in order to implement platform-independent interprocess communication such as that required by exit tests or future adoption of distributed actors. --- Sources/Testing/Support/FileHandle.swift | 108 ++++++++++++++++++ .../Support/FileHandleTests.swift | 21 +--- 2 files changed, 111 insertions(+), 18 deletions(-) diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index 2e1c67841..23b29a815 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -120,12 +120,51 @@ struct FileHandle: ~Copyable, Sendable { _closeWhenDone = closeWhenDone } + /// Initialize an instance of this type with an existing POSIX file descriptor + /// for reading. + /// + /// - Parameters: + /// - fd: The POSIX file descriptor to wrap. The caller is responsible for + /// ensuring that this file handle is open in the expected mode and that + /// another part of the system won't close it. + /// - mode: The mode `fd` was opened with, such as `"wb"`. + /// + /// - Throws: Any error preventing the stream from being opened. + /// + /// The resulting file handle takes ownership of `fd` and closes it when it is + /// deinitialized or if an error is thrown from this initializer. + init(unsafePOSIXFileDescriptor fd: CInt, mode: String) throws { +#if os(Windows) + let fileHandle = _fdopen(fd, mode) +#else + let fileHandle = fdopen(fd, mode) +#endif + guard let fileHandle else { +#if os(Windows) + _close(fd) +#else + _TestingInternals.close(fd) +#endif + throw CError(rawValue: swt_errno()) + } + self.init(unsafeCFILEHandle: fileHandle, closeWhenDone: true) + } + deinit { if _closeWhenDone { fclose(_fileHandle) } } + /// Close this file handle. + /// + /// This function effectively deinitializes the file handle. If + /// `closeWhenDone` was `false` when this file handle was initialized, this + /// function has no effect. + consuming func close() { + _ = (consume self) + } + /// Call a function and pass the underlying C file handle to it. /// /// - Parameters: @@ -383,6 +422,75 @@ extension FileHandle { } } +// MARK: - Pipes + +extension FileHandle { + /// A type representing a bidirectional pipe between two file handles. + struct Pipe: ~Copyable, Sendable { + /// The end of the pipe capable of reading. + var readEnd: FileHandle + + /// The end of the pipe capable of writing. + var writeEnd: FileHandle + + /// Initialize a new anonymous pipe. + /// + /// - Throws: Any error that prevented creation of the pipe. + init() throws { + let (fdReadEnd, fdWriteEnd) = try withUnsafeTemporaryAllocation(of: CInt.self, capacity: 2) { fds in +#if os(Windows) + guard 0 == _pipe(fds.baseAddress, 0, _O_BINARY) else { + throw CError(rawValue: swt_errno()) + } +#else + guard 0 == pipe(fds.baseAddress) else { + throw CError(rawValue: swt_errno()) + } +#endif + return (fds[0], fds[1]) + } + + // NOTE: Partial initialization of a move-only type is disallowed, as is + // conditional initialization of a local move-only value, which is why + // this section looks a little awkward. + let readEnd: FileHandle + do { + readEnd = try FileHandle(unsafePOSIXFileDescriptor: fdReadEnd, mode: "rb") + } catch { +#if os(Windows) + _close(fdWriteEnd) +#else + _TestingInternals.close(fdWriteEnd) +#endif + throw error + } + let writeEnd = try FileHandle(unsafePOSIXFileDescriptor: fdWriteEnd, mode: "wb") + self.readEnd = readEnd + self.writeEnd = writeEnd + } + + /// Close the read end of this pipe. + /// + /// - Returns: The remaining open end of the pipe. + /// + /// After calling this function, the read end is closed and the write end + /// remains open. + consuming func closeReadEnd() -> FileHandle { + (consume self).writeEnd + } + + /// Close the write end of this pipe. + /// + /// - Returns: The remaining open end of the pipe. + /// + /// After calling this function, the write end is closed and the read end + /// remains open. + consuming func closeWriteEnd() -> FileHandle { + (consume self).readEnd + } + } +} + // MARK: - Attributes extension FileHandle { diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index 977264010..ae2898599 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -132,24 +132,9 @@ struct FileHandleTests { @Test("Can recognize opened pipe") func isPipe() throws { -#if os(Windows) - var rHandle: HANDLE? - var wHandle: HANDLE? - try #require(CreatePipe(&rHandle, &wHandle, nil, 0)) - if let rHandle { - CloseHandle(rHandle) - } - let fdWrite = _open_osfhandle(intptr_t(bitPattern: wHandle), 0) - let file = try #require(_fdopen(fdWrite, "wb")) -#else - var fds: [CInt] = [-1, -1] - try #require(0 == pipe(&fds)) - try #require(fds[1] >= 0) - close(fds[0]) - let file = try #require(fdopen(fds[1], "wb")) -#endif - let fileHandle = FileHandle(unsafeCFILEHandle: file, closeWhenDone: true) - #expect(Bool(fileHandle.isPipe)) + let pipe = try FileHandle.Pipe() + #expect(pipe.readEnd.isPipe as Bool) + #expect(pipe.writeEnd.isPipe as Bool) } @Test("/dev/null is not a TTY or pipe") From 933b59d030b7946adf1be401f21fdb286b70d583 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 16 Sep 2024 16:27:40 -0400 Subject: [PATCH 2/4] Improve test coverage --- Sources/Testing/Support/FileHandle.swift | 20 +++++--- .../Support/FileHandleTests.swift | 49 +++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index 23b29a815..ba5857e5d 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -140,12 +140,13 @@ struct FileHandle: ~Copyable, Sendable { let fileHandle = fdopen(fd, mode) #endif guard let fileHandle else { + let errorCode = swt_errno() #if os(Windows) _close(fd) #else _TestingInternals.close(fd) #endif - throw CError(rawValue: swt_errno()) + throw CError(rawValue: errorCode) } self.init(unsafeCFILEHandle: fileHandle, closeWhenDone: true) } @@ -158,11 +159,14 @@ struct FileHandle: ~Copyable, Sendable { /// Close this file handle. /// - /// This function effectively deinitializes the file handle. If - /// `closeWhenDone` was `false` when this file handle was initialized, this - /// function has no effect. + /// This function effectively deinitializes the file handle. + /// + /// - Warning: This function closes the underlying C file handle even if + /// `closeWhenDone` was `false` when this instance was initialized. Callers + /// must take care not to close file handles they do not own. consuming func close() { - _ = (consume self) + fclose(_fileHandle) + _closeWhenDone = false } /// Call a function and pass the underlying C file handle to it. @@ -476,7 +480,8 @@ extension FileHandle { /// After calling this function, the read end is closed and the write end /// remains open. consuming func closeReadEnd() -> FileHandle { - (consume self).writeEnd + readEnd.close() + return writeEnd } /// Close the write end of this pipe. @@ -486,7 +491,8 @@ extension FileHandle { /// After calling this function, the write end is closed and the read end /// remains open. consuming func closeWriteEnd() -> FileHandle { - (consume self).readEnd + writeEnd.close() + return readEnd } } } diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index ae2898599..38ea019c9 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -44,6 +44,13 @@ struct FileHandleTests { } } + @Test("Init from invalid file descriptor") + func invalidFileDescriptor() throws { + #expect(throws: CError.self) { + _ = try FileHandle(unsafePOSIXFileDescriptor: -1, mode: "") + } + } + #if os(Windows) @Test("Can get Windows file HANDLE") func fileHANDLE() throws { @@ -54,6 +61,16 @@ struct FileHandleTests { } #endif +#if SWT_TARGET_OS_APPLE + @Test("close() function") + func closeFunction() async throws { + try await confirmation("File handle closed") { closed in + let fileHandle = try fileHandleForCloseMonitoring(with: closed) + fileHandle.close() + } + } +#endif + @Test("Can write to a file") func canWrite() throws { try withTemporaryPath { path in @@ -137,6 +154,19 @@ struct FileHandleTests { #expect(pipe.writeEnd.isPipe as Bool) } + @Test("Can close ends of a pipe") + func closeEndsOfPipe() async throws { + try await confirmation("File handle closed", expectedCount: 2) { closed in + var pipe1 = try FileHandle.Pipe() + pipe1.readEnd = try fileHandleForCloseMonitoring(with: closed) + _ = pipe1.closeReadEnd() + + var pipe2 = try FileHandle.Pipe() + pipe2.writeEnd = try fileHandleForCloseMonitoring(with: closed) + _ = pipe2.closeWriteEnd() + } + } + @Test("/dev/null is not a TTY or pipe") func devNull() throws { let fileHandle = try FileHandle.null(mode: "wb") @@ -224,4 +254,23 @@ func temporaryDirectory() throws -> String { #endif } +#if SWT_TARGET_OS_APPLE +func fileHandleForCloseMonitoring(with confirmation: Confirmation) throws -> FileHandle { + let context = Unmanaged.passRetained(confirmation as AnyObject).toOpaque() + let file = try #require( + funopen( + context, + { _, _, _ in 0 }, + nil, + nil, + { context in + let confirmation = Unmanaged.fromOpaque(context!).takeRetainedValue() as! Confirmation + confirmation() + return 0 + } + ) as SWT_FILEHandle? + ) + return FileHandle(unsafeCFILEHandle: file, closeWhenDone: false) +} +#endif #endif From 11550ed13be5b1badad81892ba38dbe77f0276cf Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 16 Sep 2024 16:36:01 -0400 Subject: [PATCH 3/4] Test works on Darwin only --- Tests/TestingTests/Support/FileHandleTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index 38ea019c9..feb0cc0de 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -154,6 +154,7 @@ struct FileHandleTests { #expect(pipe.writeEnd.isPipe as Bool) } +#if SWT_TARGET_OS_APPLE @Test("Can close ends of a pipe") func closeEndsOfPipe() async throws { try await confirmation("File handle closed", expectedCount: 2) { closed in @@ -166,6 +167,7 @@ struct FileHandleTests { _ = pipe2.closeWriteEnd() } } +#endif @Test("/dev/null is not a TTY or pipe") func devNull() throws { From b6972f75ae6a7dcbbd0c111e3b372ce1ecc46ac7 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 16 Sep 2024 17:03:09 -0400 Subject: [PATCH 4/4] Windows angry --- Sources/Testing/Support/FileHandle.swift | 3 +-- Tests/TestingTests/Support/FileHandleTests.swift | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index ba5857e5d..a4b179a3a 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -165,8 +165,7 @@ struct FileHandle: ~Copyable, Sendable { /// `closeWhenDone` was `false` when this instance was initialized. Callers /// must take care not to close file handles they do not own. consuming func close() { - fclose(_fileHandle) - _closeWhenDone = false + _closeWhenDone = true } /// Call a function and pass the underlying C file handle to it. diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index feb0cc0de..b352a1ec7 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -44,12 +44,14 @@ struct FileHandleTests { } } +#if !os(Windows) // Windows does not like invalid file descriptors. @Test("Init from invalid file descriptor") func invalidFileDescriptor() throws { #expect(throws: CError.self) { _ = try FileHandle(unsafePOSIXFileDescriptor: -1, mode: "") } } +#endif #if os(Windows) @Test("Can get Windows file HANDLE")