From 0ee1657e8a648b2997aef020e0d2b2adbf49dce8 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Mon, 25 Nov 2024 16:05:30 +0000 Subject: [PATCH] Make NIOFileDescriptor/FileRegion/IOData Sendable & soft-deprecated (#2598) Motivation: `IOData` is a legacy but alas also core type that needs to be `Sendable`. Before this PR however it can't be `Sendable` because it holds a `FileRegion` which holds a `NIOFileDescriptor`. So let's make all of these `Sendable` but let's also start the deprecation journey for the following types: - `IOData`, now soft-deprecated (no warnings) because on its reliance on `FileRegion` - `FileRegion`, now soft-deprecated (no warnings) because on its reliance on `NIOFileHandle` - `NIOFileHandle`, now soft-deprecated (warnings on the `NIOFileHandle(descriptor:)` constructor but with a `NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor:)` alternative - `NonBlockingFileIO`, now soft-deprecated (warnings on the `openFile` functions (but with `_deprecated` alternatives) because of their reliance on `NIOFileHandle) Modification: - Make `NIOFileDescriptor`, `FileRegion` and `IOData` `Sendable` by tracking the fd number and the usage state in an atomic - Enforce singular access by making the `withFileDescriptor { fd ... }` function atomically exchange the fd number for a "I'm busy" sentinel value - Start deprecating `IOData`, `NIOFileHandle`, `NonBlockingFileIO`, `FileRegion` Result: - `NIOFileDescriptor`, `FileRegion` and `IOData` can be `Sendable` --- Sources/NIOCore/ChannelInvoker.swift | 2 +- Sources/NIOCore/FileHandle.swift | 289 ++++++++++++++++-- Sources/NIOCore/FileRegion.swift | 12 +- Sources/NIOCore/IOData.swift | 12 +- Sources/NIOCore/Linux.swift | 2 +- Sources/NIOCrashTester/OutputGrepper.swift | 6 +- Sources/NIOHTTP1Server/main.swift | 2 +- Sources/NIOPosix/Bootstrap.swift | 32 +- Sources/NIOPosix/NonBlockingFileIO.swift | 109 ++++++- Sources/NIOPosix/PendingWritesManager.swift | 7 +- Sources/NIOPosix/PipeChannel.swift | 38 +-- Sources/NIOPosix/PipePair.swift | 91 ++++-- Sources/NIOPosix/SelectorEpoll.swift | 12 +- Sources/NIOPosix/SelectorGeneric.swift | 20 +- Sources/NIOPosix/SelectorKqueue.swift | 38 ++- Sources/NIOPosix/SelectorUring.swift | 14 +- Tests/NIOCoreTests/BaseObjectsTest.swift | 10 +- Tests/NIOCoreTests/NIOAnyDebugTest.swift | 2 +- Tests/NIOCoreTests/XCTest+Extensions.swift | 4 +- .../EmbeddedChannelTest.swift | 4 +- .../NIOHTTP1Tests/HTTPServerClientTest.swift | 4 +- Tests/NIOPosixTests/BootstrapTest.swift | 33 +- Tests/NIOPosixTests/ChannelPipelineTest.swift | 4 +- Tests/NIOPosixTests/ChannelTests.swift | 16 +- Tests/NIOPosixTests/FileRegionTest.swift | 14 +- Tests/NIOPosixTests/NIOFileHandleTest.swift | 171 +++++++++++ .../NIOPosixTests/NonBlockingFileIOTest.swift | 62 ++-- Tests/NIOPosixTests/TestUtils.swift | 14 +- 28 files changed, 771 insertions(+), 253 deletions(-) create mode 100644 Tests/NIOPosixTests/NIOFileHandleTest.swift diff --git a/Sources/NIOCore/ChannelInvoker.swift b/Sources/NIOCore/ChannelInvoker.swift index 8d831cac34..af3051b5f7 100644 --- a/Sources/NIOCore/ChannelInvoker.swift +++ b/Sources/NIOCore/ChannelInvoker.swift @@ -195,7 +195,7 @@ extension ChannelOutboundInvoker { public func close(mode: CloseMode = .all, file: StaticString = #fileID, line: UInt = #line) -> EventLoopFuture { let promise = makePromise(file: file, line: line) - close(mode: mode, promise: promise) + self.close(mode: mode, promise: promise) return promise.futureResult } diff --git a/Sources/NIOCore/FileHandle.swift b/Sources/NIOCore/FileHandle.swift index 733aec483e..62fd100e28 100644 --- a/Sources/NIOCore/FileHandle.swift +++ b/Sources/NIOCore/FileHandle.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -11,6 +11,9 @@ // SPDX-License-Identifier: Apache-2.0 // //===----------------------------------------------------------------------===// + +import Atomics + #if os(Windows) import ucrt #elseif canImport(Darwin) @@ -34,7 +37,105 @@ public typealias NIOPOSIXFileMode = CInt public typealias NIOPOSIXFileMode = mode_t #endif -/// A `NIOFileHandle` is a handle to an open file. +#if arch(x86_64) || arch(arm64) +// 64 bit architectures +typealias OneUInt32 = UInt32 +typealias TwoUInt32s = UInt64 + +// Now we need to make `UInt64` match `DoubleWord`'s API but we can't use a custom +// type because we need special support by the `swift-atomics` package. +extension UInt64 { + fileprivate init(first: UInt32, second: UInt32) { + self = UInt64(first) << 32 | UInt64(second) + } + + fileprivate var first: UInt32 { + get { + UInt32(truncatingIfNeeded: self >> 32) + } + set { + self = (UInt64(newValue) << 32) | UInt64(self.second) + } + } + + fileprivate var second: UInt32 { + get { + UInt32(truncatingIfNeeded: self & 0xff_ff_ff_ff) + } + set { + self = (UInt64(self.first) << 32) | UInt64(newValue) + } + } +} +#elseif arch(arm) || arch(i386) || arch(arm64_32) +// 32 bit architectures +// Note: for testing purposes you can also use these defines for 64 bit platforms, they'll just consume twice as +// much space, nothing else will go bad. +typealias OneUInt32 = UInt +typealias TwoUInt32s = DoubleWord +#else +#error("Unknown architecture") +#endif + +internal struct FileDescriptorState { + private static let closedValue: OneUInt32 = 0xdead + private static let inUseValue: OneUInt32 = 0xbeef + private static let openValue: OneUInt32 = 0xcafe + internal var rawValue: TwoUInt32s + + internal init(rawValue: TwoUInt32s) { + self.rawValue = rawValue + } + + internal init(descriptor: CInt) { + self.rawValue = TwoUInt32s( + first: .init(truncatingIfNeeded: CUnsignedInt(bitPattern: descriptor)), + second: Self.openValue + ) + } + + internal var descriptor: CInt { + get { + CInt(bitPattern: UInt32(truncatingIfNeeded: self.rawValue.first)) + } + set { + self.rawValue.first = .init(truncatingIfNeeded: CUnsignedInt(bitPattern: newValue)) + } + } + + internal var isOpen: Bool { + self.rawValue.second == Self.openValue + } + + internal var isInUse: Bool { + self.rawValue.second == Self.inUseValue + } + + internal var isClosed: Bool { + self.rawValue.second == Self.closedValue + } + + mutating func close() { + assert(self.isOpen) + self.rawValue.second = Self.closedValue + } + + mutating func markInUse() { + assert(self.isOpen) + self.rawValue.second = Self.inUseValue + } + + mutating func markNotInUse() { + assert(self.rawValue.second == Self.inUseValue) + self.rawValue.second = Self.openValue + } +} + +/// Deprecated. `NIOFileHandle` is a handle to an open file descriptor. +/// +/// - warning: The `NIOFileHandle` API is deprecated, do not use going forward. It's not marked as `deprecated` yet such +/// that users don't get the deprecation warnings affecting their APIs everywhere. For file I/O, please use +/// the `NIOFileSystem` API. /// /// When creating a `NIOFileHandle` it takes ownership of the underlying file descriptor. When a `NIOFileHandle` is no longer /// needed you must `close` it or take back ownership of the file descriptor using `takeDescriptorOwnership`. @@ -43,16 +144,54 @@ public typealias NIOPOSIXFileMode = mode_t /// /// - warning: Failing to manage the lifetime of a `NIOFileHandle` correctly will result in undefined behaviour. /// -/// - warning: `NIOFileHandle` objects are not thread-safe and are mutable. They also cannot be fully thread-safe as they refer to a global underlying file descriptor. -public final class NIOFileHandle: FileDescriptor { - public private(set) var isOpen: Bool - private let descriptor: CInt +/// - Note: As of SwiftNIO 2.77.0, `NIOFileHandle` objects are are thread-safe and enforce singular access. If you access the same `NIOFileHandle` +/// multiple times, it will throw `IOError(errorCode: EBUSY)` for the second access. +public final class NIOFileHandle: FileDescriptor & Sendable { + private static let descriptorClosed: CInt = CInt.min + private let descriptor: UnsafeAtomic + + public var isOpen: Bool { + FileDescriptorState( + rawValue: self.descriptor.load(ordering: .sequentiallyConsistent) + ).isOpen + } + + private static func interpretDescriptorValueThrowIfInUseOrNotOpen( + _ descriptor: TwoUInt32s + ) throws -> FileDescriptorState { + let descriptorState = FileDescriptorState(rawValue: descriptor) + if descriptorState.isOpen { + return descriptorState + } else if descriptorState.isClosed { + throw IOError(errnoCode: EBADF, reason: "can't close file (as it's not open anymore).") + } else { + throw IOError(errnoCode: EBUSY, reason: "file descriptor currently in use") + } + } + + private func peekAtDescriptorIfOpen() throws -> FileDescriptorState { + let descriptor = self.descriptor.load(ordering: .relaxed) + return try Self.interpretDescriptorValueThrowIfInUseOrNotOpen(descriptor) + } + + /// Create a `NIOFileHandle` taking ownership of `descriptor`. You must call `NIOFileHandle.close` or `NIOFileHandle.takeDescriptorOwnership` before + /// this object can be safely released. + @available( + *, + deprecated, + message: """ + Avoid using NIOFileHandle. The type is difficult to hold correctly, \ + use NIOFileSystem as a replacement API. + """ + ) + public convenience init(descriptor: CInt) { + self.init(_deprecatedTakingOwnershipOfDescriptor: descriptor) + } /// Create a `NIOFileHandle` taking ownership of `descriptor`. You must call `NIOFileHandle.close` or `NIOFileHandle.takeDescriptorOwnership` before /// this object can be safely released. - public init(descriptor: CInt) { - self.descriptor = descriptor - self.isOpen = true + public init(_deprecatedTakingOwnershipOfDescriptor descriptor: CInt) { + self.descriptor = UnsafeAtomic.create(FileDescriptorState(descriptor: descriptor).rawValue) } deinit { @@ -60,6 +199,7 @@ public final class NIOFileHandle: FileDescriptor { !self.isOpen, "leaked open NIOFileHandle(descriptor: \(self.descriptor)). Call `close()` to close or `takeDescriptorOwnership()` to take ownership and close by some other means." ) + self.descriptor.destroy() } #if !os(WASI) @@ -70,12 +210,64 @@ public final class NIOFileHandle: FileDescriptor { /// /// - Returns: A new `NIOFileHandle` with a fresh underlying file descriptor but shared seek pointer. public func duplicate() throws -> NIOFileHandle { - try withUnsafeFileDescriptor { fd in - NIOFileHandle(descriptor: try SystemCalls.dup(descriptor: fd)) + try self.withUnsafeFileDescriptor { fd in + NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: try SystemCalls.dup(descriptor: fd)) } } #endif + private func activateDescriptor(as descriptor: CInt) { + let desired = FileDescriptorState(descriptor: descriptor) + var expected = desired + expected.markInUse() + let (exchanged, original) = self.descriptor.compareExchange( + expected: expected.rawValue, + desired: desired.rawValue, + ordering: .sequentiallyConsistent + ) + guard exchanged || FileDescriptorState(rawValue: original).isClosed else { + fatalError("bug in NIO (please report): NIOFileDescritor activate failed \(original)") + } + } + + private func deactivateDescriptor(toClosed: Bool) throws -> CInt { + let peekedDescriptor = try self.peekAtDescriptorIfOpen() + // Don't worry, the above is just opportunistic. If we lose the race, we re-check below --> `!exchanged` + assert(peekedDescriptor.isOpen) + var desired = peekedDescriptor + if toClosed { + desired.close() + } else { + desired.markInUse() + } + assert(desired.rawValue != peekedDescriptor.rawValue, "\(desired.rawValue) == \(peekedDescriptor.rawValue)") + let (exchanged, originalDescriptor) = self.descriptor.compareExchange( + expected: peekedDescriptor.rawValue, + desired: desired.rawValue, + ordering: .sequentiallyConsistent + ) + + if exchanged { + assert(peekedDescriptor.rawValue == originalDescriptor) + return peekedDescriptor.descriptor + } else { + // We lost the race above, so this _will_ throw (as we're not closed). + let fauxDescriptor = try Self.interpretDescriptorValueThrowIfInUseOrNotOpen(originalDescriptor) + // This is impossible, because there are only 4 options in which the exchange above can fail + // 1. Descriptor already closed (would've thrown above) + // 2. Descriptor in use (would've thrown above) + // 3. Descriptor at illegal negative value (would've crashed above) + // 4. Descriptor a different, positive value (this is where we're at) --> memory corruption, let's crash + fatalError( + """ + bug in NIO (please report): \ + NIOFileDescriptor illegal state \ + (\(peekedDescriptor), \(originalDescriptor), \(fauxDescriptor))") + """ + ) + } + } + /// Take the ownership of the underlying file descriptor. This is similar to `close()` but the underlying file /// descriptor remains open. The caller is responsible for closing the file descriptor by some other means. /// @@ -83,27 +275,20 @@ public final class NIOFileHandle: FileDescriptor { /// /// - Returns: The underlying file descriptor, now owned by the caller. public func takeDescriptorOwnership() throws -> CInt { - guard self.isOpen else { - throw IOError(errnoCode: EBADF, reason: "can't close file (as it's not open anymore).") - } - - self.isOpen = false - return self.descriptor + try self.deactivateDescriptor(toClosed: true) } public func close() throws { - try withUnsafeFileDescriptor { fd in - try SystemCalls.close(descriptor: fd) - } - - self.isOpen = false + let descriptor = try self.deactivateDescriptor(toClosed: true) + try SystemCalls.close(descriptor: descriptor) } public func withUnsafeFileDescriptor(_ body: (CInt) throws -> T) throws -> T { - guard self.isOpen else { - throw IOError(errnoCode: EBADF, reason: "file descriptor already closed!") + let descriptor = try self.deactivateDescriptor(toClosed: false) + defer { + self.activateDescriptor(as: descriptor) } - return try body(self.descriptor) + return try body(descriptor) } } @@ -180,29 +365,75 @@ extension NIOFileHandle { /// - path: The path of the file to open. The ownership of the file descriptor is transferred to this `NIOFileHandle` and so it will be closed once `close` is called. /// - mode: Access mode. Default mode is `.read`. /// - flags: Additional POSIX flags. - public convenience init(path: String, mode: Mode = .read, flags: Flags = .default) throws { + @available( + *, + deprecated, + message: """ + Avoid using NIOFileHandle. The type is difficult to hold correctly, \ + use NIOFileSystem as a replacement API. + """ + ) + @available(*, noasync, message: "This method may block the calling thread") + public convenience init( + path: String, + mode: Mode = .read, + flags: Flags = .default + ) throws { + try self.init(_deprecatedPath: path, mode: mode, flags: flags) + } + + /// Open a new `NIOFileHandle`. This operation is blocking. + /// + /// - Parameters: + /// - path: The path of the file to open. The ownership of the file descriptor is transferred to this `NIOFileHandle` and so it will be closed once `close` is called. + /// - mode: Access mode. Default mode is `.read`. + /// - flags: Additional POSIX flags. + @available(*, noasync, message: "This method may block the calling thread") + public convenience init( + _deprecatedPath path: String, + mode: Mode = .read, + flags: Flags = .default + ) throws { #if os(Windows) let fl = mode.posixFlags | flags.posixFlags | _O_NOINHERIT #else let fl = mode.posixFlags | flags.posixFlags | O_CLOEXEC #endif let fd = try SystemCalls.open(file: path, oFlag: fl, mode: flags.posixMode) - self.init(descriptor: fd) + self.init(_deprecatedTakingOwnershipOfDescriptor: fd) } /// Open a new `NIOFileHandle`. This operation is blocking. /// /// - Parameters: /// - path: The path of the file to open. The ownership of the file descriptor is transferred to this `NIOFileHandle` and so it will be closed once `close` is called. + @available( + *, + deprecated, + message: """ + Avoid using NIOFileHandle. The type is difficult to hold correctly, \ + use NIOFileSystem as a replacement API. + """ + ) + @available(*, noasync, message: "This method may block the calling thread") public convenience init(path: String) throws { + try self.init(_deprecatedPath: path) + } + + /// Open a new `NIOFileHandle`. This operation is blocking. + /// + /// - Parameters: + /// - path: The path of the file to open. The ownership of the file descriptor is transferred to this `NIOFileHandle` and so it will be closed once `close` is called. + @available(*, noasync, message: "This method may block the calling thread") + public convenience init(_deprecatedPath path: String) throws { // This function is here because we had a function like this in NIO 2.0, and the one above doesn't quite match. Sadly we can't // really deprecate this either, because it'll be preferred to the one above in many cases. - try self.init(path: path, mode: .read, flags: .default) + try self.init(_deprecatedPath: path, mode: .read, flags: .default) } } extension NIOFileHandle: CustomStringConvertible { public var description: String { - "FileHandle { descriptor: \(self.descriptor) }" + "FileHandle { descriptor: \(FileDescriptorState(rawValue: self.descriptor.load(ordering: .relaxed)).descriptor) }" } } diff --git a/Sources/NIOCore/FileRegion.swift b/Sources/NIOCore/FileRegion.swift index fdeef77844..1b1f1c5b03 100644 --- a/Sources/NIOCore/FileRegion.swift +++ b/Sources/NIOCore/FileRegion.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -29,6 +29,10 @@ import WASILibc /// A `FileRegion` represent a readable portion usually created to be sent over the network. /// +/// - warning: The `FileRegion` API is deprecated, do not use going forward. It's not marked as `deprecated` yet such +/// that users don't get the deprecation warnings affecting their APIs everywhere. For file I/O, please use +/// the `NIOFileSystem` API. +/// /// Usually a `FileRegion` will allow the underlying transport to use `sendfile` to transfer its content and so allows transferring /// the file content without copying it into user-space at all. If the actual transport implementation really can make use of sendfile /// or if it will need to copy the content to user-space first and use `write` / `writev` is an implementation detail. That said @@ -37,9 +41,9 @@ import WASILibc /// One important note, depending your `ChannelPipeline` setup it may not be possible to use a `FileRegion` as a `ChannelHandler` may /// need access to the bytes (in a `ByteBuffer`) to transform these. /// -/// - Note: It is important to manually manage the lifetime of the `NIOFileHandle` used to create a `FileRegion`. -/// - warning: `FileRegion` objects are not thread-safe and are mutable. They also cannot be fully thread-safe as they refer to a global underlying file descriptor. -public struct FileRegion { +/// - Note: It is important to manually manage the lifetime of the ``NIOFileHandle`` used to create a ``FileRegion``. +/// - Note: As of SwiftNIO 2.77.0, `FileRegion` objects are are thread-safe and the underlying ``NIOFileHandle`` does enforce singular access. +public struct FileRegion: Sendable { /// The `NIOFileHandle` that is used by this `FileRegion`. public let fileHandle: NIOFileHandle diff --git a/Sources/NIOCore/IOData.swift b/Sources/NIOCore/IOData.swift index 293ad9b00b..bad0585b67 100644 --- a/Sources/NIOCore/IOData.swift +++ b/Sources/NIOCore/IOData.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -14,14 +14,19 @@ /// `IOData` unifies standard SwiftNIO types that are raw bytes of data; currently `ByteBuffer` and `FileRegion`. /// +/// - warning: `IOData` is a legacy API, please avoid using it as much as possible. +/// /// Many `ChannelHandler`s receive or emit bytes and in most cases this can be either a `ByteBuffer` or a `FileRegion` /// from disk. To still form a well-typed `ChannelPipeline` such handlers should receive and emit value of type `IOData`. -public enum IOData { +public enum IOData: Sendable { /// A `ByteBuffer`. case byteBuffer(ByteBuffer) /// A `FileRegion`. /// + /// - warning: `IOData.fileRegion` is a legacy API, please avoid using it. It cannot work with TLS and `FileRegion` + /// and the underlying `NIOFileHandle` objects are very difficult to hold correctly. + /// /// Sending a `FileRegion` through the `ChannelPipeline` using `write` can be useful because some `Channel`s can /// use `sendfile` to send a `FileRegion` more efficiently. case fileRegion(FileRegion) @@ -30,9 +35,6 @@ public enum IOData { /// `IOData` objects are comparable just like the values they wrap. extension IOData: Equatable {} -@available(*, unavailable) -extension IOData: Sendable {} - /// `IOData` provide a number of readable bytes. extension IOData { /// Returns the number of readable bytes in this `IOData`. diff --git a/Sources/NIOCore/Linux.swift b/Sources/NIOCore/Linux.swift index 3f944ee726..754a35afca 100644 --- a/Sources/NIOCore/Linux.swift +++ b/Sources/NIOCore/Linux.swift @@ -24,7 +24,7 @@ enum Linux { static let cfsCpuMaxPath = "/sys/fs/cgroup/cpu.max" private static func firstLineOfFile(path: String) throws -> Substring { - let fh = try NIOFileHandle(path: path) + let fh = try NIOFileHandle(_deprecatedPath: path) defer { try! fh.close() } // linux doesn't properly report /sys/fs/cgroup/* files lengths so we use a reasonable limit var buf = ByteBufferAllocator().buffer(capacity: 1024) diff --git a/Sources/NIOCrashTester/OutputGrepper.swift b/Sources/NIOCrashTester/OutputGrepper.swift index 069f182fae..45f56d179a 100644 --- a/Sources/NIOCrashTester/OutputGrepper.swift +++ b/Sources/NIOCrashTester/OutputGrepper.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2020-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2020-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -39,7 +39,9 @@ internal struct OutputGrepper { } } .takingOwnershipOfDescriptor(input: dup(processToChannel.fileHandleForReading.fileDescriptor)) - let processOutputPipe = NIOFileHandle(descriptor: dup(processToChannel.fileHandleForWriting.fileDescriptor)) + let processOutputPipe = NIOFileHandle( + _deprecatedTakingOwnershipOfDescriptor: dup(processToChannel.fileHandleForWriting.fileDescriptor) + ) processToChannel.fileHandleForReading.closeFile() processToChannel.fileHandleForWriting.closeFile() channelFuture.cascadeFailure(to: outputPromise) diff --git a/Sources/NIOHTTP1Server/main.swift b/Sources/NIOHTTP1Server/main.swift index 03f755442f..2b662bcc3d 100644 --- a/Sources/NIOHTTP1Server/main.swift +++ b/Sources/NIOHTTP1Server/main.swift @@ -422,7 +422,7 @@ private final class HTTPHandler: ChannelInboundHandler { return } let path = self.htdocsPath + "/" + path - let fileHandleAndRegion = self.fileIO.openFile(path: path, eventLoop: context.eventLoop) + let fileHandleAndRegion = self.fileIO.openFile(_deprecatedPath: path, eventLoop: context.eventLoop) fileHandleAndRegion.whenFailure { sendErrorResponse(request: request, $0) } diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index c48043651b..ff2773a2cd 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -2407,8 +2407,8 @@ extension NIOPipeBootstrap { let channelOptions = self._channelOptions let channel: PipeChannel - let inputFileHandle: NIOFileHandle? - let outputFileHandle: NIOFileHandle? + let pipeChannelInput: SelectablePipeHandle? + let pipeChannelOutput: SelectablePipeHandle? do { if let input = input { try self.validateFileDescriptorIsNotAFile(input) @@ -2417,18 +2417,18 @@ extension NIOPipeBootstrap { try self.validateFileDescriptorIsNotAFile(output) } - inputFileHandle = input.flatMap { NIOFileHandle(descriptor: $0) } - outputFileHandle = output.flatMap { NIOFileHandle(descriptor: $0) } + pipeChannelInput = input.flatMap { SelectablePipeHandle(takingOwnershipOfDescriptor: $0) } + pipeChannelOutput = output.flatMap { SelectablePipeHandle(takingOwnershipOfDescriptor: $0) } do { channel = try self.hooks.makePipeChannel( eventLoop: eventLoop as! SelectableEventLoop, - inputPipe: inputFileHandle, - outputPipe: outputFileHandle + input: pipeChannelInput, + output: pipeChannelOutput ) } catch { // Release file handles back to the caller in case of failure. - _ = try? inputFileHandle?.takeDescriptorOwnership() - _ = try? outputFileHandle?.takeDescriptorOwnership() + _ = try? pipeChannelInput?.takeDescriptorOwnership() + _ = try? pipeChannelOutput?.takeDescriptorOwnership() throw error } } catch { @@ -2447,10 +2447,10 @@ extension NIOPipeBootstrap { channel.registerAlreadyConfigured0(promise: promise) return promise.futureResult.map { result } }.flatMap { result -> EventLoopFuture in - if inputFileHandle == nil { + if pipeChannelInput == nil { return channel.close(mode: .input).map { result } } - if outputFileHandle == nil { + if pipeChannelOutput == nil { return channel.close(mode: .output).map { result } } return channel.selectableEventLoop.makeSucceededFuture(result) @@ -2476,17 +2476,17 @@ extension NIOPipeBootstrap: Sendable {} protocol NIOPipeBootstrapHooks { func makePipeChannel( eventLoop: SelectableEventLoop, - inputPipe: NIOFileHandle?, - outputPipe: NIOFileHandle? + input: SelectablePipeHandle?, + output: SelectablePipeHandle? ) throws -> PipeChannel } private struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks { func makePipeChannel( eventLoop: SelectableEventLoop, - inputPipe: NIOFileHandle?, - outputPipe: NIOFileHandle? + input: SelectablePipeHandle?, + output: SelectablePipeHandle? ) throws -> PipeChannel { - try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe) + try PipeChannel(eventLoop: eventLoop, input: input, output: output) } } diff --git a/Sources/NIOPosix/NonBlockingFileIO.swift b/Sources/NIOPosix/NonBlockingFileIO.swift index e1bb2eb113..fe3448d11a 100644 --- a/Sources/NIOPosix/NonBlockingFileIO.swift +++ b/Sources/NIOPosix/NonBlockingFileIO.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -17,6 +17,10 @@ import NIOCore /// ``NonBlockingFileIO`` is a helper that allows you to read files without blocking the calling thread. /// +/// - warning: The `NonBlockingFileIO` API is deprecated, do not use going forward. It's not marked as `deprecated` yet such +/// that users don't get the deprecation warnings affecting their APIs everywhere. For file I/O, please use +/// the `NIOFileSystem` API. +/// /// It is worth noting that `kqueue`, `epoll` or `poll` returning claiming a file is readable does not mean that the /// data is already available in the kernel's memory. In other words, a `read` from a file can still block even if /// reported as readable. This behaviour is also documented behaviour: @@ -553,9 +557,33 @@ public struct NonBlockingFileIO: Sendable { /// - path: The path of the file to be opened for reading. /// - eventLoop: The `EventLoop` on which the returned `EventLoopFuture` will fire. /// - Returns: An `EventLoopFuture` containing the `NIOFileHandle` and the `FileRegion` comprising the whole file. + @available( + *, + deprecated, + message: + "Avoid using NIOFileHandle. The type is difficult to hold correctly, use NIOFileSystem as a replacement API." + ) public func openFile(path: String, eventLoop: EventLoop) -> EventLoopFuture<(NIOFileHandle, FileRegion)> { + self.openFile(_deprecatedPath: path, eventLoop: eventLoop) + } + + /// Open the file at `path` for reading on a private thread pool which is separate from any `EventLoop` thread. + /// + /// This function will return (a future) of the `NIOFileHandle` associated with the file opened and a `FileRegion` + /// comprising of the whole file. The caller must close the returned `NIOFileHandle` when it's no longer needed. + /// + /// - Note: The reason this returns the `NIOFileHandle` and the `FileRegion` is that both the opening of a file as well as the querying of its size are blocking. + /// + /// - Parameters: + /// - path: The path of the file to be opened for reading. + /// - eventLoop: The `EventLoop` on which the returned `EventLoopFuture` will fire. + /// - Returns: An `EventLoopFuture` containing the `NIOFileHandle` and the `FileRegion` comprising the whole file. + public func openFile( + _deprecatedPath path: String, + eventLoop: EventLoop + ) -> EventLoopFuture<(NIOFileHandle, FileRegion)> { self.threadPool.runIfActive(eventLoop: eventLoop) { - let fh = try NIOFileHandle(path: path) + let fh = try NIOFileHandle(_deprecatedPath: path) do { let fr = try FileRegion(fileHandle: fh) return (fh, fr) @@ -577,14 +605,40 @@ public struct NonBlockingFileIO: Sendable { /// - flags: Additional POSIX flags. /// - eventLoop: The `EventLoop` on which the returned `EventLoopFuture` will fire. /// - Returns: An `EventLoopFuture` containing the `NIOFileHandle`. + @available( + *, + deprecated, + message: + "Avoid using NonBlockingFileIO. The type is difficult to hold correctly, use NIOFileSystem as a replacement API." + ) public func openFile( path: String, mode: NIOFileHandle.Mode, flags: NIOFileHandle.Flags = .default, eventLoop: EventLoop + ) -> EventLoopFuture { + self.openFile(_deprecatedPath: path, mode: mode, flags: flags, eventLoop: eventLoop) + } + + /// Open the file at `path` with specified access mode and POSIX flags on a private thread pool which is separate from any `EventLoop` thread. + /// + /// This function will return (a future) of the `NIOFileHandle` associated with the file opened. + /// The caller must close the returned `NIOFileHandle` when it's no longer needed. + /// + /// - Parameters: + /// - path: The path of the file to be opened for writing. + /// - mode: File access mode. + /// - flags: Additional POSIX flags. + /// - eventLoop: The `EventLoop` on which the returned `EventLoopFuture` will fire. + /// - Returns: An `EventLoopFuture` containing the `NIOFileHandle`. + public func openFile( + _deprecatedPath path: String, + mode: NIOFileHandle.Mode, + flags: NIOFileHandle.Flags = .default, + eventLoop: EventLoop ) -> EventLoopFuture { self.threadPool.runIfActive(eventLoop: eventLoop) { - try NIOFileHandle(path: path, mode: mode, flags: flags) + try NIOFileHandle(_deprecatedPath: path, mode: mode, flags: flags) } } @@ -1009,16 +1063,29 @@ extension NonBlockingFileIO { /// - path: The path of the file to be opened for reading. /// - body: operation to run with file handle and region /// - Returns: return value of operation + @available( + *, + deprecated, + message: + "Avoid using NonBlockingFileIO. The API is difficult to hold correctly, use NIOFileSystem as a replacement API." + ) @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public func withFileRegion( path: String, _ body: (_ fileRegion: FileRegion) async throws -> Result + ) async throws -> Result { + try await self.withFileRegion(_deprecatedPath: path, body) + } + + @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) + public func withFileRegion( + _deprecatedPath path: String, + _ body: (_ fileRegion: FileRegion) async throws -> Result ) async throws -> Result { let fileRegion = try await self.threadPool.runIfActive { - let fh = try NIOFileHandle(path: path) + let fh = try NIOFileHandle(_deprecatedPath: path) do { - let fr = try FileRegion(fileHandle: fh) - return UnsafeTransfer(fr) + return try FileRegion(fileHandle: fh) } catch { _ = try? fh.close() throw error @@ -1026,12 +1093,12 @@ extension NonBlockingFileIO { } let result: Result do { - result = try await body(fileRegion.wrappedValue) + result = try await body(fileRegion) } catch { - try fileRegion.wrappedValue.fileHandle.close() + try fileRegion.fileHandle.close() throw error } - try fileRegion.wrappedValue.fileHandle.close() + try fileRegion.fileHandle.close() return result } @@ -1045,24 +1112,40 @@ extension NonBlockingFileIO { /// - flags: Additional POSIX flags. /// - body: operation to run with the file handle /// - Returns: return value of operation + @available( + *, + deprecated, + message: + "Avoid using NonBlockingFileIO. The API is difficult to hold correctly, use NIOFileSystem as a replacement API." + ) @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public func withFileHandle( path: String, mode: NIOFileHandle.Mode, flags: NIOFileHandle.Flags = .default, _ body: (NIOFileHandle) async throws -> Result + ) async throws -> Result { + try await self.withFileHandle(_deprecatedPath: path, mode: mode, flags: flags, body) + } + + @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) + public func withFileHandle( + _deprecatedPath path: String, + mode: NIOFileHandle.Mode, + flags: NIOFileHandle.Flags = .default, + _ body: (NIOFileHandle) async throws -> Result ) async throws -> Result { let fileHandle = try await self.threadPool.runIfActive { - try UnsafeTransfer(NIOFileHandle(path: path, mode: mode, flags: flags)) + try NIOFileHandle(_deprecatedPath: path, mode: mode, flags: flags) } let result: Result do { - result = try await body(fileHandle.wrappedValue) + result = try await body(fileHandle) } catch { - try fileHandle.wrappedValue.close() + try fileHandle.close() throw error } - try fileHandle.wrappedValue.close() + try fileHandle.close() return result } diff --git a/Sources/NIOPosix/PendingWritesManager.swift b/Sources/NIOPosix/PendingWritesManager.swift index 83323f480f..001b05c215 100644 --- a/Sources/NIOPosix/PendingWritesManager.swift +++ b/Sources/NIOPosix/PendingWritesManager.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -434,9 +434,10 @@ final class PendingStreamWritesManager: PendingWritesManager { case .fileRegion(let file): let readerIndex = file.readerIndex let endIndex = file.endIndex - return try file.fileHandle.withUnsafeFileDescriptor { fd in - self.didWrite(itemCount: 1, result: try operation(fd, readerIndex, endIndex)) + let writeResult = try file.fileHandle.withUnsafeFileDescriptor { fd in + try operation(fd, readerIndex, endIndex) } + return self.didWrite(itemCount: 1, result: writeResult) case .byteBuffer: preconditionFailure("called \(#function) but first item to write was a ByteBuffer") } diff --git a/Sources/NIOPosix/PipeChannel.swift b/Sources/NIOPosix/PipeChannel.swift index 1a85a30359..e105a724f1 100644 --- a/Sources/NIOPosix/PipeChannel.swift +++ b/Sources/NIOPosix/PipeChannel.swift @@ -23,10 +23,10 @@ final class PipeChannel: BaseStreamSocketChannel { init( eventLoop: SelectableEventLoop, - inputPipe: NIOFileHandle?, - outputPipe: NIOFileHandle? + input: SelectablePipeHandle?, + output: SelectablePipeHandle? ) throws { - self.pipePair = try PipePair(inputFD: inputPipe, outputFD: outputPipe) + self.pipePair = try PipePair(input: input, output: output) try super.init( socket: self.pipePair, parent: nil, @@ -65,17 +65,17 @@ final class PipeChannel: BaseStreamSocketChannel { } override func register(selector: Selector, interested: SelectorEventSet) throws { - if let inputFD = self.pipePair.inputFD { + if let inputSPH = self.pipePair.input { try selector.register( - selectable: inputFD, + selectable: inputSPH, interested: interested.intersection([.read, .reset, .error]), makeRegistration: self.registrationForInput ) } - if let outputFD = self.pipePair.outputFD { + if let outputSPH = self.pipePair.output { try selector.register( - selectable: outputFD, + selectable: outputSPH, interested: interested.intersection([.write, .reset, .error]), makeRegistration: self.registrationForOutput ) @@ -83,24 +83,24 @@ final class PipeChannel: BaseStreamSocketChannel { } override func deregister(selector: Selector, mode: CloseMode) throws { - if let inputFD = self.pipePair.inputFD, (mode == .all || mode == .input) && inputFD.isOpen { - try selector.deregister(selectable: inputFD) + if let inputSPH = self.pipePair.input, (mode == .all || mode == .input) && inputSPH.isOpen { + try selector.deregister(selectable: inputSPH) } - if let outputFD = self.pipePair.outputFD, (mode == .all || mode == .output) && outputFD.isOpen { - try selector.deregister(selectable: outputFD) + if let outputSPH = self.pipePair.output, (mode == .all || mode == .output) && outputSPH.isOpen { + try selector.deregister(selectable: outputSPH) } } override func reregister(selector: Selector, interested: SelectorEventSet) throws { - if let inputFD = self.pipePair.inputFD, inputFD.isOpen { + if let inputSPH = self.pipePair.input, inputSPH.isOpen { try selector.reregister( - selectable: inputFD, + selectable: inputSPH, interested: interested.intersection([.read, .reset, .error]) ) } - if let outputFD = self.pipePair.outputFD, outputFD.isOpen { + if let outputSPH = self.pipePair.output, outputSPH.isOpen { try selector.reregister( - selectable: outputFD, + selectable: outputSPH, interested: interested.intersection([.write, .reset, .error]) ) } @@ -108,19 +108,19 @@ final class PipeChannel: BaseStreamSocketChannel { override func readEOF() { super.readEOF() - guard let inputFD = self.pipePair.inputFD, inputFD.isOpen else { + guard let inputSPH = self.pipePair.input, inputSPH.isOpen else { return } try! self.selectableEventLoop.deregister(channel: self, mode: .input) - try! inputFD.close() + try! inputSPH.close() } override func writeEOF() { - guard let outputFD = self.pipePair.outputFD, outputFD.isOpen else { + guard let outputSPH = self.pipePair.output, outputSPH.isOpen else { return } try! self.selectableEventLoop.deregister(channel: self, mode: .output) - try! outputFD.close() + try! outputSPH.close() } override func shutdownSocket(mode: CloseMode) throws { diff --git a/Sources/NIOPosix/PipePair.swift b/Sources/NIOPosix/PipePair.swift index 76b58c2f25..ffa76a2617 100644 --- a/Sources/NIOPosix/PipePair.swift +++ b/Sources/NIOPosix/PipePair.swift @@ -13,47 +13,72 @@ //===----------------------------------------------------------------------===// import NIOCore -struct SelectableFileHandle { - var handle: NIOFileHandle +final class SelectablePipeHandle { + var fileDescriptor: CInt var isOpen: Bool { - handle.isOpen + self.fileDescriptor >= 0 } - init(_ handle: NIOFileHandle) { - self.handle = handle + init(takingOwnershipOfDescriptor fd: CInt) { + precondition(fd >= 0) + self.fileDescriptor = fd } func close() throws { - try handle.close() + let fd = try self.takeDescriptorOwnership() + try Posix.close(descriptor: fd) + } + + func takeDescriptorOwnership() throws -> CInt { + guard self.isOpen else { + throw IOError(errnoCode: EBADF, reason: "SelectablePipeHandle already closed [in close]") + } + defer { + self.fileDescriptor = -1 + } + return self.fileDescriptor + } + + deinit { + assert(!self.isOpen, "leaking \(self)") } } -extension SelectableFileHandle: Selectable { +extension SelectablePipeHandle: Selectable { func withUnsafeHandle(_ body: (CInt) throws -> T) throws -> T { - try self.handle.withUnsafeFileDescriptor(body) + guard self.isOpen else { + throw IOError(errnoCode: EBADF, reason: "SelectablePipeHandle already closed [in wUH]") + } + return try body(self.fileDescriptor) + } +} + +extension SelectablePipeHandle: CustomStringConvertible { + public var description: String { + "SelectableFileHandle(isOpen: \(self.isOpen), fd: \(self.fileDescriptor))" } } final class PipePair: SocketProtocol { - typealias SelectableType = SelectableFileHandle + typealias SelectableType = SelectablePipeHandle - let inputFD: SelectableFileHandle? - let outputFD: SelectableFileHandle? + let input: SelectablePipeHandle? + let output: SelectablePipeHandle? - init(inputFD: NIOFileHandle?, outputFD: NIOFileHandle?) throws { - self.inputFD = inputFD.flatMap { SelectableFileHandle($0) } - self.outputFD = outputFD.flatMap { SelectableFileHandle($0) } + init(input: SelectablePipeHandle?, output: SelectablePipeHandle?) throws { + self.input = input + self.output = output try self.ignoreSIGPIPE() - for fileHandle in [inputFD, outputFD].compactMap({ $0 }) { - try fileHandle.withUnsafeFileDescriptor { - try NIOFileHandle.setNonBlocking(fileDescriptor: $0) + for fh in [input, output].compactMap({ $0 }) { + try fh.withUnsafeHandle { fd in + try NIOFileHandle.setNonBlocking(fileDescriptor: fd) } } } func ignoreSIGPIPE() throws { - for fileHandle in [self.inputFD, self.outputFD].compactMap({ $0 }) { + for fileHandle in [self.input, self.output].compactMap({ $0 }) { try fileHandle.withUnsafeHandle { try PipePair.ignoreSIGPIPE(descriptor: $0) } @@ -61,7 +86,7 @@ final class PipePair: SocketProtocol { } var description: String { - "PipePair { in=\(String(describing: inputFD)), out=\(String(describing: inputFD)) }" + "PipePair { in=\(String(describing: self.input)), out=\(String(describing: self.output)) }" } func connect(to address: SocketAddress) throws -> Bool { @@ -73,28 +98,28 @@ final class PipePair: SocketProtocol { } func write(pointer: UnsafeRawBufferPointer) throws -> IOResult { - guard let outputFD = self.outputFD else { - fatalError("Internal inconsistency inside NIO. Please file a bug") + guard let outputSPH = self.output else { + fatalError("Internal inconsistency inside NIO: outputSPH closed on write. Please file a bug") } - return try outputFD.withUnsafeHandle { + return try outputSPH.withUnsafeHandle { try Posix.write(descriptor: $0, pointer: pointer.baseAddress!, size: pointer.count) } } func writev(iovecs: UnsafeBufferPointer) throws -> IOResult { - guard let outputFD = self.outputFD else { - fatalError("Internal inconsistency inside NIO. Please file a bug") + guard let outputSPH = self.output else { + fatalError("Internal inconsistency inside NIO: outputSPH closed on writev. Please file a bug") } - return try outputFD.withUnsafeHandle { + return try outputSPH.withUnsafeHandle { try Posix.writev(descriptor: $0, iovecs: iovecs) } } func read(pointer: UnsafeMutableRawBufferPointer) throws -> IOResult { - guard let inputFD = self.inputFD else { - fatalError("Internal inconsistency inside NIO. Please file a bug") + guard let inputSPH = self.input else { + fatalError("Internal inconsistency inside NIO: inputSPH closed on read. Please file a bug") } - return try inputFD.withUnsafeHandle { + return try inputSPH.withUnsafeHandle { try Posix.read(descriptor: $0, pointer: pointer.baseAddress!, size: pointer.count) } } @@ -132,16 +157,16 @@ final class PipePair: SocketProtocol { func shutdown(how: Shutdown) throws { switch how { case .RD: - try self.inputFD?.close() + try self.input?.close() case .WR: - try self.outputFD?.close() + try self.output?.close() case .RDWR: try self.close() } } var isOpen: Bool { - self.inputFD?.isOpen ?? false || self.outputFD?.isOpen ?? false + self.input?.isOpen ?? false || self.output?.isOpen ?? false } func close() throws { @@ -149,12 +174,12 @@ final class PipePair: SocketProtocol { throw ChannelError._alreadyClosed } let r1 = Result { - if let inputFD = self.inputFD, inputFD.isOpen { + if let inputFD = self.input, inputFD.isOpen { try inputFD.close() } } let r2 = Result { - if let outputFD = self.outputFD, outputFD.isOpen { + if let outputFD = self.output, outputFD.isOpen { try outputFD.close() } } diff --git a/Sources/NIOPosix/SelectorEpoll.swift b/Sources/NIOPosix/SelectorEpoll.swift index 5da3978b8c..832dc41dff 100644 --- a/Sources/NIOPosix/SelectorEpoll.swift +++ b/Sources/NIOPosix/SelectorEpoll.swift @@ -167,8 +167,8 @@ extension Selector: _SelectorBackendProtocol { assert(self.timerFD == -1, "self.timerFD == \(self.timerFD) in deinitAssertions0, forgot close?") } - func register0( - selectable: S, + func register0( + selectableFD: CInt, fileDescriptor: CInt, interested: SelectorEventSet, registrationID: SelectorRegistrationID @@ -180,8 +180,8 @@ extension Selector: _SelectorBackendProtocol { try Epoll.epoll_ctl(epfd: self.selectorFD, op: Epoll.EPOLL_CTL_ADD, fd: fileDescriptor, event: &ev) } - func reregister0( - selectable: S, + func reregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, newInterested: SelectorEventSet, @@ -194,8 +194,8 @@ extension Selector: _SelectorBackendProtocol { _ = try Epoll.epoll_ctl(epfd: self.selectorFD, op: Epoll.EPOLL_CTL_MOD, fd: fileDescriptor, event: &ev) } - func deregister0( - selectable: S, + func deregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, registrationID: SelectorRegistrationID diff --git a/Sources/NIOPosix/SelectorGeneric.swift b/Sources/NIOPosix/SelectorGeneric.swift index a587079e6b..cb2626c76c 100644 --- a/Sources/NIOPosix/SelectorGeneric.swift +++ b/Sources/NIOPosix/SelectorGeneric.swift @@ -122,27 +122,29 @@ protocol _SelectorBackendProtocol { associatedtype R: Registration func initialiseState0() throws func deinitAssertions0() // allows actual implementation to run some assertions as part of the class deinit - func register0( - selectable: S, + func register0( + selectableFD: CInt, fileDescriptor: CInt, interested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws - func reregister0( - selectable: S, + func reregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, newInterested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws - func deregister0( - selectable: S, + func deregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws + // attention, this may (will!) be called from outside the event loop, ie. can't access mutable shared state (such as `self.open`) func wakeup0() throws + /// Apply the given `SelectorStrategy` and execute `body` once it's complete (which may produce `SelectorEvent`s to handle). /// /// - Parameters: @@ -288,7 +290,7 @@ internal class Selector { try selectable.withUnsafeHandle { fd in assert(registrations[Int(fd)] == nil) try self.register0( - selectable: selectable, + selectableFD: fd, fileDescriptor: fd, interested: interested, registrationID: self.registrationID @@ -315,7 +317,7 @@ internal class Selector { try selectable.withUnsafeHandle { fd in var reg = registrations[Int(fd)]! try self.reregister0( - selectable: selectable, + selectableFD: fd, fileDescriptor: fd, oldInterested: reg.interested, newInterested: interested, @@ -343,7 +345,7 @@ internal class Selector { return } try self.deregister0( - selectable: selectable, + selectableFD: fd, fileDescriptor: fd, oldInterested: reg.interested, registrationID: reg.registrationID diff --git a/Sources/NIOPosix/SelectorKqueue.swift b/Sources/NIOPosix/SelectorKqueue.swift index 2b2b31edac..4c45dd1599 100644 --- a/Sources/NIOPosix/SelectorKqueue.swift +++ b/Sources/NIOPosix/SelectorKqueue.swift @@ -163,8 +163,8 @@ extension Selector: _SelectorBackendProtocol { } } - private func kqueueUpdateEventNotifications( - selectable: S, + private func kqueueUpdateEventNotifications( + selectableFD: CInt, interested: SelectorEventSet, oldInterested: SelectorEventSet?, registrationID: SelectorRegistrationID @@ -175,14 +175,12 @@ extension Selector: _SelectorBackendProtocol { assert(interested.contains(.reset)) assert(oldInterested?.contains(.reset) ?? true) - try selectable.withUnsafeHandle { - try newKQueueFilters.calculateKQueueFilterSetChanges( - previousKQueueFilterSet: oldKQueueFilters, - fileDescriptor: $0, - registrationID: registrationID, - kqueueApplyEventChangeSet - ) - } + try newKQueueFilters.calculateKQueueFilterSetChanges( + previousKQueueFilterSet: oldKQueueFilters, + fileDescriptor: selectableFD, + registrationID: registrationID, + kqueueApplyEventChangeSet + ) } func initialiseState0() throws { @@ -205,44 +203,44 @@ extension Selector: _SelectorBackendProtocol { func deinitAssertions0() { } - func register0( - selectable: S, + func register0( + selectableFD: CInt, fileDescriptor: CInt, interested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws { try kqueueUpdateEventNotifications( - selectable: selectable, + selectableFD: selectableFD, interested: interested, oldInterested: nil, registrationID: registrationID ) } - func reregister0( - selectable: S, + func reregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, newInterested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws { try kqueueUpdateEventNotifications( - selectable: selectable, + selectableFD: selectableFD, interested: newInterested, oldInterested: oldInterested, registrationID: registrationID ) } - func deregister0( - selectable: S, + func deregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, registrationID: SelectorRegistrationID ) throws { try kqueueUpdateEventNotifications( - selectable: selectable, - interested: [.reset, .error], + selectableFD: selectableFD, + interested: .reset, oldInterested: oldInterested, registrationID: registrationID ) diff --git a/Sources/NIOPosix/SelectorUring.swift b/Sources/NIOPosix/SelectorUring.swift index 2bc9fdb379..a92608b2dd 100644 --- a/Sources/NIOPosix/SelectorUring.swift +++ b/Sources/NIOPosix/SelectorUring.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2021-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -131,8 +131,8 @@ extension Selector: _SelectorBackendProtocol { assert(self.eventFD == -1, "self.eventFD == \(self.eventFD) on deinitAssertions0 deinit, forgot close?") } - func register0( - selectable: S, + func register0( + selectableFD: CInt, fileDescriptor: CInt, interested: SelectorEventSet, registrationID: SelectorRegistrationID @@ -150,8 +150,8 @@ extension Selector: _SelectorBackendProtocol { ) } - func reregister0( - selectable: S, + func reregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, newInterested: SelectorEventSet, @@ -190,8 +190,8 @@ extension Selector: _SelectorBackendProtocol { } } - func deregister0( - selectable: S, + func deregister0( + selectableFD: CInt, fileDescriptor: CInt, oldInterested: SelectorEventSet, registrationID: SelectorRegistrationID diff --git a/Tests/NIOCoreTests/BaseObjectsTest.swift b/Tests/NIOCoreTests/BaseObjectsTest.swift index 74f8a91563..cf541487bf 100644 --- a/Tests/NIOCoreTests/BaseObjectsTest.swift +++ b/Tests/NIOCoreTests/BaseObjectsTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -52,7 +52,7 @@ class BaseObjectTest: XCTestCase { } func testNIOFileRegionConversion() { - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let expected = FileRegion(fileHandle: handle, readerIndex: 1, endIndex: 2) defer { // fake descriptor, so shouldn't be closed. @@ -74,7 +74,7 @@ class BaseObjectTest: XCTestCase { } func testBadConversions() { - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let bb = ByteBufferAllocator().buffer(capacity: 1024) let fr = FileRegion(fileHandle: handle, readerIndex: 1, endIndex: 2) defer { @@ -95,7 +95,7 @@ class BaseObjectTest: XCTestCase { } func testFileRegionFromIOData() { - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let expected = FileRegion(fileHandle: handle, readerIndex: 1, endIndex: 2) defer { // fake descriptor, so shouldn't be closed. @@ -106,7 +106,7 @@ class BaseObjectTest: XCTestCase { } func testIODataEquals() { - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) var bb1 = ByteBufferAllocator().buffer(capacity: 1024) let bb2 = ByteBufferAllocator().buffer(capacity: 1024) bb1.writeString("hello") diff --git a/Tests/NIOCoreTests/NIOAnyDebugTest.swift b/Tests/NIOCoreTests/NIOAnyDebugTest.swift index 8d9b6f4aa3..3aa49362e2 100644 --- a/Tests/NIOCoreTests/NIOAnyDebugTest.swift +++ b/Tests/NIOCoreTests/NIOAnyDebugTest.swift @@ -27,7 +27,7 @@ class NIOAnyDebugTest: XCTestCase { "ByteBuffer: [627974652062756666657220737472696e67](18 bytes)" ) - let fileHandle = NIOFileHandle(descriptor: 1) + let fileHandle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: 1) defer { XCTAssertNoThrow(_ = try fileHandle.takeDescriptorOwnership()) } diff --git a/Tests/NIOCoreTests/XCTest+Extensions.swift b/Tests/NIOCoreTests/XCTest+Extensions.swift index 0ae75cdb49..0fc5c8b7d4 100644 --- a/Tests/NIOCoreTests/XCTest+Extensions.swift +++ b/Tests/NIOCoreTests/XCTest+Extensions.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -62,7 +62,7 @@ func withTemporaryFile(content: String? = nil, _ body: (NIOCore.NIOFileHandle XCTAssertNoThrow(try FileManager.default.removeItem(atPath: temporaryFilePath)) } - let fileHandle = try NIOFileHandle(path: temporaryFilePath, mode: [.read, .write]) + let fileHandle = try NIOFileHandle(_deprecatedPath: temporaryFilePath, mode: [.read, .write]) defer { XCTAssertNoThrow(try fileHandle.close()) } diff --git a/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift b/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift index ce5e150077..56c09b67f8 100644 --- a/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift +++ b/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift @@ -232,7 +232,7 @@ class EmbeddedChannelTest: XCTestCase { let buffer = channel.allocator.buffer(capacity: 0) let ioData = IOData.byteBuffer(buffer) - let fileHandle = NIOFileHandle(descriptor: -1) + let fileHandle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let fileRegion = FileRegion(fileHandle: fileHandle, readerIndex: 0, endIndex: 0) defer { XCTAssertNoThrow(_ = try fileHandle.takeDescriptorOwnership()) @@ -387,7 +387,7 @@ class EmbeddedChannelTest: XCTestCase { let channel = EmbeddedChannel() let buffer = ByteBufferAllocator().buffer(capacity: 5) let socketAddress = try SocketAddress(unixDomainSocketPath: "path") - let handle = NIOFileHandle(descriptor: 1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: 1) let fileRegion = FileRegion(fileHandle: handle, readerIndex: 1, endIndex: 2) defer { // fake descriptor, so shouldn't be closed. diff --git a/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift b/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift index a9f56d4adb..28a42923a7 100644 --- a/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -98,7 +98,7 @@ class HTTPServerClientTest: XCTestCase { let content = buffer.getData(at: 0, length: buffer.readableBytes)! XCTAssertNoThrow(try content.write(to: URL(fileURLWithPath: filePath))) - let fh = try! NIOFileHandle(path: filePath) + let fh = try! NIOFileHandle(_deprecatedPath: filePath) let region = FileRegion( fileHandle: fh, readerIndex: 0, diff --git a/Tests/NIOPosixTests/BootstrapTest.swift b/Tests/NIOPosixTests/BootstrapTest.swift index cd75b45a36..8c64a578e3 100644 --- a/Tests/NIOPosixTests/BootstrapTest.swift +++ b/Tests/NIOPosixTests/BootstrapTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -451,26 +451,25 @@ class BootstrapTest: XCTestCase { let eventLoop = self.group.next() - eventLoop.execute { - do { + XCTAssertNoThrow( + try eventLoop.submit { let pipe = Pipe() - let readHandle = NIOFileHandle(descriptor: pipe.fileHandleForReading.fileDescriptor) - let writeHandle = NIOFileHandle(descriptor: pipe.fileHandleForWriting.fileDescriptor) - _ = NIOPipeBootstrap(group: self.group) + defer { + XCTAssertNoThrow(try pipe.fileHandleForReading.close()) + XCTAssertNoThrow(try pipe.fileHandleForWriting.close()) + } + return NIOPipeBootstrap(group: self.group) .takingOwnershipOfDescriptors( - input: try readHandle.takeDescriptorOwnership(), - output: try writeHandle.takeDescriptorOwnership() + input: dup(pipe.fileHandleForReading.fileDescriptor), + output: dup(pipe.fileHandleForWriting.fileDescriptor) ) .flatMap({ channel in channel.close() }).always({ _ in testGrp.leave() }) - } catch { - XCTFail("Failed to bootstrap pipechannel in eventloop: \(error)") - testGrp.leave() - } - } + }.wait() + ) testGrp.wait() } @@ -777,9 +776,9 @@ class BootstrapTest: XCTestCase { struct NIOPipeBootstrapHooksChannelFail: NIOPipeBootstrapHooks { func makePipeChannel( eventLoop: NIOPosix.SelectableEventLoop, - inputPipe: NIOCore.NIOFileHandle?, - outputPipe: NIOCore.NIOFileHandle? - ) throws -> NIOPosix.PipeChannel { + input: SelectablePipeHandle?, + output: SelectablePipeHandle? + ) throws -> PipeChannel { throw IOError(errnoCode: EBADF, reason: "testing") } } @@ -790,7 +789,7 @@ class BootstrapTest: XCTestCase { } let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) defer { - try! elg.syncShutdownGracefully() + XCTAssertNoThrow(try elg.syncShutdownGracefully()) } let bootstrap = NIOPipeBootstrap(validatingGroup: elg, hooks: NIOPipeBootstrapHooksChannelFail()) diff --git a/Tests/NIOPosixTests/ChannelPipelineTest.swift b/Tests/NIOPosixTests/ChannelPipelineTest.swift index e813328981..9226d7a0a6 100644 --- a/Tests/NIOPosixTests/ChannelPipelineTest.swift +++ b/Tests/NIOPosixTests/ChannelPipelineTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -320,7 +320,7 @@ class ChannelPipelineTest: XCTestCase { XCTAssertTrue(loop.inEventLoop) do { - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let fr = FileRegion(fileHandle: handle, readerIndex: 0, endIndex: 0) defer { // fake descriptor, so shouldn't be closed. diff --git a/Tests/NIOPosixTests/ChannelTests.swift b/Tests/NIOPosixTests/ChannelTests.swift index a1df4bee11..8774c4f6a5 100644 --- a/Tests/NIOPosixTests/ChannelTests.swift +++ b/Tests/NIOPosixTests/ChannelTests.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -776,7 +776,7 @@ public final class ChannelTests: XCTestCase { ) buffer.clear() buffer.writeBytes([UInt8](repeating: 0xff, count: 1)) - let handle = NIOFileHandle(descriptor: -1) + let handle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) defer { // fake file handle, so don't actually close XCTAssertNoThrow(try handle.takeDescriptorOwnership()) @@ -962,8 +962,8 @@ public final class ChannelTests: XCTestCase { try withPendingStreamWritesManager { pwm in let ps: [EventLoopPromise] = (0..<2).map { (_: Int) in el.makePromise() } - let fh1 = NIOFileHandle(descriptor: -1) - let fh2 = NIOFileHandle(descriptor: -2) + let fh1 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) + let fh2 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -2) let fr1 = FileRegion(fileHandle: fh1, readerIndex: 12, endIndex: 14) let fr2 = FileRegion(fileHandle: fh2, readerIndex: 0, endIndex: 2) defer { @@ -1027,7 +1027,7 @@ public final class ChannelTests: XCTestCase { try withPendingStreamWritesManager { pwm in let ps: [EventLoopPromise] = (0..<1).map { (_: Int) in el.makePromise() } - let fh = NIOFileHandle(descriptor: -1) + let fh = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let fr = FileRegion(fileHandle: fh, readerIndex: 99, endIndex: 99) defer { // fake descriptor, so shouldn't be closed. @@ -1061,8 +1061,8 @@ public final class ChannelTests: XCTestCase { try withPendingStreamWritesManager { pwm in let ps: [EventLoopPromise] = (0..<5).map { (_: Int) in el.makePromise() } - let fh1 = NIOFileHandle(descriptor: -1) - let fh2 = NIOFileHandle(descriptor: -1) + let fh1 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) + let fh2 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let fr1 = FileRegion(fileHandle: fh1, readerIndex: 99, endIndex: 99) let fr2 = FileRegion(fileHandle: fh1, readerIndex: 0, endIndex: 10) defer { @@ -1320,7 +1320,7 @@ public final class ChannelTests: XCTestCase { try withPendingStreamWritesManager { pwm in let ps: [EventLoopPromise] = (0..<1).map { (_: Int) in el.makePromise() } - let fh = NIOFileHandle(descriptor: -1) + let fh = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: -1) let fr = FileRegion(fileHandle: fh, readerIndex: 0, endIndex: 8192) defer { // fake descriptor, so shouldn't be closed. diff --git a/Tests/NIOPosixTests/FileRegionTest.swift b/Tests/NIOPosixTests/FileRegionTest.swift index 764ecfff39..4e06a86ee3 100644 --- a/Tests/NIOPosixTests/FileRegionTest.swift +++ b/Tests/NIOPosixTests/FileRegionTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -60,7 +60,7 @@ class FileRegionTest: XCTestCase { try withTemporaryFile { _, filePath in try content.write(toFile: filePath, atomically: false, encoding: .ascii) try clientChannel.eventLoop.submit { - try NIOFileHandle(path: filePath) + try NIOFileHandle(_deprecatedPath: filePath) }.flatMap { (handle: NIOFileHandle) in let fr = FileRegion(fileHandle: handle, readerIndex: 0, endIndex: bytes.count) let promise = clientChannel.eventLoop.makePromise(of: Void.self) @@ -118,7 +118,7 @@ class FileRegionTest: XCTestCase { try "".write(toFile: filePath, atomically: false, encoding: .ascii) try clientChannel.eventLoop.submit { - try NIOFileHandle(path: filePath) + try NIOFileHandle(_deprecatedPath: filePath) }.flatMap { (handle: NIOFileHandle) in let fr = FileRegion(fileHandle: handle, readerIndex: 0, endIndex: 0) var futures: [EventLoopFuture] = [] @@ -180,8 +180,8 @@ class FileRegionTest: XCTestCase { try content.write(toFile: filePath, atomically: false, encoding: .ascii) let future = clientChannel.eventLoop.submit { - let fh1 = try NIOFileHandle(path: filePath) - let fh2 = try NIOFileHandle(path: filePath) + let fh1 = try NIOFileHandle(_deprecatedPath: filePath) + let fh2 = try NIOFileHandle(_deprecatedPath: filePath) return (fh1, fh2) }.flatMap { (fh1, fh2) in let fr1 = FileRegion(fileHandle: fh1, readerIndex: 0, endIndex: bytes.count) @@ -229,7 +229,7 @@ class FileRegionTest: XCTestCase { func testWholeFileFileRegion() throws { try withTemporaryFile(content: "hello") { fd, path in - let handle = try NIOFileHandle(path: path) + let handle = try NIOFileHandle(_deprecatedPath: path) let region = try FileRegion(fileHandle: handle) defer { XCTAssertNoThrow(try handle.close()) @@ -242,7 +242,7 @@ class FileRegionTest: XCTestCase { func testWholeEmptyFileFileRegion() throws { try withTemporaryFile(content: "") { _, path in - let handle = try NIOFileHandle(path: path) + let handle = try NIOFileHandle(_deprecatedPath: path) let region = try FileRegion(fileHandle: handle) defer { XCTAssertNoThrow(try handle.close()) diff --git a/Tests/NIOPosixTests/NIOFileHandleTest.swift b/Tests/NIOPosixTests/NIOFileHandleTest.swift new file mode 100644 index 0000000000..6493914c52 --- /dev/null +++ b/Tests/NIOPosixTests/NIOFileHandleTest.swift @@ -0,0 +1,171 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2024 Apple Inc. and the SwiftNIO project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of SwiftNIO project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import Dispatch +import NIOPosix +import XCTest + +@testable import NIOCore + +final class NIOFileHandleTest: XCTestCase { + func testOpenCloseWorks() throws { + let pipeFDs = try Self.makePipe() + let fh1 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: pipeFDs.0) + XCTAssertTrue(fh1.isOpen) + defer { + XCTAssertTrue(fh1.isOpen) + XCTAssertNoThrow(try fh1.close()) + XCTAssertFalse(fh1.isOpen) + } + let fh2 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: pipeFDs.1) + XCTAssertTrue(fh2.isOpen) + defer { + XCTAssertTrue(fh2.isOpen) + XCTAssertNoThrow(try fh2.close()) + XCTAssertFalse(fh2.isOpen) + } + XCTAssertTrue(fh1.isOpen) + XCTAssertTrue(fh2.isOpen) + } + + func testCloseStorm() throws { + for _ in 0..<1000 { + let pipeFDs = try Self.makePipe() + let fh1 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: pipeFDs.0) + let fh2 = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: pipeFDs.1) + + let threads = 32 + let threadReadySems = (0..= 0) + usleep(.random(in: 0..<10)) + } + case 3: + try fh2.withUnsafeFileDescriptor { fd in + precondition(fd >= 0) + usleep(.random(in: 0..<10)) + } + default: + fatalError("impossible") + } + } catch let error as IOError where error.errnoCode == EBADF || error.errnoCode == EBUSY { + // expected + } catch { + XCTFail("unexpected error \(error)") + } + } + } + + for threadReadySem in threadReadySems { + threadReadySem.wait() + } + for threadGoSem in threadGoSems { + threadGoSem.signal() + } + allDoneGroup.wait() + for fh in [fh1, fh2] { + // They may or may not be closed, depends on races above. + do { + try fh.close() + } catch let error as IOError where error.errnoCode == EBADF { + // expected + } + } + XCTAssertFalse(fh1.isOpen) + XCTAssertFalse(fh2.isOpen) + } + } + + // MARK: - Helpers + struct POSIXError: Error { + var what: String + var errnoCode: CInt + } + + private static func makePipe() throws -> (CInt, CInt) { + var pipeFDs: [CInt] = [-1, -1] + let err = pipeFDs.withUnsafeMutableBufferPointer { pipePtr in + pipe(pipePtr.baseAddress!) + } + guard err == 0 else { + throw POSIXError(what: "pipe", errnoCode: errno) + } + return (pipeFDs[0], pipeFDs[1]) + } +} diff --git a/Tests/NIOPosixTests/NonBlockingFileIOTest.swift b/Tests/NIOPosixTests/NonBlockingFileIOTest.swift index 405f7e84b7..26ab80d614 100644 --- a/Tests/NIOPosixTests/NonBlockingFileIOTest.swift +++ b/Tests/NIOPosixTests/NonBlockingFileIOTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -634,7 +634,7 @@ class NonBlockingFileIOTest: XCTestCase { func testFileOpenWorks() throws { let content = "123" try withTemporaryFile(content: content) { (fileHandle, path) -> Void in - try self.fileIO.openFile(path: path, eventLoop: self.eventLoop).flatMapThrowing { vals in + try self.fileIO.openFile(_deprecatedPath: path, eventLoop: self.eventLoop).flatMapThrowing { vals in let (fh, fr) = vals try fh.withUnsafeFileDescriptor { fd in XCTAssertGreaterThanOrEqual(fd, 0) @@ -650,7 +650,7 @@ class NonBlockingFileIOTest: XCTestCase { func testFileOpenWorksWithEmptyFile() throws { let content = "" try withTemporaryFile(content: content) { (fileHandle, path) -> Void in - try self.fileIO.openFile(path: path, eventLoop: self.eventLoop).flatMapThrowing { vals in + try self.fileIO.openFile(_deprecatedPath: path, eventLoop: self.eventLoop).flatMapThrowing { vals in let (fh, fr) = vals try fh.withUnsafeFileDescriptor { fd in XCTAssertGreaterThanOrEqual(fd, 0) @@ -666,7 +666,7 @@ class NonBlockingFileIOTest: XCTestCase { func testFileOpenFails() throws { do { try self.fileIO.openFile( - path: "/dev/null/this/does/not/exist", + _deprecatedPath: "/dev/null/this/does/not/exist", eventLoop: self.eventLoop ).map { _ in }.wait() XCTFail("should've thrown") @@ -681,7 +681,7 @@ class NonBlockingFileIOTest: XCTestCase { XCTAssertNoThrow( try withTemporaryDirectory { dir in try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -694,7 +694,7 @@ class NonBlockingFileIOTest: XCTestCase { XCTAssertThrowsError( try withTemporaryDirectory { dir in try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .default, eventLoop: self.eventLoop @@ -709,7 +709,7 @@ class NonBlockingFileIOTest: XCTestCase { XCTAssertNoThrow( try withTemporaryDirectory { dir in let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -734,7 +734,7 @@ class NonBlockingFileIOTest: XCTestCase { XCTAssertNoThrow( try withTemporaryDirectory { dir in let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -761,7 +761,7 @@ class NonBlockingFileIOTest: XCTestCase { // open 1 + write try { let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -782,7 +782,7 @@ class NonBlockingFileIOTest: XCTestCase { // open 2 + write again + read try { let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .default, eventLoop: self.eventLoop @@ -826,7 +826,7 @@ class NonBlockingFileIOTest: XCTestCase { // open 1 + write try { let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -847,7 +847,7 @@ class NonBlockingFileIOTest: XCTestCase { // open 2 (with truncation) + write again + read try { let fileHandle = try self.fileIO!.openFile( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .posix(flags: O_TRUNC, mode: 0), eventLoop: self.eventLoop @@ -1076,7 +1076,7 @@ class NonBlockingFileIOTest: XCTestCase { let expectation = XCTestExpectation(description: "Opened file") let threadPool = NIOThreadPool(numberOfThreads: 1) let fileIO = NonBlockingFileIO(threadPool: threadPool) - fileIO.openFile(path: path, eventLoop: eventLoopGroup.next()).whenFailure { (error) in + fileIO.openFile(_deprecatedPath: path, eventLoop: eventLoopGroup.next()).whenFailure { (error) in XCTAssertTrue(error is NIOThreadPoolError.ThreadPoolInactive) expectation.fulfill() } @@ -1166,7 +1166,7 @@ class NonBlockingFileIOTest: XCTestCase { try withTemporaryDirectory { path in let file = "\(path)/file" let handle = try self.fileIO.openFile( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -1186,7 +1186,7 @@ class NonBlockingFileIOTest: XCTestCase { try withTemporaryDirectory { path in let file = "\(path)/file" let handle = try self.fileIO.openFile( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -1216,7 +1216,7 @@ class NonBlockingFileIOTest: XCTestCase { try withTemporaryDirectory { path in let file = "\(path)/file" let handle = try self.fileIO.openFile( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation(), eventLoop: self.eventLoop @@ -1557,7 +1557,7 @@ extension NonBlockingFileIOTest { func testAsyncFileOpenWorks() async throws { let content = "123" try await withTemporaryFile(content: content) { (fileHandle, path) -> Void in - try await self.fileIO.withFileRegion(path: path) { fr in + try await self.fileIO.withFileRegion(_deprecatedPath: path) { fr in try fr.fileHandle.withUnsafeFileDescriptor { fd in XCTAssertGreaterThanOrEqual(fd, 0) } @@ -1571,7 +1571,7 @@ extension NonBlockingFileIOTest { func testAsyncFileOpenWorksWithEmptyFile() async throws { let content = "" try await withTemporaryFile(content: content) { (fileHandle, path) -> Void in - try await self.fileIO.withFileRegion(path: path) { fr in + try await self.fileIO.withFileRegion(_deprecatedPath: path) { fr in try fr.fileHandle.withUnsafeFileDescriptor { fd in XCTAssertGreaterThanOrEqual(fd, 0) } @@ -1584,7 +1584,7 @@ extension NonBlockingFileIOTest { func testAsyncFileOpenFails() async throws { do { - _ = try await self.fileIO.withFileRegion(path: "/dev/null/this/does/not/exist") { _ in } + _ = try await self.fileIO.withFileRegion(_deprecatedPath: "/dev/null/this/does/not/exist") { _ in } XCTFail("should've thrown") } catch let e as IOError where e.errnoCode == ENOTDIR { // OK @@ -1596,7 +1596,7 @@ extension NonBlockingFileIOTest { func testAsyncOpeningFilesForWriting() async throws { try await withTemporaryDirectory { dir in try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .allowFileCreation() ) { _ in } @@ -1607,7 +1607,7 @@ extension NonBlockingFileIOTest { do { try await withTemporaryDirectory { dir in try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .default ) { _ in } @@ -1621,7 +1621,7 @@ extension NonBlockingFileIOTest { func testAsyncOpeningFilesForWritingDoesNotAllowReading() async throws { try await withTemporaryDirectory { dir in try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: .write, flags: .allowFileCreation() ) { fileHandle in @@ -1641,7 +1641,7 @@ extension NonBlockingFileIOTest { func testAsyncOpeningFilesForWritingAndReading() async throws { try await withTemporaryDirectory { dir in try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation() ) { fileHandle in @@ -1663,7 +1663,7 @@ extension NonBlockingFileIOTest { // open 1 + write do { try await self.fileIO.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation() ) { fileHandle in @@ -1682,7 +1682,7 @@ extension NonBlockingFileIOTest { // open 2 + write again + read do { try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .default ) { fileHandle in @@ -1721,7 +1721,7 @@ extension NonBlockingFileIOTest { // open 1 + write do { try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .allowFileCreation() ) { fileHandle in @@ -1739,7 +1739,7 @@ extension NonBlockingFileIOTest { // open 2 (with truncation) + write again + read do { try await self.fileIO!.withFileHandle( - path: "\(dir)/file", + _deprecatedPath: "\(dir)/file", mode: [.write, .read], flags: .posix(flags: O_TRUNC, mode: 0) ) { fileHandle in @@ -1811,7 +1811,7 @@ extension NonBlockingFileIOTest { let threadPool = NIOThreadPool(numberOfThreads: 1) let fileIO = NonBlockingFileIO(threadPool: threadPool) do { - try await fileIO.withFileRegion(path: path) { _ in } + try await fileIO.withFileRegion(_deprecatedPath: path) { _ in } XCTFail("testAsyncThrowsErrorOnUnstartedPool: openFile should throw an error") } catch { } @@ -1873,7 +1873,7 @@ extension NonBlockingFileIOTest { try await withTemporaryDirectory { path in let file = "\(path)/file" try await self.fileIO.withFileHandle( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation() ) { handle in @@ -1887,7 +1887,7 @@ extension NonBlockingFileIOTest { try await withTemporaryDirectory { path in let file = "\(path)/file" try await self.fileIO.withFileHandle( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation() ) { handle in @@ -1914,7 +1914,7 @@ extension NonBlockingFileIOTest { try await withTemporaryDirectory { path in let file = "\(path)/file" try await self.fileIO.withFileHandle( - path: file, + _deprecatedPath: file, mode: .write, flags: .allowFileCreation() ) { handle in diff --git a/Tests/NIOPosixTests/TestUtils.swift b/Tests/NIOPosixTests/TestUtils.swift index c2bb121903..c239a09c8a 100644 --- a/Tests/NIOPosixTests/TestUtils.swift +++ b/Tests/NIOPosixTests/TestUtils.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2017-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -45,8 +45,8 @@ func withPipe(_ body: (NIOCore.NIOFileHandle, NIOCore.NIOFileHandle) throws -> [ fds.withUnsafeMutableBufferPointer { ptr in XCTAssertEqual(0, pipe(ptr.baseAddress!)) } - let readFH = NIOFileHandle(descriptor: fds[0]) - let writeFH = NIOFileHandle(descriptor: fds[1]) + let readFH = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fds[0]) + let writeFH = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fds[1]) var toClose: [NIOFileHandle] = [readFH, writeFH] var error: Error? = nil do { @@ -70,8 +70,8 @@ func withPipe( fds.withUnsafeMutableBufferPointer { ptr in XCTAssertEqual(0, pipe(ptr.baseAddress!)) } - let readFH = NIOFileHandle(descriptor: fds[0]) - let writeFH = NIOFileHandle(descriptor: fds[1]) + let readFH = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fds[0]) + let writeFH = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fds[1]) var toClose: [NIOFileHandle] = [readFH, writeFH] var error: Error? = nil do { @@ -149,7 +149,7 @@ func withTemporaryUnixDomainSocketPathName( func withTemporaryFile(content: String? = nil, _ body: (NIOCore.NIOFileHandle, String) throws -> T) rethrows -> T { let (fd, path) = openTemporaryFile() - let fileHandle = NIOFileHandle(descriptor: fd) + let fileHandle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fd) defer { XCTAssertNoThrow(try fileHandle.close()) XCTAssertEqual(0, unlink(path)) @@ -181,7 +181,7 @@ func withTemporaryFile( _ body: @escaping @Sendable (NIOCore.NIOFileHandle, String) async throws -> T ) async rethrows -> T { let (fd, path) = openTemporaryFile() - let fileHandle = NIOFileHandle(descriptor: fd) + let fileHandle = NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor: fd) defer { XCTAssertNoThrow(try fileHandle.close()) XCTAssertEqual(0, unlink(path))