From 196ceab4b21ba5b84eea522f9829db31b7b91f19 Mon Sep 17 00:00:00 2001 From: ser <122270051+ser-0xff@users.noreply.github.com> Date: Mon, 20 May 2024 19:52:05 +0300 Subject: [PATCH 01/16] Fix race in TCPThroughputBenchmark (#2724) --- .../TCPThroughputBenchmark.swift | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Sources/NIOPerformanceTester/TCPThroughputBenchmark.swift b/Sources/NIOPerformanceTester/TCPThroughputBenchmark.swift index 6318a40413..6886267df7 100644 --- a/Sources/NIOPerformanceTester/TCPThroughputBenchmark.swift +++ b/Sources/NIOPerformanceTester/TCPThroughputBenchmark.swift @@ -38,10 +38,16 @@ final class TCPThroughputBenchmark: Benchmark { public typealias InboundIn = ByteBuffer public typealias OutboundOut = ByteBuffer + private let connectionEstablishedPromise: EventLoopPromise private var context: ChannelHandlerContext! + init(_ connectionEstablishedPromise: EventLoopPromise) { + self.connectionEstablishedPromise = connectionEstablishedPromise + } + public func channelActive(context: ChannelHandlerContext) { self.context = context + connectionEstablishedPromise.succeed(context.eventLoop) } public func send(_ message: ByteBuffer, times count: Int) { @@ -106,12 +112,11 @@ final class TCPThroughputBenchmark: Benchmark { func setUp() throws { self.group = MultiThreadedEventLoopGroup(numberOfThreads: 4) - let connectionEstablished: EventLoopPromise = self.group.next().makePromise() + let connectionEstablishedPromise: EventLoopPromise = self.group.next().makePromise() self.serverChannel = try ServerBootstrap(group: self.group) .childChannelInitializer { channel in - self.serverHandler = ServerHandler() - connectionEstablished.succeed(channel.eventLoop) + self.serverHandler = ServerHandler(connectionEstablishedPromise) return channel.pipeline.addHandler(self.serverHandler) } .bind(host: "127.0.0.1", port: 0) @@ -134,7 +139,7 @@ final class TCPThroughputBenchmark: Benchmark { } self.message = message - self.serverEventLoop = try connectionEstablished.futureResult.wait() + self.serverEventLoop = try connectionEstablishedPromise.futureResult.wait() } func tearDown() { From 7d1b76d27b2b124cbd945c3b3cee62a0e79b94e7 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Thu, 23 May 2024 10:07:45 +0100 Subject: [PATCH 02/16] testSimpleMPTCP should not fail for ENOPROTOOPT (#2725) Motivation: MPTCP implementations are permitted to return `ENOPROTOOPT` if MPTCP has been disabled. This should not report as a test failure. Modifications: Instead of failing the test, log the permitted error and return. Result: No more misleading testSimpleMPTCP test failures --- Tests/NIOPosixTests/SocketChannelTest.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NIOPosixTests/SocketChannelTest.swift b/Tests/NIOPosixTests/SocketChannelTest.swift index 2b1b6c4dc0..c309302631 100644 --- a/Tests/NIOPosixTests/SocketChannelTest.swift +++ b/Tests/NIOPosixTests/SocketChannelTest.swift @@ -922,7 +922,7 @@ public final class SocketChannelTest : XCTestCase { .wait() } catch let error as IOError { // Older Linux kernel versions don't support MPTCP, which is fine. - if error.errnoCode != EINVAL && error.errnoCode != EPROTONOSUPPORT { + if error.errnoCode != EINVAL && error.errnoCode != EPROTONOSUPPORT && error.errnoCode != ENOPROTOOPT { XCTFail("Unexpected error: \(error)") } return From 4612941246f51ce3fb686418014e2cdd7de74c2c Mon Sep 17 00:00:00 2001 From: George Barnett Date: Thu, 23 May 2024 12:58:59 +0100 Subject: [PATCH 03/16] Remove storage indirection for FileSystemError (#2726) Motivation: Existentials are boxed if they are wider than 24 bytes. A number of value types in NIO are implemented with storage classes so that the alloaction is only paid for once. However, existential errors are treated differently, they are unconditionally boxed. Implementing errors with storage classes therefore introduces an unnecessary allocation. Modifications: - Remove the storage class for FileSystemError - Remove the copy-on-write test Result: Fewer allocations --- Sources/NIOFileSystem/FileSystemError.swift | 71 +++---------------- .../FileSystemErrorTests.swift | 35 --------- 2 files changed, 9 insertions(+), 97 deletions(-) diff --git a/Sources/NIOFileSystem/FileSystemError.swift b/Sources/NIOFileSystem/FileSystemError.swift index 633db54f31..2bfa20ea21 100644 --- a/Sources/NIOFileSystem/FileSystemError.swift +++ b/Sources/NIOFileSystem/FileSystemError.swift @@ -26,75 +26,19 @@ import SystemPackage /// /// Errors may have a ``FileSystemError/cause``, an underlying error which caused the operation to /// fail which may be platform specific. -public struct FileSystemError: Error, @unchecked Sendable { - // Note: @unchecked because we use a backing class for storage. - - private var storage: Storage - private mutating func ensureStorageIsUnique() { - if !isKnownUniquelyReferenced(&self.storage) { - self.storage = self.storage.copy() - } - } - - private final class Storage { - var code: Code - var message: String - var cause: Error? - var location: SourceLocation - - init(code: Code, message: String, cause: Error?, location: SourceLocation) { - self.code = code - self.message = message - self.cause = cause - self.location = location - } - - func copy() -> Self { - return Self( - code: self.code, - message: self.message, - cause: self.cause, - location: self.location - ) - } - } - +public struct FileSystemError: Error, Sendable { /// A high-level error code to provide broad a classification. - public var code: Code { - get { self.storage.code } - set { - self.ensureStorageIsUnique() - self.storage.code = newValue - } - } + public var code: Code /// A message describing what went wrong and how it may be remedied. - public var message: String { - get { self.storage.message } - set { - self.ensureStorageIsUnique() - self.storage.message = newValue - } - } + public var message: String /// An underlying error which caused the operation to fail. This may include additional details /// about the root cause of the failure. - public var cause: Error? { - get { self.storage.cause } - set { - self.ensureStorageIsUnique() - self.storage.cause = newValue - } - } + public var cause: Error? /// The location from which this error was thrown. - public var location: SourceLocation { - get { self.storage.location } - set { - self.ensureStorageIsUnique() - self.storage.location = newValue - } - } + public var location: SourceLocation public init( code: Code, @@ -102,7 +46,10 @@ public struct FileSystemError: Error, @unchecked Sendable { cause: Error?, location: SourceLocation ) { - self.storage = Storage(code: code, message: message, cause: cause, location: location) + self.code = code + self.message = message + self.cause = cause + self.location = location } /// Creates a ``FileSystemError`` by wrapping the given `cause` and its location and code. diff --git a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift index b6f2c36cec..565c0930f0 100644 --- a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift +++ b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift @@ -147,41 +147,6 @@ final class FileSystemErrorTests: XCTestCase { ) } - @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) - func testCopyOnWrite() throws { - let error1 = FileSystemError( - code: .io, - message: "a message", - cause: nil, - location: .init(function: "fn(_:)", file: "file.swift", line: 42) - ) - - var error2 = error1 - error2.code = .invalidArgument - XCTAssertEqual(error1.code, .io) - XCTAssertEqual(error2.code, .invalidArgument) - - var error3 = error1 - error3.message = "a different message" - XCTAssertEqual(error1.message, "a message") - XCTAssertEqual(error3.message, "a different message") - - var error4 = error1 - error4.cause = CancellationError() - XCTAssertNil(error1.cause) - XCTAssert(error4.cause is CancellationError) - - var error5 = error1 - error5.location.file = "different-file.swift" - XCTAssertEqual(error1.location.function, "fn(_:)") - XCTAssertEqual(error1.location.file, "file.swift") - XCTAssertEqual(error1.location.line, 42) - - XCTAssertEqual(error5.location.function, "fn(_:)") - XCTAssertEqual(error5.location.file, "different-file.swift") - XCTAssertEqual(error5.location.line, 42) - } - @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) func testErrorsMapToCorrectSyscallCause() throws { let here = FileSystemError.SourceLocation(function: "fn", file: "file", line: 42) From 9f63b12a05f79652ddbd901b97698b04ec37f20b Mon Sep 17 00:00:00 2001 From: George Barnett Date: Wed, 29 May 2024 14:17:52 +0100 Subject: [PATCH 04/16] Imrprove rename error (#2731) Motivation: The name of the syscall used in errors for all rename calls is "rename". However, this isn't always correct, in some cases "renamex_np" is used, and in others "renameat2" is used. Modifications: - Allow the name of the syscall to be provided to the rename error - Use the correct syscall name Result: Better errors --- Sources/NIOFileSystem/FileSystem.swift | 1 + Sources/NIOFileSystem/FileSystemError+Syscall.swift | 3 ++- Sources/NIOFileSystem/Internal/SystemFileHandle.swift | 4 ++++ Tests/NIOFileSystemTests/FileSystemErrorTests.swift | 4 ++-- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Sources/NIOFileSystem/FileSystem.swift b/Sources/NIOFileSystem/FileSystem.swift index 8faf2bbed1..20c431a8a6 100644 --- a/Sources/NIOFileSystem/FileSystem.swift +++ b/Sources/NIOFileSystem/FileSystem.swift @@ -1216,6 +1216,7 @@ extension FileSystem { case let .failure(errno): let error = FileSystemError.rename( + "rename", errno: errno, oldName: sourcePath, newName: destinationPath, diff --git a/Sources/NIOFileSystem/FileSystemError+Syscall.swift b/Sources/NIOFileSystem/FileSystemError+Syscall.swift index 33bba3ff99..8f9808c005 100644 --- a/Sources/NIOFileSystem/FileSystemError+Syscall.swift +++ b/Sources/NIOFileSystem/FileSystemError+Syscall.swift @@ -689,6 +689,7 @@ extension FileSystemError { @_spi(Testing) public static func rename( + _ name: String, errno: Errno, oldName: FilePath, newName: FilePath, @@ -739,7 +740,7 @@ extension FileSystemError { return FileSystemError( code: code, message: message, - systemCall: "rename", + systemCall: name, errno: errno, location: location ) diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index d080b39523..c56a24cf7b 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -804,7 +804,9 @@ extension SystemFileHandle.SendableView { case .rename: if materialize { let renameResult: Result + let renameFunction: String #if canImport(Darwin) + renameFunction = "renamex_np" renameResult = Syscall.rename( from: createdPath, to: desiredPath, @@ -814,6 +816,7 @@ extension SystemFileHandle.SendableView { // The created and desired paths are absolute, so the relative descriptors are // ignored. However, they must still be provided to 'rename' in order to pass // flags. + renameFunction = "renameat2" renameResult = Syscall.rename( from: createdPath, relativeTo: .currentWorkingDirectory, @@ -831,6 +834,7 @@ extension SystemFileHandle.SendableView { result = renameResult.mapError { errno in .rename( + renameFunction, errno: errno, oldName: createdPath, newName: desiredPath, diff --git a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift index 565c0930f0..308b21da80 100644 --- a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift +++ b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift @@ -230,7 +230,7 @@ final class FileSystemErrorTests: XCTestCase { } assertCauseIsSyscall("rename", here) { - .rename(errno: .badFileDescriptor, oldName: "old", newName: "new", location: here) + .rename("rename", errno: .badFileDescriptor, oldName: "old", newName: "new", location: here) } assertCauseIsSyscall("remove", here) { @@ -465,7 +465,7 @@ final class FileSystemErrorTests: XCTestCase { .ioError: .io, ] ) { errno in - .rename(errno: errno, oldName: "old", newName: "new", location: .fixed) + .rename("rename", errno: errno, oldName: "old", newName: "new", location: .fixed) } } From d8bf55d69347c5a92858f9768a462cbada8eb948 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Wed, 29 May 2024 14:59:06 +0100 Subject: [PATCH 05/16] Add a version of 'write' for 'ByteBuffer' (#2730) * Add a version of 'write' for 'ByteBuffer' Motivation: At the moment 'WritableFileHandleProtocol' is in terms of `some Sequence`. To use a `ByteBuffer` users must pass in the readable view of the buffer, which is inconvenient. Modifications: - Add an extension to `WritableFileHandleProtocol` which takes a `ByteBuffer`. Result: Easier to use API * fix docs --- .../Extensions/WritableFileHandleProtocol.md | 3 ++- .../NIOFileSystem/FileHandleProtocol.swift | 23 ++++++++++++++++++ .../FileHandleTests.swift | 24 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Sources/NIOFileSystem/Docs.docc/Extensions/WritableFileHandleProtocol.md b/Sources/NIOFileSystem/Docs.docc/Extensions/WritableFileHandleProtocol.md index f18de129da..74cbda5896 100644 --- a/Sources/NIOFileSystem/Docs.docc/Extensions/WritableFileHandleProtocol.md +++ b/Sources/NIOFileSystem/Docs.docc/Extensions/WritableFileHandleProtocol.md @@ -4,7 +4,8 @@ ### Write bytes to a file -- ``write(contentsOf:toAbsoluteOffset:)`` +- ``write(contentsOf:toAbsoluteOffset:)-57fnc`` +- ``write(contentsOf:toAbsoluteOffset:)-4na03`` - ``bufferedWriter(startingAtAbsoluteOffset:capacity:)`` ### Resize a file diff --git a/Sources/NIOFileSystem/FileHandleProtocol.swift b/Sources/NIOFileSystem/FileHandleProtocol.swift index 03044e5a48..372bf0b01c 100644 --- a/Sources/NIOFileSystem/FileHandleProtocol.swift +++ b/Sources/NIOFileSystem/FileHandleProtocol.swift @@ -465,6 +465,29 @@ public protocol WritableFileHandleProtocol: FileHandleProtocol { func close(makeChangesVisible: Bool) async throws } +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +extension WritableFileHandleProtocol { + /// Write the readable bytes of the `ByteBuffer` to the open file. + /// + /// - Important: This method checks whether the file is seekable or not (i.e., whether it's a socket, + /// pipe or FIFO), and will throw ``FileSystemError/Code-swift.struct/unsupported`` + /// if an offset other than zero is passed. + /// + /// - Parameters: + /// - buffer: The bytes to write. + /// - offset: The absolute offset into the file to write the bytes. + /// - Returns: The number of bytes written. + /// - Throws: ``FileSystemError/Code-swift.struct/unsupported`` if file is + /// unseekable and `offset` is not 0. + @discardableResult + public func write( + contentsOf buffer: ByteBuffer, + toAbsoluteOffset offset: Int64 + ) async throws -> Int64 { + try await self.write(contentsOf: buffer.readableBytesView, toAbsoluteOffset: offset) + } +} + /// A file handle which is suitable for reading and writing. @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) public typealias ReadableAndWritableFileHandleProtocol = ReadableFileHandleProtocol diff --git a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift index 9a5e845d6e..5531ea35cb 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift @@ -788,6 +788,30 @@ final class FileHandleTests: XCTestCase { } } + func testWriteByteBuffer() async throws { + try await withTemporaryFile { handle in + let someBytes = ByteBuffer(repeating: 42, count: 1024) + try await handle.write(contentsOf: someBytes, toAbsoluteOffset: 0) + + let readSomeBytes = try await handle.readToEnd(maximumSizeAllowed: .bytes(1024)) + XCTAssertEqual(readSomeBytes, someBytes) + + let moreBytes = ByteBuffer(repeating: 13, count: 1024) + try await handle.write(contentsOf: moreBytes, toAbsoluteOffset: 512) + + let readMoreBytes = try await handle.readToEnd( + fromAbsoluteOffset: 512, + maximumSizeAllowed: .bytes(1024) + ) + XCTAssertEqual(readMoreBytes, moreBytes) + + var allTheBytes = try await handle.readToEnd(maximumSizeAllowed: .bytes(1536)) + let firstBytes = allTheBytes.readSlice(length: 512)! + XCTAssertTrue(firstBytes.readableBytesView.allSatisfy({ $0 == 42 })) + XCTAssertTrue(allTheBytes.readableBytesView.allSatisfy({ $0 == 13 })) + } + } + func testWriteSequenceOfBytes() async throws { try await withTemporaryFile { handle in let byteSequence = UInt8(0).. Date: Thu, 30 May 2024 10:57:36 +0100 Subject: [PATCH 06/16] Release file handles back to caller on failure to take ownership (#2715) Motivation: When taking ownership of input and output file descriptors in a bootstrap the ownership of the file handles remains with the caller in the function taking ownership fails. This is currently where _takingOwnershipOfDescriptors uses NIOFileHandle to take ownership of descriptors but then experiences a later failure. Modifications: If an exception is throw in _takingOwnershipOfDescriptors release file descriptors from NIOFileHandle. Result: No crash on failure to close file handles before end of scoped usage. --- Sources/NIOPosix/Bootstrap.swift | 45 ++++++++++++++++++++++--- Tests/NIOPosixTests/BootstrapTest.swift | 24 +++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index 3b565f1b71..c99be2ccfd 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -1926,6 +1926,7 @@ public final class NIOPipeBootstrap { private var channelInitializer: Optional @usableFromInline internal var _channelOptions: ChannelOptions.Storage + private let hooks: any NIOPipeBootstrapHooks /// Create a `NIOPipeBootstrap` on the `EventLoopGroup` `group`. /// @@ -1956,6 +1957,19 @@ public final class NIOPipeBootstrap { self._channelOptions = ChannelOptions.Storage() self.group = group self.channelInitializer = nil + self.hooks = DefaultNIOPipeBootstrapHooks() + } + + /// Initialiser for hooked testing + init?(validatingGroup group: EventLoopGroup, hooks: any NIOPipeBootstrapHooks) { + guard NIOOnSocketsBootstraps.isCompatible(group: group) else { + return nil + } + + self._channelOptions = ChannelOptions.Storage() + self.group = group + self.channelInitializer = nil + self.hooks = hooks } /// Initialize the connected `PipeChannel` with `initializer`. The most common task in initializer is to add @@ -2281,11 +2295,18 @@ extension NIOPipeBootstrap { inputFileHandle = input.flatMap { NIOFileHandle(descriptor: $0) } outputFileHandle = output.flatMap { NIOFileHandle(descriptor: $0) } - channel = try PipeChannel( - eventLoop: eventLoop as! SelectableEventLoop, - inputPipe: inputFileHandle, - outputPipe: outputFileHandle - ) + do { + channel = try self.hooks.makePipeChannel( + eventLoop: eventLoop as! SelectableEventLoop, + inputPipe: inputFileHandle, + outputPipe: outputFileHandle + ) + } catch { + // Release file handles back to the caller in case of failure. + _ = try? inputFileHandle?.takeDescriptorOwnership() + _ = try? outputFileHandle?.takeDescriptorOwnership() + throw error + } } catch { return eventLoop.makeFailedFuture(error) } @@ -2327,3 +2348,17 @@ extension NIOPipeBootstrap { @available(*, unavailable) extension NIOPipeBootstrap: Sendable {} + +protocol NIOPipeBootstrapHooks { + func makePipeChannel(eventLoop: SelectableEventLoop, + inputPipe: NIOFileHandle?, + outputPipe: NIOFileHandle?) throws -> PipeChannel +} + +fileprivate struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks { + func makePipeChannel(eventLoop: SelectableEventLoop, + inputPipe: NIOFileHandle?, + outputPipe: NIOFileHandle?) throws -> PipeChannel { + return try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe) + } +} diff --git a/Tests/NIOPosixTests/BootstrapTest.swift b/Tests/NIOPosixTests/BootstrapTest.swift index 0d9409f9dd..931f244c81 100644 --- a/Tests/NIOPosixTests/BootstrapTest.swift +++ b/Tests/NIOPosixTests/BootstrapTest.swift @@ -680,6 +680,30 @@ class BootstrapTest: XCTestCase { .wait()) } } + + // There was a bug where file handle ownership was not released when creating pipe channels failed. + func testReleaseFileHandleOnOwningFailure() { + struct NIOPipeBootstrapHooksChannelFail: NIOPipeBootstrapHooks { + func makePipeChannel(eventLoop: NIOPosix.SelectableEventLoop, inputPipe: NIOCore.NIOFileHandle?, outputPipe: NIOCore.NIOFileHandle?) throws -> NIOPosix.PipeChannel { + throw IOError(errnoCode: EBADF, reason: "testing") + } + } + + let sock = socket(NIOBSDSocket.ProtocolFamily.local.rawValue, NIOBSDSocket.SocketType.stream.rawValue, 0) + defer { + close(sock) + } + let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { + try! elg.syncShutdownGracefully() + } + + let bootstrap = NIOPipeBootstrap(validatingGroup: elg, hooks: NIOPipeBootstrapHooksChannelFail()) + XCTAssertNotNil(bootstrap) + + let channelFuture = bootstrap?.takingOwnershipOfDescriptor(inputOutput: sock) + XCTAssertThrowsError(try channelFuture?.wait()) + } } private final class WriteStringOnChannelActive: ChannelInboundHandler { From 9428f62793696d9a0cc1f26a63f63bb31da0516d Mon Sep 17 00:00:00 2001 From: George Barnett Date: Fri, 31 May 2024 09:30:50 +0100 Subject: [PATCH 07/16] Add a fallback path if renameat2 fails (#2733) Motivation: 'renameat2' can fail with EINVAL if the RENAME_NOREPLACE flag isn't supported. However, not all kernel versions support this flag which can result in an error when creating a file 'transactionally'. Using 'renameat2' only happens on Linux as a fallback when O_TMPFILE isn't available. Modifications: - On Linux, fallback to a combination of 'stat' and 'rename' if 'renameat2' fails with EINVAL. - Add a couple of tests Result: Files can still be created exclusively --- .../Internal/SystemFileHandle.swift | 91 +++++++++++++++---- .../FileHandleTests.swift | 49 ++++++++++ 2 files changed, 124 insertions(+), 16 deletions(-) diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index c56a24cf7b..54496673d3 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -650,7 +650,7 @@ extension SystemFileHandle.SendableView { } @_spi(Testing) - public func _close(materialize: Bool) -> Result { + public func _close(materialize: Bool, failRenameat2WithEINVAL: Bool = false) -> Result { let descriptor: FileDescriptor? = self.lifecycle.withLockedValue { lifecycle in switch lifecycle { case let .open(descriptor): @@ -666,7 +666,7 @@ extension SystemFileHandle.SendableView { } // Materialize then close. - let materializeResult = self._materialize(materialize, descriptor: descriptor) + let materializeResult = self._materialize(materialize, descriptor: descriptor, failRenameat2WithEINVAL: failRenameat2WithEINVAL) return Result { try descriptor.close() @@ -778,7 +778,8 @@ extension SystemFileHandle.SendableView { func _materialize( _ materialize: Bool, - descriptor: FileDescriptor + descriptor: FileDescriptor, + failRenameat2WithEINVAL: Bool ) -> Result { guard let materialization = self.materialization else { return .success(()) } @@ -803,7 +804,7 @@ extension SystemFileHandle.SendableView { case .rename: if materialize { - let renameResult: Result + var renameResult: Result let renameFunction: String #if canImport(Darwin) renameFunction = "renamex_np" @@ -817,19 +818,76 @@ extension SystemFileHandle.SendableView { // ignored. However, they must still be provided to 'rename' in order to pass // flags. renameFunction = "renameat2" - renameResult = Syscall.rename( - from: createdPath, - relativeTo: .currentWorkingDirectory, - to: desiredPath, - relativeTo: .currentWorkingDirectory, - flags: materialization.exclusive ? [.exclusive] : [] - ) + if materialization.exclusive, failRenameat2WithEINVAL { + renameResult = .failure(.invalidArgument) + } else { + renameResult = Syscall.rename( + from: createdPath, + relativeTo: .currentWorkingDirectory, + to: desiredPath, + relativeTo: .currentWorkingDirectory, + flags: materialization.exclusive ? [.exclusive] : [] + ) + } #endif - // A file exists at the desired path and the user specified exclusive creation, - // clear up by removing the file we did create. - if materialization.exclusive, case .failure(.fileExists) = renameResult { - _ = Libc.remove(createdPath) + if materialization.exclusive { + switch renameResult { + case .failure(.fileExists): + // A file exists at the desired path and the user specified exclusive + // creation, clear up by removing the file that we did create. + _ = Libc.remove(createdPath) + + case .failure(.invalidArgument): + // If 'renameat2' failed on Linux with EINVAL then in all likelihood the + // 'RENAME_NOREPLACE' option isn't supported. As we're doing an exclusive + // create, check the desired path doesn't exist then do a regular rename. + #if canImport(Glibc) || canImport(Musl) + switch Syscall.stat(path: desiredPath) { + case .failure(.noSuchFileOrDirectory): + // File doesn't exist, do a 'regular' rename. + renameResult = Syscall.rename(from: createdPath, to: desiredPath) + + case .success: + // File exists so exclusive create isn't possible. Remove the file + // we did create then throw. + _ = Libc.remove(createdPath) + let error = FileSystemError( + code: .fileAlreadyExists, + message: """ + Couldn't open '\(desiredPath)', it already exists and the \ + file was opened with the 'existingFile' option set to 'none'. + """, + cause: nil, + location: .here() + ) + return .failure(error) + + case .failure: + // Failed to stat the desired file for reasons unknown. Remove the file + // we did create then throw. + _ = Libc.remove(createdPath) + let error = FileSystemError( + code: .unknown, + message: "Couldn't open '\(desiredPath)'.", + cause: FileSystemError.rename( + "renameat2", + errno: .invalidArgument, + oldName: createdPath, + newName: desiredPath, + location: .here() + ), + location: .here() + ) + return .failure(error) + } + #else + () // Not Linux, use the normal error flow. + #endif + + case .success, .failure: + () + } } result = renameResult.mapError { errno in @@ -1238,7 +1296,8 @@ extension SystemFileHandle { } } - static func syncOpenWithMaterialization( + @_spi(Testing) + public static func syncOpenWithMaterialization( atPath path: FilePath, mode: FileDescriptor.AccessMode, options originalOptions: FileDescriptor.OpenOptions, diff --git a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift index 5531ea35cb..7356a9f12d 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift @@ -995,6 +995,55 @@ final class FileHandleTests: XCTestCase { } } + func testOpenExclusiveCreateForFileWhichExistsWithoutOTMPFILE() async throws { + // Takes the path where 'O_TMPFILE' doesn't exist, so materializing the file is done via + // creating a temporary file and then renaming it using 'renamex_np'/'renameat2' (Darwin/Linux). + let temporaryDirectory = try await FileSystem.shared.temporaryDirectory + let path = temporaryDirectory.appending(Self.temporaryFileName().components) + let handle = try SystemFileHandle.syncOpenWithMaterialization( + atPath: path, + mode: .writeOnly, + options: [.exclusiveCreate, .create], + permissions: .ownerReadWrite, + threadPool: .singleton, + useTemporaryFileIfPossible: false + ).get() + + // Closing shouldn't throw and the file should now be visible. + try await handle.close() + let info = try await FileSystem.shared.info(forFileAt: path) + XCTAssertNotNil(info) + } + + func testOpenExclusiveCreateForFileWhichExistsWithoutOTMPFILEOrRenameat2() async throws { + // Takes the path where 'O_TMPFILE' doesn't exist, so materializing the file is done via + // creating a temporary file and then renaming it using 'renameat2' and then takes a further + // fallback path where 'renameat2' returns EINVAL so the 'rename' is used in combination + // with 'stat'. This path is only reachable on Linux. + #if canImport(Glibc) || canImport(Musl) + let temporaryDirectory = try await FileSystem.shared.temporaryDirectory + let path = temporaryDirectory.appending(Self.temporaryFileName().components) + let handle = try SystemFileHandle.syncOpenWithMaterialization( + atPath: path, + mode: .writeOnly, + options: [.exclusiveCreate, .create], + permissions: .ownerReadWrite, + threadPool: .singleton, + useTemporaryFileIfPossible: false + ).get() + + // Close, but take the path where 'renameat2' fails with EINVAL. This shouldn't throw and + // the file should be available. + let result = handle.sendableView._close(materialize: true, failRenameat2WithEINVAL: true) + try result.get() + + let info = try await FileSystem.shared.info(forFileAt: path) + XCTAssertNotNil(info) + #else + throw XCTSkip("This test requires 'renameat2' which isn't supported on this platform") + #endif + } + func testOpenExclusiveCreateForFileWhichDoesNotExist() async throws { try await self.withTestDataDirectory { dir in let path = Self.temporaryFileName() From bc161803bb87cbe82f656dd4be1baf45307158b4 Mon Sep 17 00:00:00 2001 From: Gustavo Cairo Date: Mon, 10 Jun 2024 16:58:21 +0100 Subject: [PATCH 08/16] Add API for setting last accessed and last modified file times (#2735) This PR adds API to set a file's last accessed and last modified times, as well as a `touch()` shorthand that updates both to the current time. Motivation: Feature request: https://github.com/apple/swift-nio/issues/2712 Modifications: This PR adds API to set a file's last accessed and last modified times, as well as a `touch()` shorthand that updates both to the current time. Result: Files' last accessed and last modified times can be updated. --- Sources/CNIOLinux/include/CNIOLinux.h | 3 + Sources/CNIOLinux/shim.c | 4 + Sources/NIOFileSystem/FileHandle.swift | 50 ++++++++ .../NIOFileSystem/FileHandleProtocol.swift | 51 ++++++++ Sources/NIOFileSystem/FileInfo.swift | 25 ++++ .../FileSystemError+Syscall.swift | 38 ++++++ .../Internal/System Calls/Syscall.swift | 10 ++ .../Internal/System Calls/Syscalls.swift | 12 ++ .../Internal/SystemFileHandle.swift | 98 ++++++++++++++- .../FileHandleTests.swift | 115 ++++++++++++++++++ .../FileSystemErrorTests.swift | 13 ++ .../Internal/SyscallTests.swift | 16 +++ 12 files changed, 432 insertions(+), 3 deletions(-) diff --git a/Sources/CNIOLinux/include/CNIOLinux.h b/Sources/CNIOLinux/include/CNIOLinux.h index a75e07f63a..cde2942fde 100644 --- a/Sources/CNIOLinux/include/CNIOLinux.h +++ b/Sources/CNIOLinux/include/CNIOLinux.h @@ -133,5 +133,8 @@ extern const int CNIOLinux_AT_EMPTY_PATH; extern const unsigned int CNIOLinux_RENAME_NOREPLACE; extern const unsigned int CNIOLinux_RENAME_EXCHANGE; +extern const unsigned long CNIOLinux_UTIME_OMIT; +extern const unsigned long CNIOLinux_UTIME_NOW; + #endif #endif diff --git a/Sources/CNIOLinux/shim.c b/Sources/CNIOLinux/shim.c index dfe0076abb..832cef739e 100644 --- a/Sources/CNIOLinux/shim.c +++ b/Sources/CNIOLinux/shim.c @@ -33,6 +33,7 @@ void CNIOLinux_i_do_nothing_just_working_around_a_darwin_toolchain_bug(void) {} #include #include #include +#include _Static_assert(sizeof(CNIOLinux_mmsghdr) == sizeof(struct mmsghdr), "sizes of CNIOLinux_mmsghdr and struct mmsghdr differ"); @@ -211,4 +212,7 @@ const unsigned int CNIOLinux_RENAME_NOREPLACE = RENAME_NOREPLACE; const unsigned int CNIOLinux_RENAME_EXCHANGE = RENAME_EXCHANGE; const int CNIOLinux_AT_EMPTY_PATH = AT_EMPTY_PATH; +const unsigned long CNIOLinux_UTIME_OMIT = UTIME_OMIT; +const unsigned long CNIOLinux_UTIME_NOW = UTIME_NOW; + #endif diff --git a/Sources/NIOFileSystem/FileHandle.swift b/Sources/NIOFileSystem/FileHandle.swift index a59436dc5b..2f6f0ed47f 100644 --- a/Sources/NIOFileSystem/FileHandle.swift +++ b/Sources/NIOFileSystem/FileHandle.swift @@ -83,6 +83,16 @@ extension _HasFileHandle { public func close() async throws { try await self.fileHandle.close() } + + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.fileHandle.setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } } /// Implements ``FileHandleProtocol`` by making system calls to interact with the local file system. @@ -148,6 +158,16 @@ public struct FileHandle: FileHandleProtocol { public func close() async throws { try await self.systemFileHandle.close() } + + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.systemFileHandle.setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } } /// Implements ``ReadableFileHandleProtocol`` by making system calls to interact with the local @@ -173,6 +193,16 @@ public struct ReadFileHandle: ReadableFileHandleProtocol, _HasFileHandle { public func readChunks(in range: Range, chunkLength: ByteCount) -> FileChunks { self.fileHandle.systemFileHandle.readChunks(in: range, chunkLength: chunkLength) } + + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.fileHandle.systemFileHandle.setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } } /// Implements ``WritableFileHandleProtocol`` by making system calls to interact with the local @@ -203,6 +233,16 @@ public struct WriteFileHandle: WritableFileHandleProtocol, _HasFileHandle { public func close(makeChangesVisible: Bool) async throws { try await self.fileHandle.systemFileHandle.close(makeChangesVisible: makeChangesVisible) } + + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.fileHandle.systemFileHandle.setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } } /// Implements ``ReadableAndWritableFileHandleProtocol`` by making system calls to interact with the @@ -247,6 +287,16 @@ public struct ReadWriteFileHandle: ReadableAndWritableFileHandleProtocol, _HasFi public func close(makeChangesVisible: Bool) async throws { try await self.fileHandle.systemFileHandle.close(makeChangesVisible: makeChangesVisible) } + + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.fileHandle.systemFileHandle.setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } } /// Implements ``DirectoryFileHandleProtocol`` by making system calls to interact with the local diff --git a/Sources/NIOFileSystem/FileHandleProtocol.swift b/Sources/NIOFileSystem/FileHandleProtocol.swift index 372bf0b01c..e7607f7829 100644 --- a/Sources/NIOFileSystem/FileHandleProtocol.swift +++ b/Sources/NIOFileSystem/FileHandleProtocol.swift @@ -143,6 +143,27 @@ public protocol FileHandleProtocol { /// /// After closing the handle calls to other functions will throw an appropriate error. func close() async throws + + /// Sets the file's last access and last data modification times to the given values. + /// + /// If **either** time is `nil`, the current value will not be changed. + /// If **both** times are `nil`, then both times will be set to the current time. + /// + /// > Important: Times are only considered valid if their nanoseconds components are one of the following: + /// > - `UTIME_NOW` (you can use ``FileInfo/Timespec/now`` to get a Timespec set to this value), + /// > - `UTIME_OMIT` (you can use ``FileInfo/Timespec/omit`` to get a Timespec set to this value), + /// > - Greater than zero and no larger than 1000 million: if outside of this range, the value will be clamped to the closest valid value. + /// > The seconds component must also be positive: if it's not, zero will be used as the value instead. + /// + /// - Parameters: + /// - lastAccessTime: The new value of the file's last access time, as time elapsed since the Epoch. + /// - lastDataModificationTime: The new value of the file's last data modification time, as time elapsed since the Epoch. + /// + /// - Throws: If there's an error updating the times. If this happens, the original values won't be modified. + func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws } // MARK: - Readable @@ -488,6 +509,36 @@ extension WritableFileHandleProtocol { } } +// MARK: - File times modifiers + +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +extension FileHandleProtocol { + /// Sets the file's last access time to the given time. + /// + /// - Parameter time: The time to which the file's last access time should be set. + /// + /// - Throws: If there's an error updating the time. If this happens, the original value won't be modified. + public func setLastAccessTime(to time: FileInfo.Timespec) async throws { + try await self.setTimes(lastAccess: time, lastDataModification: nil) + } + + /// Sets the file's last data modification time to the given time. + /// + /// - Parameter time: The time to which the file's last data modification time should be set. + /// + /// - Throws: If there's an error updating the time. If this happens, the original value won't be modified. + public func setLastDataModificationTime(to time: FileInfo.Timespec) async throws { + try await self.setTimes(lastAccess: nil, lastDataModification: time) + } + + /// Sets the file's last access and last data modification times to the current time. + /// + /// - Throws: If there's an error updating the times. If this happens, the original values won't be modified. + public func touch() async throws { + try await self.setTimes(lastAccess: nil, lastDataModification: nil) + } +} + /// A file handle which is suitable for reading and writing. @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) public typealias ReadableAndWritableFileHandleProtocol = ReadableFileHandleProtocol diff --git a/Sources/NIOFileSystem/FileInfo.swift b/Sources/NIOFileSystem/FileInfo.swift index 7f174bf6d7..3df14db1c1 100644 --- a/Sources/NIOFileSystem/FileInfo.swift +++ b/Sources/NIOFileSystem/FileInfo.swift @@ -19,8 +19,10 @@ import SystemPackage import Darwin #elseif canImport(Glibc) import Glibc +import CNIOLinux #elseif canImport(Musl) import Musl +import CNIOLinux #endif /// Information about a file system object. @@ -145,6 +147,29 @@ extension FileInfo { /// A time interval consisting of whole seconds and nanoseconds. public struct Timespec: Hashable, Sendable { + #if canImport(Darwin) + private static let utimeOmit = Int(UTIME_OMIT) + private static let utimeNow = Int(UTIME_NOW) + #elseif canImport(Glibc) || canImport(Musl) + private static let utimeOmit = Int(CNIOLinux_UTIME_OMIT) + private static let utimeNow = Int(CNIOLinux_UTIME_NOW) + #endif + + /// A timespec where the seconds are set to zero and the nanoseconds set to `UTIME_OMIT`. + /// In syscalls such as `futimens`, this means the time component set to this value will be ignored. + public static let omit = Self( + seconds: 0, + nanoseconds: Self.utimeOmit + ) + + /// A timespec where the seconds are set to zero and the nanoseconds set to `UTIME_NOW`. + /// In syscalls such as `futimens`, this means the time component set to this value will be + /// be set to the current time or the largest value supported by the platform, whichever is smaller. + public static let now = Self( + seconds: 0, + nanoseconds: Self.utimeNow + ) + /// The number of seconds. public var seconds: Int diff --git a/Sources/NIOFileSystem/FileSystemError+Syscall.swift b/Sources/NIOFileSystem/FileSystemError+Syscall.swift index 8f9808c005..a59185bb25 100644 --- a/Sources/NIOFileSystem/FileSystemError+Syscall.swift +++ b/Sources/NIOFileSystem/FileSystemError+Syscall.swift @@ -1066,6 +1066,44 @@ extension FileSystemError { location: location ) } + + @_spi(Testing) + public static func futimens( + errno: Errno, + path: FilePath, + lastAccessTime: FileInfo.Timespec?, + lastDataModificationTime: FileInfo.Timespec?, + location: SourceLocation + ) -> FileSystemError { + let code: FileSystemError.Code + let message: String + + switch errno { + case .permissionDenied, .notPermitted: + code = .permissionDenied + message = "Not permitted to change last access or last data modification times for \(path)." + + case .readOnlyFileSystem: + code = .unsupported + message = "Not permitted to change last access or last data modification times for \(path): this is a read-only file system." + + case .badFileDescriptor: + code = .closed + message = "Could not change last access or last data modification dates for \(path): file is closed." + + default: + code = .unknown + message = "Could not change last access or last data modification dates for \(path)." + } + + return FileSystemError( + code: code, + message: message, + systemCall: "futimens", + errno: errno, + location: location + ) + } } #endif diff --git a/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift b/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift index 06b2689165..308c239c0f 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift @@ -266,6 +266,16 @@ public enum Syscall { } } #endif + + @_spi(Testing) + public static func futimens( + fileDescriptor fd: FileDescriptor, + times: UnsafePointer? + ) -> Result { + nothingOrErrno(retryOnInterrupt: false) { + system_futimens(fd.rawValue, times) + } + } } @_spi(Testing) diff --git a/Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift b/Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift index 120500c27f..284d59d779 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift @@ -349,6 +349,18 @@ internal func system_sendfile( } #endif +internal func system_futimens( + _ fd: CInt, + _ times: UnsafePointer? +) -> CInt { + #if ENABLE_MOCKING + if mockingEnabled { + return mock(fd, times) + } + #endif + return futimens(fd, times) +} + // MARK: - libc /// fdopendir(3): Opens a directory stream for the file descriptor diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index 54496673d3..34c8f62353 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -22,8 +22,10 @@ import NIOPosix import Darwin #elseif canImport(Glibc) import Glibc +import CNIOLinux #elseif canImport(Musl) import Musl +import CNIOLinux #endif /// An implementation of ``FileHandleProtocol`` which is backed by system calls and a file @@ -153,7 +155,7 @@ extension SystemFileHandle.SendableView { } } - /// Executes a closure with the file descriptor it it's available otherwise throws the result + /// Executes a closure with the file descriptor if it's available otherwise throws the result /// of `onUnavailable`. internal func _withUnsafeDescriptor( _ execute: (FileDescriptor) throws -> R, @@ -166,8 +168,8 @@ extension SystemFileHandle.SendableView { } } - /// Executes a closure with the file descriptor it it's available otherwise throws the result - /// of `onUnavailable`. + /// Executes a closure with the file descriptor if it's available otherwise returns the result + /// of `onUnavailable` as a `Result` Error. internal func _withUnsafeDescriptorResult( _ execute: (FileDescriptor) -> Result, onUnavailable: () -> FileSystemError @@ -355,6 +357,18 @@ extension SystemFileHandle: FileHandleProtocol { } } + public func setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) async throws { + try await self.threadPool.runIfActive { [sendableView] in + try sendableView._setTimes( + lastAccess: lastAccess, + lastDataModification: lastDataModification + ) + } + } + @_spi(Testing) public enum UpdatePermissionsOperation { case set, add, remove } } @@ -909,6 +923,84 @@ extension SystemFileHandle.SendableView { return result } + + func _setTimes( + lastAccess: FileInfo.Timespec?, + lastDataModification: FileInfo.Timespec? + ) throws { + try self._withUnsafeDescriptor { descriptor in + let syscallResult: Result + switch (lastAccess, lastDataModification) { + case (.none, .none): + // If the timespec array is nil, as per the `futimens` docs, + // both the last accessed and last modification times + // will be set to now. + syscallResult = Syscall.futimens( + fileDescriptor: descriptor, + times: nil + ) + + case (.some(let lastAccess), .none): + // Don't modify the last modification time. + syscallResult = Syscall.futimens( + fileDescriptor: descriptor, + times: [timespec(lastAccess), timespec(.omit)] + ) + + case (.none, .some(let lastDataModification)): + // Don't modify the last access time. + syscallResult = Syscall.futimens( + fileDescriptor: descriptor, + times: [timespec(.omit), timespec(lastDataModification)] + ) + + case (.some(let lastAccess), .some(let lastDataModification)): + syscallResult = Syscall.futimens( + fileDescriptor: descriptor, + times: [timespec(lastAccess), timespec(lastDataModification)] + ) + } + + try syscallResult.mapError { errno in + FileSystemError.futimens( + errno: errno, + path: self.path, + lastAccessTime: lastAccess, + lastDataModificationTime: lastDataModification, + location: .here() + ) + }.get() + } onUnavailable: { + FileSystemError( + code: .closed, + message: "Couldn't modify file dates, the file '\(self.path)' is closed.", + cause: nil, + location: .here() + ) + } + } +} + +extension timespec { + fileprivate init(_ fileinfoTimespec: FileInfo.Timespec) { + // Clamp seconds to be positive + let seconds = max(0, fileinfoTimespec.seconds) + + // If nanoseconds are not UTIME_NOW or UTIME_OMIT, clamp to be between + // 0 and 1,000 million. + let nanoseconds: Int + switch fileinfoTimespec { + case .now, .omit: + nanoseconds = fileinfoTimespec.nanoseconds + default: + nanoseconds = min(1_000_000_000, max(0, fileinfoTimespec.nanoseconds)) + } + + self.init( + tv_sec: seconds, + tv_nsec: nanoseconds + ) + } } // MARK: - Readable File Handle diff --git a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift index 7356a9f12d..ffcd85af17 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift @@ -1178,6 +1178,121 @@ final class FileHandleTests: XCTestCase { } } } + + func testSetLastAccesTime() async throws { + try await self.withTemporaryFile { handle in + let originalLastDataModificationTime = try await handle.info().lastDataModificationTime + let originalLastAccessTime = try await handle.info().lastAccessTime + + try await handle.setTimes( + lastAccess: FileInfo.Timespec(seconds: 10, nanoseconds: 5), + lastDataModification: nil + ) + + let actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(actualLastAccessTime, FileInfo.Timespec(seconds: 10, nanoseconds: 5)) + XCTAssertNotEqual(actualLastAccessTime, originalLastAccessTime) + + let actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(actualLastDataModificationTime, originalLastDataModificationTime) + } + } + + func testSetLastDataModificationTime() async throws { + try await self.withTemporaryFile { handle in + let originalLastDataModificationTime = try await handle.info().lastDataModificationTime + let originalLastAccessTime = try await handle.info().lastAccessTime + + try await handle.setTimes( + lastAccess: nil, + lastDataModification: FileInfo.Timespec(seconds: 10, nanoseconds: 5) + ) + + let actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(actualLastDataModificationTime, FileInfo.Timespec(seconds: 10, nanoseconds: 5)) + XCTAssertNotEqual(actualLastDataModificationTime, originalLastDataModificationTime) + + let actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(actualLastAccessTime, originalLastAccessTime) + } + } + + func testSetLastAccessAndLastDataModificationTimes() async throws { + try await self.withTemporaryFile { handle in + let originalLastDataModificationTime = try await handle.info().lastDataModificationTime + let originalLastAccessTime = try await handle.info().lastAccessTime + + try await handle.setTimes( + lastAccess: FileInfo.Timespec(seconds: 20, nanoseconds: 25), + lastDataModification: FileInfo.Timespec(seconds: 10, nanoseconds: 5) + ) + + let actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(actualLastAccessTime, FileInfo.Timespec(seconds: 20, nanoseconds: 25)) + XCTAssertNotEqual(actualLastAccessTime, originalLastAccessTime) + + let actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(actualLastDataModificationTime, FileInfo.Timespec(seconds: 10, nanoseconds: 5)) + XCTAssertNotEqual(actualLastDataModificationTime, originalLastDataModificationTime) + } + } + + func testSetLastAccessAndLastDataModificationTimesToNil() async throws { + try await self.withTemporaryFile { handle in + // Set some random value for both times, only to be overwritten by the current time + // right after. + try await handle.setTimes( + lastAccess: FileInfo.Timespec(seconds: 1, nanoseconds: 0), + lastDataModification: FileInfo.Timespec(seconds: 1, nanoseconds: 0) + ) + + var actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(actualLastAccessTime, FileInfo.Timespec(seconds: 1, nanoseconds: 0)) + + var actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(actualLastDataModificationTime, FileInfo.Timespec(seconds: 1, nanoseconds: 0)) + + try await handle.setTimes( + lastAccess: nil, + lastDataModification: nil + ) + let estimatedCurrentTime = Date.now.timeIntervalSince1970 + + // Assert that the times are equal to the current time, with up to a second difference (to avoid timing flakiness). + actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(Float(actualLastAccessTime.seconds), Float(estimatedCurrentTime), accuracy: 1) + + actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(Float(actualLastDataModificationTime.seconds), Float(estimatedCurrentTime), accuracy: 1) + } + } + + func testTouchFile() async throws { + try await self.withTemporaryFile { handle in + // Set some random value for both times, only to be overwritten by the current time + // right after. + try await handle.setTimes( + lastAccess: FileInfo.Timespec(seconds: 1, nanoseconds: 0), + lastDataModification: FileInfo.Timespec(seconds: 1, nanoseconds: 0) + ) + + var actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(actualLastAccessTime, FileInfo.Timespec(seconds: 1, nanoseconds: 0)) + + var actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(actualLastDataModificationTime, FileInfo.Timespec(seconds: 1, nanoseconds: 0)) + + try await handle.touch() + let estimatedCurrentTime = Date.now.timeIntervalSince1970 + + // Assert that the times are equal to the current time, with up to a second difference (to avoid timing flakiness). + actualLastAccessTime = try await handle.info().lastAccessTime + XCTAssertEqual(Float(actualLastAccessTime.seconds), Float(estimatedCurrentTime), accuracy: 1) + + actualLastDataModificationTime = try await handle.info().lastDataModificationTime + XCTAssertEqual(Float(actualLastDataModificationTime.seconds), Float(estimatedCurrentTime), accuracy: 1) + } + } } @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) diff --git a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift index 308b21da80..f32165f622 100644 --- a/Tests/NIOFileSystemTests/FileSystemErrorTests.swift +++ b/Tests/NIOFileSystemTests/FileSystemErrorTests.swift @@ -558,6 +558,19 @@ final class FileSystemErrorTests: XCTestCase { } } + func testErrnoMapping_futimens() { + self.testErrnoToErrorCode( + expected: [ + .permissionDenied: .permissionDenied, + .notPermitted: .permissionDenied, + .readOnlyFileSystem: .unsupported, + .badFileDescriptor: .closed, + ] + ) { errno in + .futimens(errno: errno, path: "", lastAccessTime: nil, lastDataModificationTime: nil, location: .fixed) + } + } + private func testErrnoToErrorCode( expected mapping: [Errno: FileSystemError.Code], _ makeError: (Errno) -> FileSystemError diff --git a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift index 2c22d87a19..abd7209c95 100644 --- a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift +++ b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift @@ -398,6 +398,22 @@ final class SyscallTests: XCTestCase { testCases.run() } + func test_futimens() throws { + let fd = FileDescriptor(rawValue: 42) + let times = timespec(tv_sec: 1, tv_nsec: 1) + withUnsafePointer(to: times) { unsafeTimesPointer in + let testCases = [ + MockTestCase(name: "futimens", .noInterrupt, 42, unsafeTimesPointer) { _ in + try Syscall.futimens( + fileDescriptor: fd, + times: unsafeTimesPointer + ).get() + } + ] + testCases.run() + } + } + func testValueOrErrno() throws { let r1: Result = valueOrErrno(retryOnInterrupt: false) { Errno._current = .addressInUse From 2b5d277589b9ee7c3e3dc24aeafb5890ff3644b1 Mon Sep 17 00:00:00 2001 From: Gustavo Cairo Date: Tue, 11 Jun 2024 18:55:26 +0100 Subject: [PATCH 09/16] Update availability guard (#2739) --- Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift index ffcd85af17..cbfd56887a 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift @@ -19,7 +19,7 @@ import NIOPosix import NIOFoundationCompat import XCTest -@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +@available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) final class FileHandleTests: XCTestCase { static let thisFile = FilePath(#filePath) static let testData = FilePath(#filePath) From 72cab7d9a4be37d6d123320f75d841f232e5474a Mon Sep 17 00:00:00 2001 From: Dimitri Bouniol Date: Wed, 12 Jun 2024 10:30:58 -0700 Subject: [PATCH 10/16] Correctly mark 304 as not having a response body (#2737) * Fixed an issue where HTTP/2 connections would disconnect if response compression were used and a 304 Not Modified response was returned * Added HTTP encoder tests for 304 Not Modified responses --- Sources/NIOHTTP1/HTTPTypes.swift | 1 + Tests/NIOHTTP1Tests/HTTPResponseEncoderTest.swift | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/Sources/NIOHTTP1/HTTPTypes.swift b/Sources/NIOHTTP1/HTTPTypes.swift index 616043c304..fe46d6074e 100644 --- a/Sources/NIOHTTP1/HTTPTypes.swift +++ b/Sources/NIOHTTP1/HTTPTypes.swift @@ -1159,6 +1159,7 @@ public enum HTTPResponseStatus: Sendable { .switchingProtocols, .processing, .noContent, + .notModified, .custom where (code < 200) && (code >= 100): return false default: diff --git a/Tests/NIOHTTP1Tests/HTTPResponseEncoderTest.swift b/Tests/NIOHTTP1Tests/HTTPResponseEncoderTest.swift index 1b493b98fd..7ac8031f51 100644 --- a/Tests/NIOHTTP1Tests/HTTPResponseEncoderTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPResponseEncoderTest.swift @@ -117,6 +117,12 @@ class HTTPResponseEncoderTests: XCTestCase { writtenData.assertContainsOnly("HTTP/1.1 204 No Content\r\ncontent-length: 0\r\n\r\n") } + func testNoContentLengthHeadersFor304() throws { + let headers = HTTPHeaders([("content-length", "0")]) + let writtenData = sendResponse(withStatus: .notModified, andHeaders: headers) + writtenData.assertContainsOnly("HTTP/1.1 304 Not Modified\r\n\r\n") + } + func testNoTransferEncodingHeadersFor101() throws { let headers = HTTPHeaders([("transfer-encoding", "chunked")]) let writtenData = sendResponse(withStatus: .switchingProtocols, andHeaders: headers) @@ -153,6 +159,12 @@ class HTTPResponseEncoderTests: XCTestCase { writtenData.assertContainsOnly("HTTP/1.1 204 No Content\r\ntransfer-encoding: chunked\r\n\r\n") } + func testNoTransferEncodingHeadersFor304() throws { + let headers = HTTPHeaders([("transfer-encoding", "chunked")]) + let writtenData = sendResponse(withStatus: .notModified, andHeaders: headers) + writtenData.assertContainsOnly("HTTP/1.1 304 Not Modified\r\n\r\n") + } + func testNoChunkedEncodingForHTTP10() throws { let channel = EmbeddedChannel() defer { From e5a216ba89deba84356bad9d4c2eab99071c745b Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Thu, 13 Jun 2024 13:24:28 -0500 Subject: [PATCH 11/16] Silence warning about missing include in macOS builds (#2741) --- Sources/CNIOLinux/include/CNIOLinux.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Sources/CNIOLinux/include/CNIOLinux.h b/Sources/CNIOLinux/include/CNIOLinux.h index cde2942fde..dd4bf976c8 100644 --- a/Sources/CNIOLinux/include/CNIOLinux.h +++ b/Sources/CNIOLinux/include/CNIOLinux.h @@ -31,12 +31,19 @@ #include #include #include -#include "liburing_nio.h" #include #include #include #include #include +#endif + +// We need to include this outside the `#ifdef` so macOS builds don't warn about the missing include, +// but we also need to make sure the system includes come before it on Linux, so we put it down here +// between an `#endif/#ifdef` pair rather than at the top. +#include "liburing_nio.h" + +#ifdef __linux__ #if __has_include() #include From 38f6b98c6a3e9fcd7d5be893d9dc4429294af9bd Mon Sep 17 00:00:00 2001 From: taylorswift Date: Fri, 21 Jun 2024 04:23:18 -0500 Subject: [PATCH 12/16] convert the NIOFileSystem example code to a Snippet (#2746) * convert the NIOFileSystem example code to a Snippet * fix compilation error * add missing license header * exclude snippets from soundness.sh --- Snippets/NIOFileSystemTour.swift | 97 +++++++++++++++++++ .../NIOFileSystem/Docs.docc/NIOFileSystem.md | 97 +------------------ scripts/soundness.sh | 2 +- 3 files changed, 99 insertions(+), 97 deletions(-) create mode 100644 Snippets/NIOFileSystemTour.swift diff --git a/Snippets/NIOFileSystemTour.swift b/Snippets/NIOFileSystemTour.swift new file mode 100644 index 0000000000..7fe5947e35 --- /dev/null +++ b/Snippets/NIOFileSystemTour.swift @@ -0,0 +1,97 @@ +// snippet.hide +import _NIOFileSystem +import NIOCore +// snippet.show + +// NIOFileSystem provides access to the local file system via the FileSystem +// type which is available as a global shared instance. +let fileSystem = FileSystem.shared + +// Files can be inspected by using 'info': +if let info = try await fileSystem.info(forFileAt: "/Users/hal9000/demise-of-dave.txt") { + print("demise-of-dave.txt has type '\(info.type)'") +} else { + print("demise-of-dave.txt doesn't exist") +} + +// Let's find out what's in that file. +do { + // Reading a whole file requires a limit. If the file is larger than the limit + // then an error is thrown. This avoids accidentally consuming too much memory + // if the file is larger than expected. + let plan = try await ByteBuffer( + contentsOf: "/Users/hal9000/demise-of-dave.txt", + maximumSizeAllowed: .mebibytes(1) + ) + print("Plan for Dave's demise:", String(decoding: plan.readableBytesView, as: UTF8.self)) +} catch let error as FileSystemError where error.code == .notFound { + // All errors thrown by the module have type FileSystemError (or + // Swift.CancellationError). It looks like the file doesn't exist. Let's + // create it now. + // + // The code above for reading the file is shorthand for opening the file in + // read-only mode and then reading its contents. The FileSystemProtocol + // has a few different 'withFileHandle' methods for opening a file in different + // modes. Let's open a file for writing, creating it at the same time. + try await fileSystem.withFileHandle( + forWritingAt: "/Users/hal9000/demise-of-dave.txt", + options: .newFile(replaceExisting: false) + ) { file in + let plan = ByteBuffer(string: "TODO...") + try await file.write(contentsOf: plan.readableBytesView, toAbsoluteOffset: 0) + } +} + +// Directories can be opened like regular files but they cannot be read from or +// written to. However, their contents can be listed: +let path: FilePath? = try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Music") { directory in + for try await entry in directory.listContents() { + if entry.name.extension == "mp3", entry.name.stem.contains("daisy") { + // Found it! + return entry.path + } + } + // No luck. + return nil +} + +if let path = path { + print("Found file at '\(path)'") +} + +// The file system can also be used to perform the following operations on files +// and directories: +// - copy, +// - remove, +// - rename, and +// - replace. +// +// Here's an example of copying a directory: +try await fileSystem.copyItem(at: "/Users/hal9000/Music", to: "/Volumes/Tardis/Music") + +// Symbolic links can also be created (and read with 'destinationOfSymbolicLink(at:)'). +try await fileSystem.createSymbolicLink(at: "/Users/hal9000/Backup", withDestination: "/Volumes/Tardis") + +// Opening a symbolic link opens its destination so in most cases there's no +// need to read the destination of a symbolic link: +try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Backup") { directory in + // Beyond listing the contents of a directory, the directory handle provides a + // number of other functions, many of which are also available on regular file + // handles. + // + // This includes getting information about a file, such as its permissions, last access time, + // and last modification time: + let info = try await directory.info() + print("The directory has permissions '\(info.permissions)'") + + // Where supported, the extended attributes of a file can also be accessed, read, and modified: + for attribute in try await directory.attributeNames() { + let value = try await directory.valueForAttribute(attribute) + print("Extended attribute '\(attribute)' has value '\(value)'") + } + + // Once this closure returns the file system will close the directory handle freeing + // any resources required to access it such as file descriptors. Handles can also be opened + // with the 'openFile' and 'openDirectory' APIs but that places the onus you to close the + // handle at an appropriate time to avoid leaking resources. +} diff --git a/Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md b/Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md index 1803cac5ad..4fbf466541 100644 --- a/Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md +++ b/Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md @@ -23,102 +23,7 @@ to a set of protocols for creating other file system implementations. The following sample code demonstrates a number of the APIs offered by this module: -```swift -import _NIOFileSystem - -// NIOFileSystem provides access to the local file system via the FileSystem -// type which is available as a global shared instance. -let fileSystem = FileSystem.shared - -// Files can be inspected by using 'info': -if let info = try await fileSystem.info(forFileAt: "/Users/hal9000/demise-of-dave.txt") { - print("demise-of-dave.txt has type '\(info.type)'") -} else { - print("demise-of-dave.txt doesn't exist") -} - -// Let's find out what's in that file. -do { - // Reading a whole file requires a limit. If the file is larger than the limit - // then an error is thrown. This avoids accidentally consuming too much memory - // if the file is larger than expected. - let plan = try await ByteBuffer( - contentsOf: "/Users/hal9000/demise-of-dave.txt", - maximumSizeAllowed: .mebibytes(1) - ) - print("Plan for Dave's demise:", String(decoding: plan, as: UTF8.self)) -} catch let error as FileSystemError where error.code == .notFound { - // All errors thrown by the module have type FileSystemError (or - // Swift.CancellationError). It looks like the file doesn't exist. Let's - // create it now. - // - // The code above for reading the file is shorthand for opening the file in - // read-only mode and then reading its contents. The FileSystemProtocol - // has a few different 'withFileHandle' methods for opening a file in different - // modes. Let's open a file for writing, creating it at the same time. - try await fileSystem.withFileHandle( - forWritingAt: "/Users/hal9000/demise-of-dave.txt", - options: .newFile(replaceExisting: false) - ) { file in - let plan = ByteBuffer(string: "TODO...") - try await file.write(contentsOf: plan.readableBytesView, toAbsoluteOffset: 0) - } -} - -// Directories can be opened like regular files but they cannot be read from or -// written to. However, their contents can be listed: -let path: FilePath? = try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Music") { directory in - for try await entry in directory.listContents() { - if entry.name.extension == "mp3", entry.name.stem.contains("daisy") { - // Found it! - return entry.path - } - } - // No luck. - return nil -} - -if let path = path { - print("Found file at '\(path)'") -} - -// The file system can also be used to perform the following operations on files -// and directories: -// - copy, -// - remove, -// - rename, and -// - replace. -// -// Here's an example of copying a directory: -try await fileSystem.copyItem(at: "/Users/hal9000/Music", to: "/Volumes/Tardis/Music") - -// Symbolic links can also be created (and read with 'destinationOfSymbolicLink(at:)'). -try await fileSystem.createSymbolicLink(at: "/Users/hal9000/Backup", withDestination: "/Volumes/Tardis") - -// Opening a symbolic link opens its destination so in most cases there's no -// need to read the destination of a symbolic link: -try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Backup") { directory in - // Beyond listing the contents of a directory, the directory handle provides a - // number of other functions, many of which are also available on regular file - // handles. - // - // This includes getting information about a file, such as its permissions, last access time, - // and last modification time: - let info = try await directory.info() - print("The directory has permissions '\(info.permissions)'") - - // Where supported, the extended attributes of a file can also be accessed, read, and modified: - for attribute in try await directory.attributeNames() { - let value = try await directory.valueForAttribute(attribute) - print("Extended attribute '\(attribute)' has value '\(value)'") - } - - // Once this closure returns the file system will close the directory handle freeing - // any resources required to access it such as file descriptors. Handles can also be opened - // with the 'openFile' and 'openDirectory' APIs but that places the onus you to close the - // handle at an appropriate time to avoid leaking resources. -} -``` +@Snippet(path: "swift-nio/Snippets/NIOFileSystemTour") In depth documentation can be found in the following sections. diff --git a/scripts/soundness.sh b/scripts/soundness.sh index b66cf420a6..1e6c2ca67b 100755 --- a/scripts/soundness.sh +++ b/scripts/soundness.sh @@ -51,7 +51,7 @@ for language in swift-or-c bash dtrace python; do matching_files=( -name '*' ) case "$language" in swift-or-c) - exceptions=( -name c_nio_llhttp.c -o -name c_nio_api.c -o -name c_nio_http.c -o -name c_nio_llhttp.h -o -name cpp_magic.h -o -name Package.swift -o -name 'Package@*.swift' -o -name CNIOSHA1.h -o -name c_nio_sha1.c -o -name ifaddrs-android.c -o -name ifaddrs-android.h) + exceptions=( -name c_nio_llhttp.c -o -name c_nio_api.c -o -name c_nio_http.c -o -name c_nio_llhttp.h -o -name cpp_magic.h -o -name Package.swift -o -name 'Package@*.swift' -o -path './Snippets/*' -o -name CNIOSHA1.h -o -name c_nio_sha1.c -o -name ifaddrs-android.c -o -name ifaddrs-android.h) matching_files=( -name '*.swift' -o -name '*.c' -o -name '*.h' ) cat > "$tmp" <<"EOF" //===----------------------------------------------------------------------===// From 41f66948e1936305dca7ecffbe243f2357d4e4ed Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Fri, 21 Jun 2024 16:59:46 +0100 Subject: [PATCH 13/16] fix link to NIOFileSystem from NIO index page (#2747) Motivation: The existing link to NIOFileSystem docs 404's Modifications: Speculative fixes because getting any links to work locally is challenging. * rename `NIOFileSystem.md` -> `index.md` in line with other modules * modify the link on the NIO index page to reference `_NIOFileSystem` local experiments suggest it might change the form of the generated URL to where the docs can be found (https://swiftpackageindex.com/apple/swift-nio/2.67.0/documentation/_NIOFileSystem). Result: Hopefully the NIOFileSystem docs link will work --- Sources/NIO/Docs.docc/index.md | 2 +- Sources/NIOFileSystem/Docs.docc/{NIOFileSystem.md => index.md} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename Sources/NIOFileSystem/Docs.docc/{NIOFileSystem.md => index.md} (100%) diff --git a/Sources/NIO/Docs.docc/index.md b/Sources/NIO/Docs.docc/index.md index f3f864855e..90f318e0e4 100644 --- a/Sources/NIO/Docs.docc/index.md +++ b/Sources/NIO/Docs.docc/index.md @@ -161,7 +161,7 @@ The core SwiftNIO repository will contain a few extremely important protocol imp [module-tls]: ./NIOTLS [module-websocket]: ./NIOWebSocket [module-test-utilities]: ./NIOTestUtils -[module-filesystem]: ./NIOFileSystem +[module-filesystem]: ./_NIOFileSystem [ch]: ./NIOCore/ChannelHandler [c]: ./NIOCore/Channel diff --git a/Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md b/Sources/NIOFileSystem/Docs.docc/index.md similarity index 100% rename from Sources/NIOFileSystem/Docs.docc/NIOFileSystem.md rename to Sources/NIOFileSystem/Docs.docc/index.md From ea6a8db87ae15f370e562b9692079a5909ed4d92 Mon Sep 17 00:00:00 2001 From: taylorswift Date: Fri, 21 Jun 2024 14:20:14 -0500 Subject: [PATCH 14/16] put snippet code inside @available function (#2750) --- Snippets/NIOFileSystemTour.swift | 166 ++++++++++++++++--------------- 1 file changed, 86 insertions(+), 80 deletions(-) diff --git a/Snippets/NIOFileSystemTour.swift b/Snippets/NIOFileSystemTour.swift index 7fe5947e35..f79135b45b 100644 --- a/Snippets/NIOFileSystemTour.swift +++ b/Snippets/NIOFileSystemTour.swift @@ -1,97 +1,103 @@ // snippet.hide import _NIOFileSystem import NIOCore -// snippet.show -// NIOFileSystem provides access to the local file system via the FileSystem -// type which is available as a global shared instance. -let fileSystem = FileSystem.shared +@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) +func main() async throws +{ + // snippet.show -// Files can be inspected by using 'info': -if let info = try await fileSystem.info(forFileAt: "/Users/hal9000/demise-of-dave.txt") { - print("demise-of-dave.txt has type '\(info.type)'") -} else { - print("demise-of-dave.txt doesn't exist") -} + // NIOFileSystem provides access to the local file system via the FileSystem + // type which is available as a global shared instance. + let fileSystem = FileSystem.shared -// Let's find out what's in that file. -do { - // Reading a whole file requires a limit. If the file is larger than the limit - // then an error is thrown. This avoids accidentally consuming too much memory - // if the file is larger than expected. - let plan = try await ByteBuffer( - contentsOf: "/Users/hal9000/demise-of-dave.txt", - maximumSizeAllowed: .mebibytes(1) - ) - print("Plan for Dave's demise:", String(decoding: plan.readableBytesView, as: UTF8.self)) -} catch let error as FileSystemError where error.code == .notFound { - // All errors thrown by the module have type FileSystemError (or - // Swift.CancellationError). It looks like the file doesn't exist. Let's - // create it now. - // - // The code above for reading the file is shorthand for opening the file in - // read-only mode and then reading its contents. The FileSystemProtocol - // has a few different 'withFileHandle' methods for opening a file in different - // modes. Let's open a file for writing, creating it at the same time. - try await fileSystem.withFileHandle( - forWritingAt: "/Users/hal9000/demise-of-dave.txt", - options: .newFile(replaceExisting: false) - ) { file in - let plan = ByteBuffer(string: "TODO...") - try await file.write(contentsOf: plan.readableBytesView, toAbsoluteOffset: 0) + // Files can be inspected by using 'info': + if let info = try await fileSystem.info(forFileAt: "/Users/hal9000/demise-of-dave.txt") { + print("demise-of-dave.txt has type '\(info.type)'") + } else { + print("demise-of-dave.txt doesn't exist") } -} -// Directories can be opened like regular files but they cannot be read from or -// written to. However, their contents can be listed: -let path: FilePath? = try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Music") { directory in - for try await entry in directory.listContents() { - if entry.name.extension == "mp3", entry.name.stem.contains("daisy") { - // Found it! - return entry.path + // Let's find out what's in that file. + do { + // Reading a whole file requires a limit. If the file is larger than the limit + // then an error is thrown. This avoids accidentally consuming too much memory + // if the file is larger than expected. + let plan = try await ByteBuffer( + contentsOf: "/Users/hal9000/demise-of-dave.txt", + maximumSizeAllowed: .mebibytes(1) + ) + print("Plan for Dave's demise:", String(decoding: plan.readableBytesView, as: UTF8.self)) + } catch let error as FileSystemError where error.code == .notFound { + // All errors thrown by the module have type FileSystemError (or + // Swift.CancellationError). It looks like the file doesn't exist. Let's + // create it now. + // + // The code above for reading the file is shorthand for opening the file in + // read-only mode and then reading its contents. The FileSystemProtocol + // has a few different 'withFileHandle' methods for opening a file in different + // modes. Let's open a file for writing, creating it at the same time. + try await fileSystem.withFileHandle( + forWritingAt: "/Users/hal9000/demise-of-dave.txt", + options: .newFile(replaceExisting: false) + ) { file in + let plan = ByteBuffer(string: "TODO...") + try await file.write(contentsOf: plan.readableBytesView, toAbsoluteOffset: 0) } } - // No luck. - return nil -} -if let path = path { - print("Found file at '\(path)'") -} - -// The file system can also be used to perform the following operations on files -// and directories: -// - copy, -// - remove, -// - rename, and -// - replace. -// -// Here's an example of copying a directory: -try await fileSystem.copyItem(at: "/Users/hal9000/Music", to: "/Volumes/Tardis/Music") + // Directories can be opened like regular files but they cannot be read from or + // written to. However, their contents can be listed: + let path: FilePath? = try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Music") { directory in + for try await entry in directory.listContents() { + if entry.name.extension == "mp3", entry.name.stem.contains("daisy") { + // Found it! + return entry.path + } + } + // No luck. + return nil + } -// Symbolic links can also be created (and read with 'destinationOfSymbolicLink(at:)'). -try await fileSystem.createSymbolicLink(at: "/Users/hal9000/Backup", withDestination: "/Volumes/Tardis") + if let path = path { + print("Found file at '\(path)'") + } -// Opening a symbolic link opens its destination so in most cases there's no -// need to read the destination of a symbolic link: -try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Backup") { directory in - // Beyond listing the contents of a directory, the directory handle provides a - // number of other functions, many of which are also available on regular file - // handles. + // The file system can also be used to perform the following operations on files + // and directories: + // - copy, + // - remove, + // - rename, and + // - replace. // - // This includes getting information about a file, such as its permissions, last access time, - // and last modification time: - let info = try await directory.info() - print("The directory has permissions '\(info.permissions)'") + // Here's an example of copying a directory: + try await fileSystem.copyItem(at: "/Users/hal9000/Music", to: "/Volumes/Tardis/Music") - // Where supported, the extended attributes of a file can also be accessed, read, and modified: - for attribute in try await directory.attributeNames() { - let value = try await directory.valueForAttribute(attribute) - print("Extended attribute '\(attribute)' has value '\(value)'") - } + // Symbolic links can also be created (and read with 'destinationOfSymbolicLink(at:)'). + try await fileSystem.createSymbolicLink(at: "/Users/hal9000/Backup", withDestination: "/Volumes/Tardis") + + // Opening a symbolic link opens its destination so in most cases there's no + // need to read the destination of a symbolic link: + try await fileSystem.withDirectoryHandle(atPath: "/Users/hal9000/Backup") { directory in + // Beyond listing the contents of a directory, the directory handle provides a + // number of other functions, many of which are also available on regular file + // handles. + // + // This includes getting information about a file, such as its permissions, last access time, + // and last modification time: + let info = try await directory.info() + print("The directory has permissions '\(info.permissions)'") - // Once this closure returns the file system will close the directory handle freeing - // any resources required to access it such as file descriptors. Handles can also be opened - // with the 'openFile' and 'openDirectory' APIs but that places the onus you to close the - // handle at an appropriate time to avoid leaking resources. + // Where supported, the extended attributes of a file can also be accessed, read, and modified: + for attribute in try await directory.attributeNames() { + let value = try await directory.valueForAttribute(attribute) + print("Extended attribute '\(attribute)' has value '\(value)'") + } + + // Once this closure returns the file system will close the directory handle freeing + // any resources required to access it such as file descriptors. Handles can also be opened + // with the 'openFile' and 'openDirectory' APIs but that places the onus you to close the + // handle at an appropriate time to avoid leaking resources. + } + // snippet.end } From 5d7a9997e67c180d1f9cf53995ac1ff9694700a0 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Mon, 24 Jun 2024 15:11:36 +0100 Subject: [PATCH 15/16] Throw an appropriate error from the writer when the channel closed (#2744) # Motivation Currently, when the channel closes and a user tries to write something the writer throws an `AlreadyFinished` error. This error can also be thrown when calling `finish` on the writer and then trying to call `write` again. This makes it hard to distinguish if the thrown error was due to the channel being closed or due to a business logic error in handling the writer. # Modification This PR finishes the writer with a `ChannelError.ioOnClosedChannel` if the writer gets finished to due a channel inactive or handler removed. # Result Users can now distinguish if they did something wrong with the writer or if the channel closed. --- .../AsyncChannelOutboundWriterHandler.swift | 4 ++-- .../AsyncChannel/AsyncChannelTests.swift | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift index daa35fd63e..8fa2111274 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift @@ -137,7 +137,7 @@ internal final class NIOAsyncChannelOutboundWriterHandler @inlinable func handlerRemoved(context: ChannelHandlerContext) { self.context = nil - self.sink?.finish() + self.sink?.finish(error: ChannelError.ioOnClosedChannel) self.writer = nil } @@ -150,7 +150,7 @@ internal final class NIOAsyncChannelOutboundWriterHandler @inlinable func channelInactive(context: ChannelHandlerContext) { - self.sink?.finish() + self.sink?.finish(error: ChannelError.ioOnClosedChannel) context.fireChannelInactive() } diff --git a/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift b/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift index cafddee308..cbaec06640 100644 --- a/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift +++ b/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift @@ -81,6 +81,24 @@ final class AsyncChannelTests: XCTestCase { } } + func testAsyncChannelThrowsWhenChannelClosed() async throws { + let channel = NIOAsyncTestingChannel() + let wrapped = try await channel.testingEventLoop.executeInContext { + return try NIOAsyncChannel(wrappingChannelSynchronously: channel) + } + + try await channel.close(mode: .all) + + do { + try await wrapped.executeThenClose { _, outbound in + try await outbound.write("Test") + } + XCTFail("Expected an error to be thrown") + } catch { + XCTAssertEqual(error as? ChannelError, ChannelError.ioOnClosedChannel) + } + } + func testFinishingTheWriterClosesTheWriteSideOfTheChannel() async throws { let channel = NIOAsyncTestingChannel() let closeRecorder = CloseRecorder() From fc79798d5a150d61361a27ce0c51169b889e23de Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Thu, 27 Jun 2024 15:51:24 +0100 Subject: [PATCH 16/16] NIOSendableBox: allow off-loop initialisation iff Value is Sendable (#2753) ### Motivation: `NIOLoopBound` and `NIOLoopBoundBox` are important tools for making mutable types `Sendable`. The power of `NIOLoopBoundBox` is that it allows correct mutation, whilst remaining `Sendable` without locks and without having to use `@unchecked` if the mutable class makes sure to have all accesses (reading & writing) run on one particular `EventLoop`. This is safe as `EventLoop`s guarantee sequentially consistent memory order across executions of different work items/events. Typically `EventLoop`s achieve this by just being thread-bound. These types are well used already but there's a small and common pattern which is safe but unsupported as of yet: Initialise a `NIOLoopBoundBox` with a `Sendable` value _off_ the loop. All further accesses (reads & writes) however happen _on_ the loop. That's safe because of Swift's Definitive Initialisation (DI) but `NIOLoopBoundBox` didn't support this pattern (apart from `makeEmptyBox` which always initialises with `nil`). ### Modifications: - Allow `Sendable` values to be provided whilst creating the type off loop. ### Result: - Even more types can be safely & correctly made `Sendable` without using `@unchecked. --- Sources/NIOCore/NIOLoopBound.swift | 20 ++++++++++++++++++++ Tests/NIOPosixTests/NIOLoopBoundTests.swift | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/Sources/NIOCore/NIOLoopBound.swift b/Sources/NIOCore/NIOLoopBound.swift index c105f361d1..a3631f5b2f 100644 --- a/Sources/NIOCore/NIOLoopBound.swift +++ b/Sources/NIOCore/NIOLoopBound.swift @@ -107,6 +107,26 @@ public final class NIOLoopBoundBox: @unchecked Sendable { return .init(_value: nil, uncheckedEventLoop: eventLoop) } + /// Initialise a ``NIOLoopBoundBox`` by sending a `Sendable` value, validly callable off `eventLoop`. + /// + /// Contrary to ``init(_:eventLoop:)``, this method can be called off `eventLoop` because we know that `value` is `Sendable`. + /// So we don't need to protect `value` itself, we just need to protect the ``NIOLoopBoundBox`` against mutations which we do because the ``value`` + /// accessors are checking that we're on `eventLoop`. + public static func makeBoxSendingValue( + _ value: Value, + as: Value.Type = Value.self, + eventLoop: EventLoop + ) -> NIOLoopBoundBox where Value: Sendable { + // Here, we -- possibly surprisingly -- do not precondition being on the EventLoop. This is okay for a few + // reasons: + // - This function only works with `Sendable` values, so we don't need to worry about somebody + // still holding a reference to this. + // - Because of Swift's Definitive Initialisation (DI), we know that we did write `self._value` before `init` + // returns. + // - The only way to ever write (or read indeed) `self._value` is by proving to be inside the `EventLoop`. + return .init(_value: value, uncheckedEventLoop: eventLoop) + } + /// Access the `value` with the precondition that the code is running on `eventLoop`. /// /// - note: ``NIOLoopBoundBox`` itself is reference-typed, so any writes will affect anybody sharing this reference. diff --git a/Tests/NIOPosixTests/NIOLoopBoundTests.swift b/Tests/NIOPosixTests/NIOLoopBoundTests.swift index 4785c5d4dd..0c525125ad 100644 --- a/Tests/NIOPosixTests/NIOLoopBoundTests.swift +++ b/Tests/NIOPosixTests/NIOLoopBoundTests.swift @@ -49,6 +49,25 @@ final class NIOLoopBoundTests: XCTestCase { }.wait()) } + func testLoopBoundBoxCanBeInitialisedWithSendableValueOffLoopAndLaterSetToValue() { + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + let loop = group.any() + + let sendableBox = NIOLoopBoundBox.makeBoxSendingValue(15, as: Int.self, eventLoop: loop) + for _ in 0..<(100 - 15) { + loop.execute { + sendableBox.value += 1 + } + } + XCTAssertEqual(100, try loop.submit { + sendableBox.value + }.wait()) + } + // MARK: - Helpers func sendableBlackhole(_ sendableThing: S) {}