From 0ee1657e8a648b2997aef020e0d2b2adbf49dce8 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Mon, 25 Nov 2024 16:05:30 +0000 Subject: [PATCH 1/3] 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)) From 1f1e787f0bfa9f5e4f5b54176f9f879ce44b638c Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Mon, 25 Nov 2024 16:21:41 +0000 Subject: [PATCH 2/3] Add Cxx interop swift settings to CI (#2999) ### Motivation: At the moment the Cxx interoperability CI workflow doesn't actually test Cxx interoperability. ### Modifications: The script now looks for the precise way that the `Package.swift` currently formats the target entry and appends a `swiftSettings` entry to it. This approach is pretty brittle but has the advantage that it's the same on Linux and Darwin. If this formatting changes too often then we could give up on this and assume the availability of gnu-sed instead (and make sure it's present on our CI containers). ### Result: All Cxx interoperability checks will check what they are named for. --------- Co-authored-by: George Barnett --- scripts/check-cxx-interop-compatibility.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/check-cxx-interop-compatibility.sh b/scripts/check-cxx-interop-compatibility.sh index f393eb4c78..5ad33a609c 100755 --- a/scripts/check-cxx-interop-compatibility.sh +++ b/scripts/check-cxx-interop-compatibility.sh @@ -19,7 +19,7 @@ log() { printf -- "** %s\n" "$*" >&2; } error() { printf -- "** ERROR: %s\n" "$*" >&2; } fatal() { error "$@"; exit 1; } -log "Checking for Cxx interoperability comaptibility..." +log "Checking for Cxx interoperability compatibility..." source_dir=$(pwd) working_dir=$(mktemp -d) @@ -30,6 +30,12 @@ package_name=$(swift package dump-package | jq -r '.name') cd "$working_dir" swift package init + +{ + echo "let swiftSettings: [SwiftSetting] = [.interoperabilityMode(.Cxx)]" + echo "for target in package.targets { target.swiftSettings = swiftSettings }" +} >> Package.swift + echo "package.dependencies.append(.package(path: \"$source_dir\"))" >> Package.swift for product in $library_products; do @@ -39,4 +45,4 @@ done swift build -log "✅ Passed the Cxx interoperability tests." \ No newline at end of file +log "✅ Passed the Cxx interoperability tests." From 6ba8f4f04aeefa3db0bea01a339f85c7f7241476 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Mon, 25 Nov 2024 17:56:44 +0000 Subject: [PATCH 3/3] fix almost all Sendable warnings (#2994) ### Motivation: Opening the `swift-nio` repository made me warning blind because there were always so many trivially fixable warnings about things that were correct but cannot be understood by the compiler. ### Modifications: Fix all the sendable warnings that popped up, except for one test where `NIOLockedValueBox` isn't sendable because `Foundation.Thread` seemingly isn't `Sendable` which is odd. Guessing that'll be fixed on their end. ### Result: - Fewer warnings - Less warning-blindness - More checks --- Sources/NIOCore/ChannelPipeline.swift | 19 ++++++++++- .../Internal/BufferedStream.swift | 1 - .../NIOHTTP1/HTTPServerPipelineHandler.swift | 6 ++-- .../NIOHTTP1/HTTPServerUpgradeHandler.swift | 29 ++++++++++------- .../NIOHTTPClientUpgradeHandler.swift | 22 +++++++------ .../NIOTypedHTTPClientUpgradeHandler.swift | 19 +++++++---- .../NIOTypedHTTPServerUpgradeHandler.swift | 30 +++++++++++------ Sources/NIOHTTP1Server/main.swift | 6 ++-- Sources/NIOPosix/Bootstrap.swift | 27 +++++++++++----- ...pplicationProtocolNegotiationHandler.swift | 4 ++- ...pplicationProtocolNegotiationHandler.swift | 4 ++- Sources/NIOTestUtils/NIOHTTP1TestServer.swift | 4 ++- .../WebSocketProtocolErrorHandler.swift | 4 ++- .../EmbeddedChannelTest.swift | 10 +++--- .../HTTPClientUpgradeTests.swift | 8 +++-- Tests/NIOHTTP1Tests/HTTPDecoderTest.swift | 10 +++--- .../NIOHTTP1Tests/HTTPServerClientTest.swift | 9 ++++++ .../HTTPServerPipelineHandlerTest.swift | 5 ++- .../ChannelNotificationTest.swift | 30 ++++++++--------- Tests/NIOPosixTests/ChannelPipelineTest.swift | 14 ++++++++ Tests/NIOPosixTests/ChannelTests.swift | 18 ++++++++--- .../NIOPosixTests/EchoServerClientTest.swift | 27 +++++++++++----- Tests/NIOPosixTests/EventLoopTest.swift | 4 ++- .../NIOPosixTests/IdleStateHandlerTest.swift | 5 +-- Tests/NIOPosixTests/NIOThreadPoolTest.swift | 20 +++++++----- Tests/NIOPosixTests/SocketChannelTest.swift | 4 ++- Tests/NIOPosixTests/StreamChannelsTest.swift | 32 +++++++++++++------ 27 files changed, 253 insertions(+), 118 deletions(-) diff --git a/Sources/NIOCore/ChannelPipeline.swift b/Sources/NIOCore/ChannelPipeline.swift index 3db4a88abd..2aa8d6c624 100644 --- a/Sources/NIOCore/ChannelPipeline.swift +++ b/Sources/NIOCore/ChannelPipeline.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 @@ -2131,6 +2131,23 @@ extension ChannelHandlerContext { self.writeAndFlush(data, promise: promise) return promise.futureResult } + + /// Returns this `ChannelHandlerContext` as a `NIOLoopBound`, bound to `self.eventLoop`. + /// + /// This is a shorthand for `NIOLoopBound(self, eventLoop: self.eventLoop)`. + /// + /// Being able to capture `ChannelHandlerContext`s in `EventLoopFuture` callbacks is important in SwiftNIO programs. + /// Of course, this is not always safe because the `EventLoopFuture` callbacks may run on other threads. SwiftNIO + /// programmers therefore always had to manually arrange for those callbacks to run on the correct `EventLoop` + /// (i.e. `context.eventLoop`) which then made that construction safe. + /// + /// Newer Swift versions contain a static feature to automatically detect data races which of course can't detect + /// the only _dynamically_ ``EventLoop`` a ``EventLoopFuture`` callback is running on. ``NIOLoopBound`` can be used + /// to prove to the compiler that this is safe and in case it is not, ``NIOLoopBound`` will trap at runtime. This is + /// therefore dynamically enforce the correct behaviour. + public var loopBound: NIOLoopBound { + NIOLoopBound(self, eventLoop: self.eventLoop) + } } @available(*, unavailable) diff --git a/Sources/NIOFileSystem/Internal/BufferedStream.swift b/Sources/NIOFileSystem/Internal/BufferedStream.swift index d68ed6b61d..d450b00b58 100644 --- a/Sources/NIOFileSystem/Internal/BufferedStream.swift +++ b/Sources/NIOFileSystem/Internal/BufferedStream.swift @@ -243,7 +243,6 @@ extension BufferedStream { } /// A type that indicates the result of writing elements to the source. - @frozen internal enum WriteResult: Sendable { /// A token that is returned when the asynchronous stream's backpressure strategy indicated that production should /// be suspended. Use this token to enqueue a callback by calling the ``enqueueCallback(_:)`` method. diff --git a/Sources/NIOHTTP1/HTTPServerPipelineHandler.swift b/Sources/NIOHTTP1/HTTPServerPipelineHandler.swift index c6626dd0e7..1272adc1fe 100644 --- a/Sources/NIOHTTP1/HTTPServerPipelineHandler.swift +++ b/Sources/NIOHTTP1/HTTPServerPipelineHandler.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 @@ -439,9 +439,11 @@ public final class HTTPServerPipelineHandler: ChannelDuplexHandler, RemovableCha // we just received the .end that we're missing so we can fall through to closing the connection fallthrough case .quiescingLastRequestEndReceived: + let loopBoundContext = context.loopBound self.lifecycleState = .quiescingCompleted context.write(data).flatMap { - context.close() + let context = loopBoundContext.value + return context.close() }.cascade(to: promise) case .acceptingEvents, .quiescingWaitingForRequestEnd: context.write(data, promise: promise) diff --git a/Sources/NIOHTTP1/HTTPServerUpgradeHandler.swift b/Sources/NIOHTTP1/HTTPServerUpgradeHandler.swift index cadaff37c3..464baeec36 100644 --- a/Sources/NIOHTTP1/HTTPServerUpgradeHandler.swift +++ b/Sources/NIOHTTP1/HTTPServerUpgradeHandler.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 @@ -180,14 +180,16 @@ public final class HTTPServerUpgradeHandler: ChannelInboundHandler, RemovableCha // We'll attempt to upgrade. This may take a while, so while we're waiting more data can come in. self.upgradeState = .awaitingUpgrader + let eventLoop = context.eventLoop + let loopBoundContext = context.loopBound self.handleUpgrade(context: context, request: request, requestedProtocols: requestedProtocols) - .hop(to: context.eventLoop) // the user might return a future from another EventLoop. + .hop(to: eventLoop) // the user might return a future from another EventLoop. .whenSuccess { callback in - context.eventLoop.assertInEventLoop() + eventLoop.assertInEventLoop() if let callback = callback { self.gotUpgrader(upgrader: callback) } else { - self.notUpgrading(context: context, data: requestPart) + self.notUpgrading(context: loopBoundContext.value, data: requestPart) } } } @@ -253,6 +255,8 @@ public final class HTTPServerUpgradeHandler: ChannelInboundHandler, RemovableCha } let responseHeaders = self.buildUpgradeHeaders(protocol: proto) + let pipeline = context.pipeline + let loopBoundContext = context.loopBound return upgrader.buildUpgradeResponse( channel: context.channel, upgradeRequest: request, @@ -271,18 +275,20 @@ public final class HTTPServerUpgradeHandler: ChannelInboundHandler, RemovableCha // internal handler, then call the user code, and then finally when the user code is done we do // our final cleanup steps, namely we replay the received data we buffered in the meantime and // then remove ourselves from the pipeline. - self.removeExtraHandlers(context: context).flatMap { + self.removeExtraHandlers(pipeline: pipeline).flatMap { self.sendUpgradeResponse( - context: context, + context: loopBoundContext.value, upgradeRequest: request, responseHeaders: finalResponseHeaders ) }.flatMap { - context.pipeline.syncOperations.removeHandler(self.httpEncoder) + pipeline.syncOperations.removeHandler(self.httpEncoder) }.flatMap { () -> EventLoopFuture in + let context = loopBoundContext.value self.upgradeCompletionHandler(context) return upgrader.upgrade(context: context, upgradeRequest: request) }.whenComplete { result in + let context = loopBoundContext.value switch result { case .success: context.fireUserInboundEventTriggered( @@ -300,6 +306,7 @@ public final class HTTPServerUpgradeHandler: ChannelInboundHandler, RemovableCha } }.flatMapError { error in // No upgrade here. We want to fire the error down the pipeline, and then try another loop iteration. + let context = loopBoundContext.value context.fireErrorCaught(error) return self.handleUpgradeForProtocol( context: context, @@ -366,14 +373,14 @@ public final class HTTPServerUpgradeHandler: ChannelInboundHandler, RemovableCha } /// Removes any extra HTTP-related handlers from the channel pipeline. - private func removeExtraHandlers(context: ChannelHandlerContext) -> EventLoopFuture { + private func removeExtraHandlers(pipeline: ChannelPipeline) -> EventLoopFuture { guard self.extraHTTPHandlers.count > 0 else { - return context.eventLoop.makeSucceededFuture(()) + return pipeline.eventLoop.makeSucceededFuture(()) } return .andAllSucceed( - self.extraHTTPHandlers.map { context.pipeline.removeHandler($0) }, - on: context.eventLoop + self.extraHTTPHandlers.map { pipeline.removeHandler($0) }, + on: pipeline.eventLoop ) } } diff --git a/Sources/NIOHTTP1/NIOHTTPClientUpgradeHandler.swift b/Sources/NIOHTTP1/NIOHTTPClientUpgradeHandler.swift index dbc403f69f..66a7b1a17a 100644 --- a/Sources/NIOHTTP1/NIOHTTPClientUpgradeHandler.swift +++ b/Sources/NIOHTTP1/NIOHTTPClientUpgradeHandler.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2019-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2019-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -325,19 +325,22 @@ public final class NIOHTTPClientUpgradeHandler: ChannelDuplexHandler, RemovableC // Once that's done, we call the internal handler, then call the upgrader code, and then finally when the // upgrader code is done, we do our final cleanup steps, namely we replay the received data we // buffered in the meantime and then remove ourselves from the pipeline. - { + let pipeline = context.pipeline + let loopBoundContext = context.loopBound + return { self.upgradeState = .upgrading - self.removeHTTPHandlers(context: context) + self.removeHTTPHandlers(pipeline: pipeline) .map { // Let the other handlers be removed before continuing with upgrade. - self.upgradeCompletionHandler(context) + self.upgradeCompletionHandler(loopBoundContext.value) self.upgradeState = .upgradingAddingHandlers } .flatMap { - upgrader.upgrade(context: context, upgradeResponse: response) + upgrader.upgrade(context: loopBoundContext.value, upgradeResponse: response) } .map { + let context = loopBoundContext.value // We unbuffer any buffered data here. // If we received any, we fire readComplete. @@ -356,19 +359,20 @@ public final class NIOHTTPClientUpgradeHandler: ChannelDuplexHandler, RemovableC self.upgradeState = .upgradeComplete } .whenComplete { _ in + let context = loopBoundContext.value context.pipeline.syncOperations.removeHandler(context: context, promise: nil) } } } /// Removes any extra HTTP-related handlers from the channel pipeline. - private func removeHTTPHandlers(context: ChannelHandlerContext) -> EventLoopFuture { + private func removeHTTPHandlers(pipeline: ChannelPipeline) -> EventLoopFuture { guard self.httpHandlers.count > 0 else { - return context.eventLoop.makeSucceededFuture(()) + return pipeline.eventLoop.makeSucceededFuture(()) } - let removeFutures = self.httpHandlers.map { context.pipeline.removeHandler($0) } - return .andAllSucceed(removeFutures, on: context.eventLoop) + let removeFutures = self.httpHandlers.map { pipeline.removeHandler($0) } + return .andAllSucceed(removeFutures, on: pipeline.eventLoop) } private func gotUpgrader(upgrader: @escaping (() -> Void)) { diff --git a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgradeHandler.swift b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgradeHandler.swift index 30f168c4e4..29764820a3 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgradeHandler.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgradeHandler.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2013 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 @@ -191,6 +191,7 @@ public final class NIOTypedHTTPClientUpgradeHandler: Ch } private func channelRead(context: ChannelHandlerContext, responsePart: HTTPClientResponsePart) { + let loopBoundContext = context.loopBound switch self.stateMachine.channelReadResponsePart(responsePart) { case .fireErrorCaughtAndRemoveHandler(let error): self.upgradeResultPromise.fail(error) @@ -201,6 +202,7 @@ public final class NIOTypedHTTPClientUpgradeHandler: Ch self.notUpgradingCompletionHandler(context.channel) .hop(to: context.eventLoop) .whenComplete { result in + let context = loopBoundContext.value self.upgradingHandlerCompleted(context: context, result) } @@ -223,11 +225,14 @@ public final class NIOTypedHTTPClientUpgradeHandler: Ch ) { // Before we start the upgrade we have to remove the HTTPEncoder and HTTPDecoder handlers from the // pipeline, to prevent them parsing any more data. We'll buffer the incoming data until that completes. - self.removeHTTPHandlers(context: context) + let channel = context.channel + let loopBoundContext = context.loopBound + self.removeHTTPHandlers(pipeline: context.pipeline) .flatMap { - upgrader.upgrade(channel: context.channel, upgradeResponse: responseHead) + upgrader.upgrade(channel: channel, upgradeResponse: responseHead) }.hop(to: context.eventLoop) .whenComplete { result in + let context = loopBoundContext.value self.upgradingHandlerCompleted(context: context, result) } } @@ -275,13 +280,13 @@ public final class NIOTypedHTTPClientUpgradeHandler: Ch } /// Removes any extra HTTP-related handlers from the channel pipeline. - private func removeHTTPHandlers(context: ChannelHandlerContext) -> EventLoopFuture { + private func removeHTTPHandlers(pipeline: ChannelPipeline) -> EventLoopFuture { guard self.httpHandlers.count > 0 else { - return context.eventLoop.makeSucceededFuture(()) + return pipeline.eventLoop.makeSucceededFuture(()) } - let removeFutures = self.httpHandlers.map { context.pipeline.removeHandler($0) } - return .andAllSucceed(removeFutures, on: context.eventLoop) + let removeFutures = self.httpHandlers.map { pipeline.removeHandler($0) } + return .andAllSucceed(removeFutures, on: pipeline.eventLoop) } } #endif diff --git a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgradeHandler.swift b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgradeHandler.swift index b430cf1be4..77fb961b0d 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgradeHandler.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgradeHandler.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2023-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -174,6 +174,7 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch } private func channelRead(context: ChannelHandlerContext, requestPart: HTTPServerRequestPart) { + let loopBoundContext = context.loopBound switch self.stateMachine.channelReadRequestPart(requestPart) { case .failUpgradePromise(let error): self.upgradeResultPromise.fail(error) @@ -182,6 +183,7 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch self.notUpgradingCompletionHandler(context.channel) .hop(to: context.eventLoop) .whenComplete { result in + let context = loopBoundContext.value self.upgradingHandlerCompleted(context: context, result, requestHeadAndProtocol: nil) } @@ -194,6 +196,7 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch allHeaderNames: allHeaderNames, connectionHeader: connectionHeader ).whenComplete { result in + let context = loopBoundContext.value context.eventLoop.assertInEventLoop() self.findingUpgradeCompleted(context: context, requestHead: head, result) } @@ -297,6 +300,7 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch ) } + let loopBoundContext = context.loopBound let responseHeaders = self.buildUpgradeHeaders(protocol: proto) return upgrader.buildUpgradeResponse( channel: context.channel, @@ -307,6 +311,7 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch .map { (upgrader, $0, proto) } .flatMapError { error in // No upgrade here. We want to fire the error down the pipeline, and then try another loop iteration. + let context = loopBoundContext.value context.fireErrorCaught(error) return self.handleUpgradeForProtocol( context: context, @@ -339,9 +344,11 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch ) case .runNotUpgradingInitializer: + let loopBoundContext = context.loopBound self.notUpgradingCompletionHandler(context.channel) .hop(to: context.eventLoop) .whenComplete { result in + let context = loopBoundContext.value self.upgradingHandlerCompleted(context: context, result, requestHeadAndProtocol: nil) } @@ -376,14 +383,19 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch // internal handler, then call the user code, and then finally when the user code is done we do // our final cleanup steps, namely we replay the received data we buffered in the meantime and // then remove ourselves from the pipeline. - self.removeExtraHandlers(context: context).flatMap { - self.sendUpgradeResponse(context: context, responseHeaders: responseHeaders) + let channel = context.channel + let pipeline = context.pipeline + let loopBoundContext = context.loopBound + self.removeExtraHandlers(pipeline: pipeline).flatMap { + let context = loopBoundContext.value + return self.sendUpgradeResponse(context: context, responseHeaders: responseHeaders) }.flatMap { - context.pipeline.syncOperations.removeHandler(self.httpEncoder) + pipeline.syncOperations.removeHandler(self.httpEncoder) }.flatMap { () -> EventLoopFuture in - upgrader.upgrade(channel: context.channel, upgradeRequest: requestHead) + upgrader.upgrade(channel: channel, upgradeRequest: requestHead) }.hop(to: context.eventLoop) .whenComplete { result in + let context = loopBoundContext.value self.upgradingHandlerCompleted(context: context, result, requestHeadAndProtocol: (requestHead, proto)) } } @@ -404,14 +416,14 @@ public final class NIOTypedHTTPServerUpgradeHandler: Ch } /// Removes any extra HTTP-related handlers from the channel pipeline. - private func removeExtraHandlers(context: ChannelHandlerContext) -> EventLoopFuture { + private func removeExtraHandlers(pipeline: ChannelPipeline) -> EventLoopFuture { guard self.extraHTTPHandlers.count > 0 else { - return context.eventLoop.makeSucceededFuture(()) + return pipeline.eventLoop.makeSucceededFuture(()) } return .andAllSucceed( - self.extraHTTPHandlers.map { context.pipeline.removeHandler($0) }, - on: context.eventLoop + self.extraHTTPHandlers.map { pipeline.removeHandler($0) }, + on: pipeline.eventLoop ) } diff --git a/Sources/NIOHTTP1Server/main.swift b/Sources/NIOHTTP1Server/main.swift index 2b662bcc3d..d3b6ce133e 100644 --- a/Sources/NIOHTTP1Server/main.swift +++ b/Sources/NIOHTTP1Server/main.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 @@ -207,7 +207,7 @@ private final class HTTPHandler: ChannelInboundHandler { () case .end: self.state.requestComplete() - let loopBoundContext = NIOLoopBound(context, eventLoop: context.eventLoop) + let loopBoundContext = context.loopBound let loopBoundSelf = NIOLoopBound(self, eventLoop: context.eventLoop) context.eventLoop.scheduleTask(in: delay) { () -> Void in let `self` = loopBoundSelf.value @@ -501,7 +501,7 @@ private final class HTTPHandler: ChannelInboundHandler { promise: EventLoopPromise? ) { self.state.responseComplete() - let loopBoundContext = NIOLoopBound(context, eventLoop: context.eventLoop) + let loopBoundContext = context.loopBound let promise = self.keepAlive ? promise : (promise ?? context.eventLoop.makePromise()) if !self.keepAlive { diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index ff2773a2cd..fa117e8576 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -431,6 +431,7 @@ public final class ServerBootstrap { final class AcceptHandler: ChannelInboundHandler { public typealias InboundIn = SocketChannel + public typealias InboundOut = SocketChannel private let childChannelInit: ((Channel) -> EventLoopFuture)? private let childChannelOptions: ChannelOptions.Storage @@ -445,7 +446,9 @@ public final class ServerBootstrap { func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) { if event is ChannelShouldQuiesceEvent { + let loopBoundContext = context.loopBound context.channel.close().whenFailure { error in + let context = loopBoundContext.value context.fireErrorCaught(error) } } @@ -467,28 +470,33 @@ public final class ServerBootstrap { } @inline(__always) - func fireThroughPipeline(_ future: EventLoopFuture) { + func fireThroughPipeline(_ future: EventLoopFuture, context: ChannelHandlerContext) { ctxEventLoop.assertInEventLoop() + assert(ctxEventLoop === context.eventLoop) + let loopBoundContext = context.loopBound future.flatMap { (_) -> EventLoopFuture in + let context = loopBoundContext.value ctxEventLoop.assertInEventLoop() guard context.channel.isActive else { - return context.eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel) + return ctxEventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel) } - context.fireChannelRead(data) + context.fireChannelRead(Self.wrapInboundOut(accepted)) return context.eventLoop.makeSucceededFuture(()) }.whenFailure { error in + let context = loopBoundContext.value ctxEventLoop.assertInEventLoop() self.closeAndFire(context: context, accepted: accepted, err: error) } } if childEventLoop === ctxEventLoop { - fireThroughPipeline(setupChildChannel()) + fireThroughPipeline(setupChildChannel(), context: context) } else { fireThroughPipeline( childEventLoop.flatSubmit { setupChildChannel() - }.hop(to: ctxEventLoop) + }.hop(to: ctxEventLoop), + context: context ) } } @@ -498,7 +506,9 @@ public final class ServerBootstrap { if context.eventLoop.inEventLoop { context.fireErrorCaught(err) } else { + let loopBoundContext = context.loopBound context.eventLoop.execute { + let context = loopBoundContext.value context.fireErrorCaught(err) } } @@ -998,7 +1008,8 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { let connectPromise = channel.eventLoop.makePromise(of: Void.self) channel.connect(to: address, promise: connectPromise) let cancelTask = channel.eventLoop.scheduleTask(in: self.connectTimeout) { - connectPromise.fail(ChannelError.connectTimeout(self.connectTimeout)) + [connectTimeout = self.connectTimeout] in + connectPromise.fail(ChannelError.connectTimeout(connectTimeout)) channel.close(promise: nil) } @@ -1147,8 +1158,8 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { @inline(__always) func setupChannel() -> EventLoopFuture { eventLoop.assertInEventLoop() - return channelOptions.applyAllChannelOptions(to: channel).flatMap { - if let bindTarget = self.bindTarget { + return channelOptions.applyAllChannelOptions(to: channel).flatMap { [bindTarget = self.bindTarget] in + if let bindTarget = bindTarget { return channel.bind(to: bindTarget).flatMap { channelInitializer(channel) } diff --git a/Sources/NIOTLS/ApplicationProtocolNegotiationHandler.swift b/Sources/NIOTLS/ApplicationProtocolNegotiationHandler.swift index 3d6859dec9..71186337f2 100644 --- a/Sources/NIOTLS/ApplicationProtocolNegotiationHandler.swift +++ b/Sources/NIOTLS/ApplicationProtocolNegotiationHandler.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 @@ -119,10 +119,12 @@ public final class ApplicationProtocolNegotiationHandler: ChannelInboundHandler, private func invokeUserClosure(context: ChannelHandlerContext, result: ALPNResult) { let switchFuture = self.completionHandler(result, context.channel) + let loopBoundSelfAndContext = NIOLoopBound((self, context), eventLoop: context.eventLoop) switchFuture .hop(to: context.eventLoop) .whenComplete { result in + let (`self`, context) = loopBoundSelfAndContext.value self.userFutureCompleted(context: context, result: result) } } diff --git a/Sources/NIOTLS/NIOTypedApplicationProtocolNegotiationHandler.swift b/Sources/NIOTLS/NIOTypedApplicationProtocolNegotiationHandler.swift index 4749be12ec..75d95c5a45 100644 --- a/Sources/NIOTLS/NIOTypedApplicationProtocolNegotiationHandler.swift +++ b/Sources/NIOTLS/NIOTypedApplicationProtocolNegotiationHandler.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2023-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -125,10 +125,12 @@ public final class NIOTypedApplicationProtocolNegotiationHandler?) { + let loopBoundContext = context.loopBound switch Self.unwrapOutboundIn(data) { case .head(var head): head.headers.replaceOrAdd(name: "connection", value: "close") @@ -127,6 +128,7 @@ private final class WebServerHandler: ChannelDuplexHandler { context.write(data, promise: promise) case .end: context.write(data).map { + let context = loopBoundContext.value context.close(promise: nil) }.cascade(to: promise) } diff --git a/Sources/NIOWebSocket/WebSocketProtocolErrorHandler.swift b/Sources/NIOWebSocket/WebSocketProtocolErrorHandler.swift index 89714f4c90..e367184b62 100644 --- a/Sources/NIOWebSocket/WebSocketProtocolErrorHandler.swift +++ b/Sources/NIOWebSocket/WebSocketProtocolErrorHandler.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 @@ -25,6 +25,7 @@ public final class WebSocketProtocolErrorHandler: ChannelInboundHandler { public init() {} public func errorCaught(context: ChannelHandlerContext, error: Error) { + let loopBoundContext = context.loopBound if let error = error as? NIOWebSocketError { var data = context.channel.allocator.buffer(capacity: 2) data.write(webSocketErrorCode: WebSocketErrorCode(error)) @@ -34,6 +35,7 @@ public final class WebSocketProtocolErrorHandler: ChannelInboundHandler { data: data ) context.writeAndFlush(Self.wrapOutboundOut(frame)).whenComplete { (_: Result) in + let context = loopBoundContext.value context.close(promise: nil) } } diff --git a/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift b/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift index 56c09b67f8..3c77ca624a 100644 --- a/Tests/NIOEmbeddedTests/EmbeddedChannelTest.swift +++ b/Tests/NIOEmbeddedTests/EmbeddedChannelTest.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 @@ -478,7 +478,7 @@ class EmbeddedChannelTest: XCTestCase { func testFinishWithRecursivelyScheduledTasks() throws { let channel = EmbeddedChannel() - var tasks: [Scheduled] = [] + let tasks: NIOLoopBoundBox<[Scheduled]> = NIOLoopBoundBox([], eventLoop: channel.eventLoop) var invocations = 0 func recursivelyScheduleAndIncrement() { @@ -486,7 +486,7 @@ class EmbeddedChannelTest: XCTestCase { invocations += 1 recursivelyScheduleAndIncrement() } - tasks.append(task) + tasks.value.append(task) } recursivelyScheduleAndIncrement() @@ -497,11 +497,11 @@ class EmbeddedChannelTest: XCTestCase { XCTAssertEqual(invocations, 0) // Because the root task didn't run, it should be the onnly one scheduled. - XCTAssertEqual(tasks.count, 1) + XCTAssertEqual(tasks.value.count, 1) // Check the task was failed with cancelled error. let taskChecked = expectation(description: "task future fulfilled") - tasks.first?.futureResult.whenComplete { result in + tasks.value.first?.futureResult.whenComplete { result in switch result { case .success: XCTFail("Expected task to be cancelled, not run.") case .failure(let error): XCTAssertEqual(error as? EventLoopError, .cancelled) diff --git a/Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift b/Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift index 537fa0740a..43cfbb4c6b 100644 --- a/Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift +++ b/Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2019-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2019-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -200,8 +200,8 @@ private final class UpgradeDelayClientUpgrader: TypedAndUntypedHTTPClientProtoco fileprivate func upgrade(context: ChannelHandlerContext, upgradeResponse: HTTPResponseHead) -> EventLoopFuture { self.upgradePromise = context.eventLoop.makePromise() - return self.upgradePromise!.futureResult.flatMap { - context.pipeline.addHandler(self.upgradedHandler) + return self.upgradePromise!.futureResult.flatMap { [pipeline = context.pipeline] in + pipeline.addHandler(self.upgradedHandler) } } @@ -1104,10 +1104,12 @@ final class TypedHTTPClientUpgradeTestCase: HTTPClientUpgradeTestCase { handlerType: NIOTypedHTTPClientUpgradeHandler.self ) + let loopBoundContext = context.loopBound try channel.connect(to: SocketAddress(ipAddress: "127.0.0.1", port: 0)) .wait() upgradeResult.whenSuccess { result in if result { + let context = loopBoundContext.value upgradeCompletionHandler(context) } } diff --git a/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift b/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift index da74c7685d..ffb144e5c5 100644 --- a/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPDecoderTest.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 @@ -251,8 +251,8 @@ class HTTPDecoderTest: XCTestCase { let part = Self.unwrapInboundIn(data) switch part { case .end: - _ = context.pipeline.removeHandler(self).flatMap { _ in - context.pipeline.addHandler(self.collector) + _ = context.pipeline.removeHandler(self).flatMap { [pipeline = context.pipeline] _ in + pipeline.addHandler(self.collector) } default: // ignore @@ -324,8 +324,8 @@ class HTTPDecoderTest: XCTestCase { let part = Self.unwrapInboundIn(data) switch part { case .end: - _ = context.pipeline.removeHandler(self).flatMap { _ in - context.pipeline.addHandler(ByteCollector()) + _ = context.pipeline.removeHandler(self).flatMap { [pipeline = context.pipeline] _ in + pipeline.addHandler(ByteCollector()) } break default: diff --git a/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift b/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift index 28a42923a7..c42e7c4068 100644 --- a/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPServerClientTest.swift @@ -109,6 +109,7 @@ class HTTPServerClientTest: XCTestCase { } public func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let loopBoundContext = context.loopBound switch Self.unwrapInboundIn(data) { case .head(let req): switch req.uri { @@ -129,6 +130,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -154,6 +156,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -184,6 +187,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(trailers))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -208,6 +212,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -221,6 +226,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -233,6 +239,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -251,6 +258,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } @@ -271,6 +279,7 @@ class HTTPServerClientTest: XCTestCase { context.write(Self.wrapOutboundOut(.end(nil))).recover { error in XCTFail("unexpected error \(error)") }.whenComplete { (_: Result) in + let context = loopBoundContext.value self.sentEnd = true self.maybeClose(context: context) } diff --git a/Tests/NIOHTTP1Tests/HTTPServerPipelineHandlerTest.swift b/Tests/NIOHTTP1Tests/HTTPServerPipelineHandlerTest.swift index 15144e8f2b..f78cdff130 100644 --- a/Tests/NIOHTTP1Tests/HTTPServerPipelineHandlerTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPServerPipelineHandlerTest.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 @@ -912,11 +912,13 @@ class HTTPServerPipelineHandlerTest: XCTestCase { } func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let loopBoundContext = context.loopBound switch Self.unwrapInboundIn(data) { case .head: // We dispatch this to the event loop so that it doesn't happen immediately but rather can be // run from the driving test code whenever it wants by running the EmbeddedEventLoop. context.eventLoop.execute { + let context = loopBoundContext.value context.writeAndFlush( Self.wrapOutboundOut( .head( @@ -937,6 +939,7 @@ class HTTPServerPipelineHandlerTest: XCTestCase { // We dispatch this to the event loop so that it doesn't happen immediately but rather can be // run from the driving test code whenever it wants by running the EmbeddedEventLoop. context.eventLoop.execute { + let context = loopBoundContext.value context.writeAndFlush(Self.wrapOutboundOut(.end(nil)), promise: nil) } XCTAssertEqual(.reqEndExpected, self.state) diff --git a/Tests/NIOPosixTests/ChannelNotificationTest.swift b/Tests/NIOPosixTests/ChannelNotificationTest.swift index cf0b963d5d..fe6fdf4729 100644 --- a/Tests/NIOPosixTests/ChannelNotificationTest.swift +++ b/Tests/NIOPosixTests/ChannelNotificationTest.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 @@ -138,8 +138,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.connectPromise) XCTAssertNil(self.closePromise) - promise!.futureResult.whenSuccess { - XCTAssertFalse(context.channel.isActive) + promise!.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertFalse(channel.isActive) } self.registerPromise = promise @@ -157,8 +157,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.connectPromise) XCTAssertNil(self.closePromise) - promise!.futureResult.whenSuccess { - XCTAssertTrue(context.channel.isActive) + promise!.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertTrue(channel.isActive) } self.connectPromise = promise @@ -170,8 +170,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNotNil(self.connectPromise) XCTAssertNil(self.closePromise) - promise!.futureResult.whenSuccess { - XCTAssertFalse(context.channel.isActive) + promise!.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertFalse(channel.isActive) } self.closePromise = promise @@ -248,8 +248,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.registerPromise) let p = promise ?? context.eventLoop.makePromise() - p.futureResult.whenSuccess { - XCTAssertFalse(context.channel.isActive) + p.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertFalse(channel.isActive) } self.registerPromise = p @@ -354,8 +354,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.closePromise) let p = promise ?? context.eventLoop.makePromise() - p.futureResult.whenSuccess { - XCTAssertFalse(context.channel.isActive) + p.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertFalse(channel.isActive) } self.registerPromise = p @@ -373,8 +373,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.bindPromise) XCTAssertNil(self.closePromise) - promise?.futureResult.whenSuccess { - XCTAssertTrue(context.channel.isActive) + promise?.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertTrue(channel.isActive) } self.bindPromise = promise @@ -387,8 +387,8 @@ class ChannelNotificationTest: XCTestCase { XCTAssertNil(self.closePromise) let p = promise ?? context.eventLoop.makePromise() - p.futureResult.whenSuccess { - XCTAssertFalse(context.channel.isActive) + p.futureResult.whenSuccess { [channel = context.channel] in + XCTAssertFalse(channel.isActive) } self.closePromise = p diff --git a/Tests/NIOPosixTests/ChannelPipelineTest.swift b/Tests/NIOPosixTests/ChannelPipelineTest.swift index 9226d7a0a6..130e39fd2c 100644 --- a/Tests/NIOPosixTests/ChannelPipelineTest.swift +++ b/Tests/NIOPosixTests/ChannelPipelineTest.swift @@ -803,7 +803,9 @@ class ChannelPipelineTest: XCTestCase { buffer.writeStaticString("Hello, world!") let removalPromise = channel.eventLoop.makePromise(of: Void.self) + let loopBoundContext = context.loopBound removalPromise.futureResult.whenSuccess { + let context = loopBoundContext.value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) } @@ -845,7 +847,9 @@ class ChannelPipelineTest: XCTestCase { XCTAssertNoThrow(XCTAssertNil(try channel.readOutbound())) XCTAssertNoThrow(try channel.throwIfErrorCaught()) + let loopBoundContext = context.loopBound channel.pipeline.syncOperations.removeHandler(context: context).whenSuccess { + let context = loopBoundContext.value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) } @@ -873,7 +877,9 @@ class ChannelPipelineTest: XCTestCase { buffer.writeStaticString("Hello, world!") let removalPromise = channel.eventLoop.makePromise(of: Void.self) + let loopBoundContext = context.loopBound removalPromise.futureResult.map { + let context = loopBoundContext._value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) }.whenFailure { @@ -912,7 +918,9 @@ class ChannelPipelineTest: XCTestCase { XCTAssertNoThrow(XCTAssertNil(try channel.readOutbound())) XCTAssertNoThrow(try channel.throwIfErrorCaught()) + let loopBoundContext = context.loopBound channel.pipeline.removeHandler(name: "TestHandler").whenSuccess { + let context = loopBoundContext.value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) } @@ -941,7 +949,9 @@ class ChannelPipelineTest: XCTestCase { buffer.writeStaticString("Hello, world!") let removalPromise = channel.eventLoop.makePromise(of: Void.self) + let loopBoundContext = context.loopBound removalPromise.futureResult.whenSuccess { + let context = loopBoundContext.value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) } @@ -979,7 +989,9 @@ class ChannelPipelineTest: XCTestCase { XCTAssertNoThrow(XCTAssertNil(try channel.readOutbound())) XCTAssertNoThrow(try channel.throwIfErrorCaught()) + let loopBoundContext = context.loopBound channel.pipeline.removeHandler(handler).whenSuccess { + let context = loopBoundContext.value context.writeAndFlush(NIOAny(buffer), promise: nil) context.fireErrorCaught(DummyError()) } @@ -1403,7 +1415,9 @@ class ChannelPipelineTest: XCTestCase { self.removeHandlerCalls += 1 XCTAssertEqual(1, self.removeHandlerCalls) self.removalTriggeredPromise.succeed(()) + let loopBoundContext = context.loopBound self.continueRemovalFuture.whenSuccess { + let context = loopBoundContext.value context.leavePipeline(removalToken: removalToken) } } diff --git a/Tests/NIOPosixTests/ChannelTests.swift b/Tests/NIOPosixTests/ChannelTests.swift index 8774c4f6a5..362260ebe0 100644 --- a/Tests/NIOPosixTests/ChannelTests.swift +++ b/Tests/NIOPosixTests/ChannelTests.swift @@ -2953,7 +2953,9 @@ public final class ChannelTests: XCTestCase { func channelActive(context: ChannelHandlerContext) { var buffer = context.channel.allocator.buffer(capacity: 1) buffer.writeStaticString("X") - context.channel.writeAndFlush(buffer).map { context.channel }.cascade( + context.channel.writeAndFlush(buffer).map { [channel = context.channel] in + channel + }.cascade( to: self.channelAvailablePromise ) } @@ -3003,7 +3005,8 @@ public final class ChannelTests: XCTestCase { func channelActive(context: ChannelHandlerContext) { XCTAssert(serverChannel.eventLoop === context.eventLoop) - self.serverChannel.whenSuccess { serverChannel in + let loopBoundContext = context.loopBound + self.serverChannel.whenSuccess { [channel = context.channel] serverChannel in // all of the following futures need to complete synchronously for this test to test the correct // thing. Therefore we keep track if we're still on the same stack frame. var inSameStackFrame = true @@ -3014,9 +3017,10 @@ public final class ChannelTests: XCTestCase { XCTAssertTrue(serverChannel.isActive) // we allow auto-read again to make sure that the socket buffer is drained on write error // (cf. https://github.com/apple/swift-nio/issues/593) - context.channel.setOption(.autoRead, value: true).flatMap { + channel.setOption(.autoRead, value: true).flatMap { + let context = loopBoundContext.value // let's trigger the write error - var buffer = context.channel.allocator.buffer(capacity: 16) + var buffer = channel.allocator.buffer(capacity: 16) buffer.writeStaticString("THIS WILL FAIL ANYWAY") // this needs to be in a function as otherwise the Swift compiler believes this is throwing @@ -3025,7 +3029,7 @@ public final class ChannelTests: XCTestCase { // arrived at the time the write fails. So this is a hack that makes sure they do have arrived. // (https://github.com/apple/swift-nio/issues/657) XCTAssertNoThrow( - try self.veryNasty_blockUntilReadBufferIsNonEmpty(channel: context.channel) + try self.veryNasty_blockUntilReadBufferIsNonEmpty(channel: channel) ) } workaroundSR487() @@ -3489,8 +3493,10 @@ private final class FailRegistrationAndDelayCloseHandler: ChannelOutboundHandler } func close(context: ChannelHandlerContext, mode: CloseMode, promise: EventLoopPromise?) { + let loopBoundContext = context.loopBound // for extra nastiness, let's delay close. This makes sure the ChannelPipeline correctly retains the Channel _ = context.eventLoop.scheduleTask(in: .milliseconds(10)) { + let context = loopBoundContext.value context.close(mode: mode, promise: promise) } } @@ -3559,7 +3565,9 @@ final class ReentrantWritabilityChangingHandler: ChannelInboundHandler { // emitted. The flush for that write should result in the writability flipping back // again. let b1 = context.channel.allocator.buffer(repeating: 0, count: 50) + let loopBoundContext = context.loopBound context.write(Self.wrapOutboundOut(b1)).whenSuccess { _ in + let context = loopBoundContext.value // We should still be writable. XCTAssertTrue(context.channel.isWritable) XCTAssertEqual(self.isNotWritableCount, 0) diff --git a/Tests/NIOPosixTests/EchoServerClientTest.swift b/Tests/NIOPosixTests/EchoServerClientTest.swift index 2be0133d9d..678f6da805 100644 --- a/Tests/NIOPosixTests/EchoServerClientTest.swift +++ b/Tests/NIOPosixTests/EchoServerClientTest.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 @@ -382,6 +382,7 @@ class EchoServerClientTest: XCTestCase { private final class EchoAndEchoAgainAfterSomeTimeServer: ChannelInboundHandler { typealias InboundIn = ByteBuffer + typealias InboundOut = ByteBuffer typealias OutboundOut = ByteBuffer private let timeAmount: TimeAmount @@ -398,10 +399,13 @@ class EchoServerClientTest: XCTestCase { } func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let bytes = Self.unwrapInboundIn(data) self.numberOfReads += 1 precondition(self.numberOfReads == 1, "\(self) is only ever allowed to read once") + let loopBoundContext = context.loopBound _ = context.eventLoop.scheduleTask(in: self.timeAmount) { - context.writeAndFlush(data, promise: nil) + let context = loopBoundContext.value + context.writeAndFlush(Self.wrapInboundOut(bytes), promise: nil) self.group.leave() }.futureResult.recover { e in XCTFail("we failed to schedule the task: \(e)") @@ -753,8 +757,10 @@ class EchoServerClientTest: XCTestCase { } private func writeUntilFailed(_ context: ChannelHandlerContext, _ buffer: ByteBuffer) { - context.writeAndFlush(NIOAny(buffer)).whenSuccess { - context.eventLoop.execute { + let loopBoundContext = context.loopBound + context.writeAndFlush(NIOAny(buffer)).whenSuccess { [eventLoop = context.eventLoop] in + eventLoop.execute { + let context = loopBoundContext.value self.writeUntilFailed(context, buffer) } } @@ -776,14 +782,19 @@ class EchoServerClientTest: XCTestCase { let buffer = context.channel.allocator.buffer(string: str) // write it four times and then close the connect. + let loopBoundContext = context.loopBound context.writeAndFlush(NIOAny(buffer)).flatMap { - context.writeAndFlush(NIOAny(buffer)) + let context = loopBoundContext.value + return context.writeAndFlush(NIOAny(buffer)) }.flatMap { - context.writeAndFlush(NIOAny(buffer)) + let context = loopBoundContext.value + return context.writeAndFlush(NIOAny(buffer)) }.flatMap { - context.writeAndFlush(NIOAny(buffer)) + let context = loopBoundContext.value + return context.writeAndFlush(NIOAny(buffer)) }.flatMap { - context.close() + let context = loopBoundContext.value + return context.close() }.whenComplete { (_: Result) in self.dpGroup.leave() } diff --git a/Tests/NIOPosixTests/EventLoopTest.swift b/Tests/NIOPosixTests/EventLoopTest.swift index 4b0abd9530..f14ba2dc4f 100644 --- a/Tests/NIOPosixTests/EventLoopTest.swift +++ b/Tests/NIOPosixTests/EventLoopTest.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 @@ -542,7 +542,9 @@ public final class EventLoopTest: XCTestCase { } XCTAssertTrue(context.channel.isActive) self.closePromise = context.eventLoop.makePromise() + let loopBoundContext = context.loopBound self.closePromise!.futureResult.whenSuccess { + let context = loopBoundContext.value context.close(mode: mode, promise: promise) } promiseRegisterCallback(self.closePromise!) diff --git a/Tests/NIOPosixTests/IdleStateHandlerTest.swift b/Tests/NIOPosixTests/IdleStateHandlerTest.swift index 87ae3d8cd7..04b81173cf 100644 --- a/Tests/NIOPosixTests/IdleStateHandlerTest.swift +++ b/Tests/NIOPosixTests/IdleStateHandlerTest.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 @@ -37,7 +37,7 @@ class IdleStateHandlerTest: XCTestCase { } private func testIdle( - _ handler: IdleStateHandler, + _ handler: @escaping @Sendable @autoclosure () -> IdleStateHandler, _ writeToChannel: Bool, _ assertEventFn: @escaping (IdleStateHandler.IdleStateEvent) -> Bool ) throws { @@ -86,6 +86,7 @@ class IdleStateHandlerTest: XCTestCase { .serverChannelOption(.socketOption(.so_reuseaddr), value: 1) .childChannelInitializer { channel in channel.eventLoop.makeCompletedFuture { + let handler = handler() try channel.pipeline.syncOperations.addHandler(handler) try channel.pipeline.syncOperations.addHandler(TestWriteHandler(writeToChannel, assertEventFn)) } diff --git a/Tests/NIOPosixTests/NIOThreadPoolTest.swift b/Tests/NIOPosixTests/NIOThreadPoolTest.swift index a1969f6204..ed39352df0 100644 --- a/Tests/NIOPosixTests/NIOThreadPoolTest.swift +++ b/Tests/NIOPosixTests/NIOThreadPoolTest.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 @@ -79,15 +79,17 @@ class NIOThreadPoolTest: XCTestCase { // The lock here is arguably redundant with the dispatchgroup, but let's make // this test thread-safe even if I screw up. let lock = NIOLock() - var threadOne = Thread?.none - var threadTwo = Thread?.none + let threadOne: NIOLockedValueBox = NIOLockedValueBox(Thread?.none) + let threadTwo: NIOLockedValueBox = NIOLockedValueBox(Thread?.none) completionGroup.enter() pool.submit { s in precondition(s == .active) lock.withLock { () -> Void in - XCTAssertEqual(threadOne, nil) - threadOne = Thread.current + threadOne.withLockedValue { threadOne in + XCTAssertEqual(threadOne, nil) + threadOne = Thread.current + } } completionGroup.leave() } @@ -98,8 +100,10 @@ class NIOThreadPoolTest: XCTestCase { pool.submit { s in precondition(s == .active) lock.withLock { () -> Void in - XCTAssertEqual(threadTwo, nil) - threadTwo = Thread.current + threadTwo.withLockedValue { threadTwo in + XCTAssertEqual(threadTwo, nil) + threadTwo = Thread.current + } } completionGroup.leave() } @@ -109,7 +113,7 @@ class NIOThreadPoolTest: XCTestCase { lock.withLock { () -> Void in XCTAssertNotNil(threadOne) XCTAssertNotNil(threadTwo) - XCTAssertEqual(threadOne, threadTwo) + XCTAssertEqual(threadOne.withLockedValue { $0 }, threadTwo.withLockedValue { $0 }) } } diff --git a/Tests/NIOPosixTests/SocketChannelTest.swift b/Tests/NIOPosixTests/SocketChannelTest.swift index ebd4787600..cb8161a7e5 100644 --- a/Tests/NIOPosixTests/SocketChannelTest.swift +++ b/Tests/NIOPosixTests/SocketChannelTest.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 @@ -550,7 +550,9 @@ public final class SocketChannelTest: XCTestCase { XCTAssertEqual(.inactive, state) state = .removed + let loopBoundContext = context.loopBound context.channel.closeFuture.whenComplete { (_: Result) in + let context = loopBoundContext.value XCTAssertNil(context.localAddress) XCTAssertNil(context.remoteAddress) diff --git a/Tests/NIOPosixTests/StreamChannelsTest.swift b/Tests/NIOPosixTests/StreamChannelsTest.swift index 277b88594c..072f35d41b 100644 --- a/Tests/NIOPosixTests/StreamChannelsTest.swift +++ b/Tests/NIOPosixTests/StreamChannelsTest.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2019-2021 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2019-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -486,9 +486,11 @@ class StreamChannelTest: XCTestCase { // raise the high water mark so we don't get another call straight away. var buffer = context.channel.allocator.buffer(capacity: 5) buffer.writeString("hello") + let loopBoundContext = context.loopBound context.channel.setOption(.writeBufferWaterMark, value: .init(low: 1024, high: 1024)) .flatMap { - context.writeAndFlush(Self.wrapOutboundOut(buffer)) + let context = loopBoundContext.value + return context.writeAndFlush(Self.wrapOutboundOut(buffer)) }.whenFailure { error in XCTFail("unexpected error: \(error)") } @@ -638,7 +640,9 @@ class StreamChannelTest: XCTestCase { XCTFail("unexpected error \(error)") } + let loopBoundContext = context.loopBound context.eventLoop.execute { + let context = loopBoundContext.value self.kickOff(context: context) } } @@ -648,26 +652,34 @@ class StreamChannelTest: XCTestCase { } func kickOff(context: ChannelHandlerContext) { - var buffer = context.channel.allocator.buffer(capacity: self.chunkSize) - buffer.writeBytes(Array(repeating: UInt8(ascii: "0"), count: chunkSize)) + let buffer = NIOLoopBoundBox( + context.channel.allocator.buffer(capacity: self.chunkSize), + eventLoop: context.eventLoop + ) + buffer.value.writeBytes(Array(repeating: UInt8(ascii: "0"), count: chunkSize)) + let loopBoundContext = context.loopBound + let loopBoundSelf = NIOLoopBound(self, eventLoop: context.eventLoop) func writeOneMore() { - self.bytesWritten += buffer.readableBytes - context.writeAndFlush(Self.wrapOutboundOut(buffer)).whenFailure { error in + let context = loopBoundContext.value + self.bytesWritten += buffer.value.readableBytes + context.writeAndFlush(Self.wrapOutboundOut(buffer.value)).whenFailure { error in XCTFail("unexpected error \(error)") } context.eventLoop.scheduleTask(in: .microseconds(100)) { + let context = loopBoundContext.value switch self.state { case .writingUntilFull: // We're just enqueuing another chunk. writeOneMore() case .writeSentinel: + let `self` = loopBoundSelf.value // We've seen the notification that the channel is unwritable, let's write one more byte. - buffer.clear() - buffer.writeString("1") + buffer.value.clear() + buffer.value.writeString("1") self.state = .done self.bytesWritten += 1 - context.writeAndFlush(Self.wrapOutboundOut(buffer)).whenFailure { error in + context.writeAndFlush(Self.wrapOutboundOut(buffer.value)).whenFailure { error in XCTFail("unexpected error \(error)") } self.wroteEnoughToBeStuckPromise.succeed(self.bytesWritten) @@ -692,7 +704,9 @@ class StreamChannelTest: XCTestCase { () // ignored, we're done } context.fireChannelWritabilityChanged() + let loopBoundContext = context.loopBound self.wroteEnoughToBeStuckPromise.futureResult.whenSuccess { _ in + let context = loopBoundContext.value context.pipeline.removeHandler(self).whenFailure { error in XCTFail("unexpected error \(error)") }