From 2f99bc3fa5b83c1a01b22da7217a3c9677358d04 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Tue, 2 Jul 2024 13:26:56 +0100 Subject: [PATCH 01/17] [GHA] Introduce the first GitHub Action (#2760) # Motivation We want to move our CI over to use GitHub Actions. We need to cover multiple different things like unit tests, benchmarks, soundness, API breaks, doc validation etc. # Modification This PR starts our journey to GitHub Actions by introducing the PR workflow. Right now this workflow only runs the Swift tests on our repository without any additional compiler flags like `-warnings-as-errors`. This aims to get us started and we will continue to modify them in subsequent PRs. # Result We got our first GH workflows running --- .github/workflows/pull_requests.yml | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/pull_requests.yml diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml new file mode 100644 index 0000000000..3797817522 --- /dev/null +++ b/.github/workflows/pull_requests.yml @@ -0,0 +1,31 @@ +name: Pull Request + +on: + pull_request: + types: [opened, reopened, synchronize, ready_for_review] + +## We are cancelling previously triggered workflow runs +concurrency: + group: ${{ github.workflow }}-${{ github.event.ref }} + cancel-in-progress: true + +jobs: + unit-tests: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + swift: + - image: swift5.8-jammy + - image: swift5.9-jammy + - image: swift5.10-noble + - image: swiftlang/swift:nightly-6.0-jammy + - image: swiftlang/swift:nightly-main-jammy + container: + image: ${{ matrix.swift.image }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run tests + run: swift test + timeout-minutes: 20 \ No newline at end of file From 304a238b09a3daae081e53470e8df9a4dc84e0ba Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Tue, 2 Jul 2024 15:58:07 +0100 Subject: [PATCH 02/17] [GHA] Fix docker images (#2762) --- .github/workflows/pull_requests.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml index 3797817522..89fda3a67b 100644 --- a/.github/workflows/pull_requests.yml +++ b/.github/workflows/pull_requests.yml @@ -16,9 +16,9 @@ jobs: fail-fast: false matrix: swift: - - image: swift5.8-jammy - - image: swift5.9-jammy - - image: swift5.10-noble + - image: swift:5.8-jammy + - image: swift:5.9-jammy + - image: swift:5.10-noble - image: swiftlang/swift:nightly-6.0-jammy - image: swiftlang/swift:nightly-main-jammy container: From 9d5d0b6007856a94f918ec515db81de9a63e5d99 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Thu, 4 Jul 2024 11:27:38 +0100 Subject: [PATCH 03/17] [GHA] Add API breaking check (#2763) # Motivation The next GH action pipeline that we need is to add our API breaking change checker. # Modification This PR adds an API breaking change job to our pull request workflow. --- .github/workflows/pull_requests.yml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml index 89fda3a67b..0996d479d0 100644 --- a/.github/workflows/pull_requests.yml +++ b/.github/workflows/pull_requests.yml @@ -6,11 +6,12 @@ on: ## We are cancelling previously triggered workflow runs concurrency: - group: ${{ github.workflow }}-${{ github.event.ref }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true jobs: unit-tests: + name: Unit tests runs-on: ubuntu-latest strategy: fail-fast: false @@ -23,9 +24,27 @@ jobs: - image: swiftlang/swift:nightly-main-jammy container: image: ${{ matrix.swift.image }} + timeout-minutes: 20 steps: - name: Checkout repository uses: actions/checkout@v4 - name: Run tests run: swift test - timeout-minutes: 20 \ No newline at end of file + + api-breakage: + name: API breakage checks + runs-on: ubuntu-latest + container: + image: swift:5.10-noble + timeout-minutes: 20 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + # We need to fetch everything otherwise only the head commit will be fetched. + fetch-depth: 0 + - name: Mark the workspace as safe + # https://github.com/actions/checkout/issues/766 + run: git config --global --add safe.directory ${GITHUB_WORKSPACE} + - name: Run API breakage check + run: swift package diagnose-api-breaking-changes origin/main \ No newline at end of file From 973093879358cc0551ce3448d32b1d626809e8b7 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Thu, 4 Jul 2024 16:21:18 +0100 Subject: [PATCH 04/17] [GHA] Documentation check (#2764) # Motivation Next up replacing the documentation check. # Modification This moves the current approach of the script into a GH action. I tried just running the documentation check against all targets but that also results in warnings from downstream deps in their docs to show up. I also fixed up some new errors that appeared since our current pipeline was running on 5.8. # Result Next CI job migrated to GHA --- .github/workflows/pull_requests.yml | 25 +++++++++++++++++-- Sources/NIOCore/AsyncAwaitSupport.swift | 4 +-- .../NIOCore/AsyncChannel/AsyncChannel.swift | 2 +- Sources/NIOCore/EventLoop.swift | 2 +- Sources/NIOFileSystem/FileSystem.swift | 2 +- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml index 0996d479d0..a455b2dabd 100644 --- a/.github/workflows/pull_requests.yml +++ b/.github/workflows/pull_requests.yml @@ -32,7 +32,7 @@ jobs: run: swift test api-breakage: - name: API breakage checks + name: API breakage check runs-on: ubuntu-latest container: image: swift:5.10-noble @@ -47,4 +47,25 @@ jobs: # https://github.com/actions/checkout/issues/766 run: git config --global --add safe.directory ${GITHUB_WORKSPACE} - name: Run API breakage check - run: swift package diagnose-api-breaking-changes origin/main \ No newline at end of file + run: swift package diagnose-api-breaking-changes origin/main + + docs-check: + name: Documentation check + runs-on: ubuntu-latest + container: + image: swift:5.10-noble + timeout-minutes: 20 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run documentation check + shell: bash + run: | + set -eu + + raw_targets=$(sed -E -n -e 's/^.* - documentation_targets: \[(.*)\].*$/\1/p' .spi.yml) + targets=(${raw_targets//,/ }) + + for target in "${targets[@]}"; do + swift package plugin generate-documentation --target "$target" --warnings-as-errors --analyze --level detailed + done \ No newline at end of file diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index 659df1ad70..f235d8cfd7 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -225,7 +225,7 @@ public struct NIOTooManyBytesError: Error, Hashable { @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) extension AsyncSequence where Element: RandomAccessCollection, Element.Element == UInt8 { - /// Accumulates an ``Swift/AsyncSequence`` of ``Swift/RandomAccessCollection``s into a single `accumulationBuffer`. + /// Accumulates an `AsyncSequence` of `RandomAccessCollection`s into a single `accumulationBuffer`. /// - Parameters: /// - accumulationBuffer: buffer to write all the elements of `self` into /// - maxBytes: The maximum number of bytes this method is allowed to write into `accumulationBuffer` @@ -247,7 +247,7 @@ extension AsyncSequence where Element: RandomAccessCollection, Element.Element = } } - /// Accumulates an ``Swift/AsyncSequence`` of ``Swift/RandomAccessCollection``s into a single ``ByteBuffer``. + /// Accumulates an `AsyncSequence` of `RandomAccessCollection`s into a single ``ByteBuffer``. /// - Parameters: /// - maxBytes: The maximum number of bytes this method is allowed to accumulate /// - allocator: Allocator used for allocating the result `ByteBuffer` diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannel.swift b/Sources/NIOCore/AsyncChannel/AsyncChannel.swift index 3bbaaf4ba9..27ef318293 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannel.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannel.swift @@ -26,7 +26,7 @@ /// does not expose the following functionality: /// /// - user events -/// - traditional NIO back pressure such as writability signals and the ``Channel/read()`` call +/// - traditional NIO back pressure such as writability signals and the channel's read call /// /// Users are encouraged to separate their ``ChannelHandler``s into those that implement /// protocol-specific logic (such as parsers and encoders) and those that implement business diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 50b90ed925..0e0742c73e 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -314,7 +314,7 @@ public protocol EventLoop: EventLoopGroup { /// This executor can be used to isolate an actor to a given ``EventLoop``. Implementers are encouraged to customise /// this implementation by conforming their ``EventLoop`` to ``NIOSerialEventLoopExecutor`` which will provide an /// optimised implementation of this method, and will conform their type to `SerialExecutor`. The default - /// implementation returns a ``NIODefaultSerialEventLoopExecutor`` instead, which provides suboptimal performance. + /// implementation provides suboptimal performance. @available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *) var executor: any SerialExecutor { get } diff --git a/Sources/NIOFileSystem/FileSystem.swift b/Sources/NIOFileSystem/FileSystem.swift index 20c431a8a6..7a55fecd94 100644 --- a/Sources/NIOFileSystem/FileSystem.swift +++ b/Sources/NIOFileSystem/FileSystem.swift @@ -656,7 +656,7 @@ public struct FileSystem: Sendable, FileSystemProtocol { extension NIOSingletons { /// A suggestion of how many threads the global singleton ``FileSystem`` uses for blocking I/O. /// - /// The thread count is ``System/coreCount`` unless the environment variable + /// The thread count is the system's available core count unless the environment variable /// `NIO_SINGLETON_FILESYSTEM_THREAD_COUNT` is set or this value was set manually by the user. /// /// - note: This value must be set _before_ any singletons are used and must only be set once. From 96149964cba9982b40729589376050307972f1c8 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Fri, 5 Jul 2024 09:47:12 +0100 Subject: [PATCH 05/17] [GHA] Create reusable workflow (#2767) # Motivation In the future, we want to share the workflow definition across multiple repos to avoid duplication. # Modification This PR extracts the current workflow definition into a reusable workflow and calls the reusable workflow from a new workflow that is triggered on PR events. The new reusable workflow starts off with a few simple inputs to enable/disable the three current jobs. By default all jobs are enabled. # Result First step into reusability of our workflow. --- .github/workflows/pull_request.yml | 10 +++++++++ ...requests.yml => reusable_pull_request.yml} | 21 ++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/pull_request.yml rename .github/workflows/{pull_requests.yml => reusable_pull_request.yml} (74%) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml new file mode 100644 index 0000000000..5ebeeca100 --- /dev/null +++ b/.github/workflows/pull_request.yml @@ -0,0 +1,10 @@ +name: Pull Request + +on: + pull_request: + types: [opened, reopened, synchronize, ready_for_review] + +jobs: + call-reusable-pull-request-workflow: + name: Checks + uses: ./.github/workflows/reusable_pull_request.yml \ No newline at end of file diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/reusable_pull_request.yml similarity index 74% rename from .github/workflows/pull_requests.yml rename to .github/workflows/reusable_pull_request.yml index a455b2dabd..4a4b05bc5e 100644 --- a/.github/workflows/pull_requests.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -1,8 +1,20 @@ name: Pull Request on: - pull_request: - types: [opened, reopened, synchronize, ready_for_review] + workflow_call: + inputs: + enable_unit_tests: + type: boolean + description: "Boolean to enable the unit tests job. Defaults to true." + default: true + enable_api_breakage_check: + type: boolean + description: "Boolean to enable the API breakage check job. Defaults to true." + default: true + enable_docs_check: + type: boolean + description: "Boolean to enable the docs check job. Defaults to true." + default: true ## We are cancelling previously triggered workflow runs concurrency: @@ -12,6 +24,7 @@ concurrency: jobs: unit-tests: name: Unit tests + if: ${{ inputs.enable_unit_tests }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -31,8 +44,9 @@ jobs: - name: Run tests run: swift test - api-breakage: + api-breakage-check: name: API breakage check + if: ${{ inputs.enable_api_breakage_check }} runs-on: ubuntu-latest container: image: swift:5.10-noble @@ -51,6 +65,7 @@ jobs: docs-check: name: Documentation check + if: ${{ inputs.enable_docs_check }} runs-on: ubuntu-latest container: image: swift:5.10-noble From 9b865595063e639468a5d97bf4e29d5290b50138 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Fri, 5 Jul 2024 16:01:29 +0100 Subject: [PATCH 06/17] Pre-box some errors to reduce allocations (#2765) Motivation: Existential errors (`any Error`) are unconditionally boxed. In NIO we often create errors to fail promises. One place we do this is in the head channel handler when the pipeline is closed. This error is created regardless of whether there are actually any promises to fail. The result is an unnecessary allocation when every channel is closed. This is particuarly egregious when multiplexed protocols such as HTTP/2 are used. As the majority of errors in NIO carry no information beyond their type/name we can declare these statically and pay the cost of each error just once rather than once per use. Modifications: - Add internal static constants for `ChannelError` and `EventLoopError` and use them throughout NIOCore and NIOPosix where they would otherwise be boxed. Result: Fewer allocations --- .../AsyncChannelOutboundWriterHandler.swift | 4 +- Sources/NIOCore/Channel.swift | 13 +++- Sources/NIOCore/ChannelPipeline.swift | 44 ++++++------- Sources/NIOCore/DeadChannel.swift | 22 +++---- Sources/NIOCore/EventLoop.swift | 9 ++- Sources/NIOPosix/BSDSocketAPIPosix.swift | 8 +-- Sources/NIOPosix/BaseSocketChannel.swift | 32 ++++----- .../NIOPosix/BaseStreamSocketChannel.swift | 14 ++-- Sources/NIOPosix/Bootstrap.swift | 10 +-- Sources/NIOPosix/Errors+Any.swift | 38 +++++++++++ .../MultiThreadedEventLoopGroup.swift | 2 +- .../PendingDatagramWritesManager.swift | 4 +- Sources/NIOPosix/PipeChannel.swift | 6 +- Sources/NIOPosix/PipePair.swift | 28 ++++---- Sources/NIOPosix/SelectableEventLoop.swift | 8 +-- Sources/NIOPosix/SelectorGeneric.swift | 2 +- Sources/NIOPosix/SelectorKqueue.swift | 2 +- Sources/NIOPosix/SocketChannel.swift | 66 +++++++++---------- docker/docker-compose.2204.510.yaml | 18 ++--- docker/docker-compose.2204.58.yaml | 18 ++--- docker/docker-compose.2204.59.yaml | 18 ++--- 21 files changed, 210 insertions(+), 156 deletions(-) create mode 100644 Sources/NIOPosix/Errors+Any.swift diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift index 8fa2111274..0e73fde347 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift @@ -137,7 +137,7 @@ internal final class NIOAsyncChannelOutboundWriterHandler @inlinable func handlerRemoved(context: ChannelHandlerContext) { self.context = nil - self.sink?.finish(error: ChannelError.ioOnClosedChannel) + self.sink?.finish(error: ChannelError._ioOnClosedChannel) self.writer = nil } @@ -150,7 +150,7 @@ internal final class NIOAsyncChannelOutboundWriterHandler @inlinable func channelInactive(context: ChannelHandlerContext) { - self.sink?.finish(error: ChannelError.ioOnClosedChannel) + self.sink?.finish(error: ChannelError._ioOnClosedChannel) context.fireChannelInactive() } diff --git a/Sources/NIOCore/Channel.swift b/Sources/NIOCore/Channel.swift index d61e3b7dcf..b2523e955f 100644 --- a/Sources/NIOCore/Channel.swift +++ b/Sources/NIOCore/Channel.swift @@ -202,7 +202,7 @@ extension Channel { } public func registerAlreadyConfigured0(promise: EventLoopPromise?) { - promise?.fail(ChannelError.operationUnsupported) + promise?.fail(ChannelError._operationUnsupported) } public func triggerUserOutboundEvent(_ event: Any, promise: EventLoopPromise?) { @@ -373,6 +373,17 @@ public enum ChannelError: Error { case unremovableHandler } +extension ChannelError { + // 'any Error' is unconditionally boxed, avoid allocating per use by statically boxing them. + static let _alreadyClosed: any Error = ChannelError.alreadyClosed + static let _inputClosed: any Error = ChannelError.inputClosed + @usableFromInline + static let _ioOnClosedChannel: any Error = ChannelError.ioOnClosedChannel + static let _operationUnsupported: any Error = ChannelError.operationUnsupported + static let _outputClosed: any Error = ChannelError.outputClosed + static let _unremovableHandler: any Error = ChannelError.unremovableHandler +} + extension ChannelError: Equatable { } /// The removal of a `ChannelHandler` using `ChannelPipeline.removeHandler` has been attempted more than once. diff --git a/Sources/NIOCore/ChannelPipeline.swift b/Sources/NIOCore/ChannelPipeline.swift index b2b505e4bf..e5ad620a05 100644 --- a/Sources/NIOCore/ChannelPipeline.swift +++ b/Sources/NIOCore/ChannelPipeline.swift @@ -198,7 +198,7 @@ public final class ChannelPipeline: ChannelInvoker { self.eventLoop.assertInEventLoop() if self.destroyed { - return .failure(ChannelError.ioOnClosedChannel) + return .failure(ChannelError._ioOnClosedChannel) } switch position { @@ -247,7 +247,7 @@ public final class ChannelPipeline: ChannelInvoker { operation: (ChannelHandlerContext, ChannelHandlerContext) -> Void) -> Result { self.eventLoop.assertInEventLoop() if self.destroyed { - return .failure(ChannelError.ioOnClosedChannel) + return .failure(ChannelError._ioOnClosedChannel) } guard let context = self.contextForPredicate0({ $0.handler === relativeHandler }) else { @@ -280,7 +280,7 @@ public final class ChannelPipeline: ChannelInvoker { self.eventLoop.assertInEventLoop() if self.destroyed { - return .failure(ChannelError.ioOnClosedChannel) + return .failure(ChannelError._ioOnClosedChannel) } let context = ChannelHandlerContext(name: name ?? nextName(), handler: handler, pipeline: self) @@ -416,7 +416,7 @@ public final class ChannelPipeline: ChannelInvoker { /// - promise: An `EventLoopPromise` that will complete when the `ChannelHandler` is removed. public func removeHandler(context: ChannelHandlerContext, promise: EventLoopPromise?) { guard context.handler is RemovableChannelHandler else { - promise?.fail(ChannelError.unremovableHandler) + promise?.fail(ChannelError._unremovableHandler) return } func removeHandler0() { @@ -826,7 +826,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeClose(mode: mode, promise: promise) } else { - promise?.fail(ChannelError.alreadyClosed) + promise?.fail(ChannelError._alreadyClosed) } } @@ -846,7 +846,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeWrite(data, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -854,7 +854,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeWriteAndFlush(data, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -862,7 +862,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeBind(to: address, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -870,7 +870,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeConnect(to: address, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -878,7 +878,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeRegister(promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -886,7 +886,7 @@ public final class ChannelPipeline: ChannelInvoker { if let firstOutboundCtx = firstOutboundCtx { firstOutboundCtx.invokeTriggerUserOutboundEvent(event, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1378,14 +1378,14 @@ extension ChannelPipeline.Position: Sendable {} private extension CloseMode { /// Returns the error to fail outstanding operations writes with. - var error: ChannelError { + var error: any Error { switch self { case .all: - return .ioOnClosedChannel + return ChannelError._ioOnClosedChannel case .output: - return .outputClosed + return ChannelError._outputClosed case .input: - return .inputClosed + return ChannelError._inputClosed } } } @@ -1577,7 +1577,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeRegister(promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1591,7 +1591,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeBind(to: address, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1605,7 +1605,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeConnect(to: address, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1620,7 +1620,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeWrite(data, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1647,7 +1647,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeWriteAndFlush(data, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } @@ -1671,7 +1671,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeClose(mode: mode, promise: promise) } else { - promise?.fail(ChannelError.alreadyClosed) + promise?.fail(ChannelError._alreadyClosed) } } @@ -1684,7 +1684,7 @@ public final class ChannelHandlerContext: ChannelInvoker { if let outboundNext = self.prev { outboundNext.invokeTriggerUserOutboundEvent(event, promise: promise) } else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } } diff --git a/Sources/NIOCore/DeadChannel.swift b/Sources/NIOCore/DeadChannel.swift index 3c4aec90d3..256e3480ac 100644 --- a/Sources/NIOCore/DeadChannel.swift +++ b/Sources/NIOCore/DeadChannel.swift @@ -17,31 +17,31 @@ /// all operations. private final class DeadChannelCore: ChannelCore { func localAddress0() throws -> SocketAddress { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } func remoteAddress0() throws -> SocketAddress { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } func register0(promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func registerAlreadyConfigured0(promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func bind0(to: SocketAddress, promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func connect0(to: SocketAddress, promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func write0(_ data: NIOAny, promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func flush0() { @@ -51,11 +51,11 @@ private final class DeadChannelCore: ChannelCore { } func close0(error: Error, mode: CloseMode, promise: EventLoopPromise?) { - promise?.fail(ChannelError.alreadyClosed) + promise?.fail(ChannelError._alreadyClosed) } func triggerUserOutboundEvent0(_ event: Any, promise: EventLoopPromise?) { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) } func channelRead0(_ data: NIOAny) { @@ -104,11 +104,11 @@ internal final class DeadChannel: Channel, @unchecked Sendable { let parent: Channel? = nil func setOption(_ option: Option, value: Option.Value) -> EventLoopFuture { - return self.pipeline.eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel) + return self.pipeline.eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel) } func getOption(_ option: Option) -> EventLoopFuture { - return eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel) + return eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel) } let isWritable = false diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 0e0742c73e..7bfe8fa309 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -26,7 +26,7 @@ public struct Scheduled { @usableFromInline typealias CancelationCallback = @Sendable () -> Void /* private but usableFromInline */ @usableFromInline let _promise: EventLoopPromise /* private but usableFromInline */ @usableFromInline let _cancellationTask: CancelationCallback - + @inlinable @preconcurrency public init(promise: EventLoopPromise, cancellationTask: @escaping @Sendable () -> Void) { @@ -40,7 +40,7 @@ public struct Scheduled { /// This means that cancellation is not guaranteed. @inlinable public func cancel() { - self._promise.fail(EventLoopError.cancelled) + self._promise.fail(EventLoopError._cancelled) self._cancellationTask() } @@ -1219,6 +1219,11 @@ public enum EventLoopError: Error { case shutdownFailed } +extension EventLoopError { + @usableFromInline + static let _cancelled: any Error = EventLoopError.cancelled +} + extension EventLoopError: CustomStringConvertible { public var description: String { switch self { diff --git a/Sources/NIOPosix/BSDSocketAPIPosix.swift b/Sources/NIOPosix/BSDSocketAPIPosix.swift index c62c61fbb0..7b77527b6f 100644 --- a/Sources/NIOPosix/BSDSocketAPIPosix.swift +++ b/Sources/NIOPosix/BSDSocketAPIPosix.swift @@ -277,7 +277,7 @@ extension NIOBSDSocket { option_value: &segmentSize, option_len: socklen_t(MemoryLayout.size)) #else - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported #endif } @@ -294,7 +294,7 @@ extension NIOBSDSocket { } return segmentSize #else - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported #endif } @@ -307,7 +307,7 @@ extension NIOBSDSocket { option_value: &isEnabled, option_len: socklen_t(MemoryLayout.size)) #else - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported #endif } @@ -324,7 +324,7 @@ extension NIOBSDSocket { } return enabled != 0 #else - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported #endif } } diff --git a/Sources/NIOPosix/BaseSocketChannel.swift b/Sources/NIOPosix/BaseSocketChannel.swift index c8618b97eb..21ee1f39ea 100644 --- a/Sources/NIOPosix/BaseSocketChannel.swift +++ b/Sources/NIOPosix/BaseSocketChannel.swift @@ -515,7 +515,7 @@ class BaseSocketChannel: SelectableChannel, Chan public final func localAddress0() throws -> SocketAddress { self.eventLoop.assertInEventLoop() guard self.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } return try self.socket.localAddress() } @@ -523,7 +523,7 @@ class BaseSocketChannel: SelectableChannel, Chan public final func remoteAddress0() throws -> SocketAddress { self.eventLoop.assertInEventLoop() guard self.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } return try self.socket.remoteAddress() } @@ -610,7 +610,7 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -658,7 +658,7 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -697,7 +697,7 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard self.isOpen else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) return } @@ -712,7 +712,7 @@ class BaseSocketChannel: SelectableChannel, Chan guard self.isOpen else { // Channel was already closed, fail the promise and not even queue it. - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) return } @@ -834,12 +834,12 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard self.isOpen else { - promise?.fail(ChannelError.alreadyClosed) + promise?.fail(ChannelError._alreadyClosed) return } guard mode == .all else { - promise?.fail(ChannelError.operationUnsupported) + promise?.fail(ChannelError._operationUnsupported) return } @@ -906,17 +906,17 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard self.isOpen else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) return } guard !self.lifecycleManager.isPreRegistered else { - promise?.fail(ChannelError.inappropriateOperationForState) + promise?.fail(ChannelError._inappropriateOperationForState) return } guard self.selectableEventLoop.isOpen else { - let error = EventLoopError.shutdown + let error = EventLoopError._shutdown self.pipeline.syncOperations.fireErrorCaught(error) // `close0`'s error is about the result of the `close` operation, ... self.close0(error: error, mode: .all, promise: nil) @@ -956,7 +956,7 @@ class BaseSocketChannel: SelectableChannel, Chan case let event as VsockChannelEvents.ConnectToAddress: self.connect0(to: .vsockAddress(event.address), promise: promise) default: - promise?.fail(ChannelError.operationUnsupported) + promise?.fail(ChannelError._operationUnsupported) } } @@ -1210,17 +1210,17 @@ class BaseSocketChannel: SelectableChannel, Chan self.eventLoop.assertInEventLoop() guard self.isOpen else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) return } guard pendingConnect == nil else { - promise?.fail(ChannelError.connectPending) + promise?.fail(ChannelError._connectPending) return } guard self.lifecycleManager.isPreRegistered else { - promise?.fail(ChannelError.inappropriateOperationForState) + promise?.fail(ChannelError._inappropriateOperationForState) return } @@ -1289,7 +1289,7 @@ class BaseSocketChannel: SelectableChannel, Chan assert(!self.lifecycleManager.isRegisteredFully) guard self.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } self.interestedEvent = interested diff --git a/Sources/NIOPosix/BaseStreamSocketChannel.swift b/Sources/NIOPosix/BaseStreamSocketChannel.swift index ca3c652f83..6e9c2a8334 100644 --- a/Sources/NIOPosix/BaseStreamSocketChannel.swift +++ b/Sources/NIOPosix/BaseStreamSocketChannel.swift @@ -47,7 +47,7 @@ class BaseStreamSocketChannel: BaseSocketChannel self.eventLoop.assertInEventLoop() guard self.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -66,7 +66,7 @@ class BaseStreamSocketChannel: BaseSocketChannel self.eventLoop.assertInEventLoop() guard self.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -112,7 +112,7 @@ class BaseStreamSocketChannel: BaseSocketChannel var result = ReadResult.none for _ in 1...self.maxMessagesPerRead { guard self.isOpen && !self.inputShutdown else { - throw ChannelError.eof + throw ChannelError._eof } let (buffer, readResult) = try self.recvBufferPool.buffer(allocator: self.allocator) { buffer in @@ -148,7 +148,7 @@ class BaseStreamSocketChannel: BaseSocketChannel return result } // end-of-file - throw ChannelError.eof + throw ChannelError._eof } case .wouldBlock(let bytesRead): assert(bytesRead == 0) @@ -180,7 +180,7 @@ class BaseStreamSocketChannel: BaseSocketChannel switch mode { case .output: if self.outputShutdown { - promise?.fail(ChannelError.outputClosed) + promise?.fail(ChannelError._outputClosed) return } if self.inputShutdown { @@ -197,7 +197,7 @@ class BaseStreamSocketChannel: BaseSocketChannel self.pipeline.fireUserInboundEventTriggered(ChannelEvent.outputClosed) case .input: if self.inputShutdown { - promise?.fail(ChannelError.inputClosed) + promise?.fail(ChannelError._inputClosed) return } if self.outputShutdown { @@ -261,7 +261,7 @@ class BaseStreamSocketChannel: BaseSocketChannel final override func bufferPendingWrite(data: NIOAny, promise: EventLoopPromise?) { if self.outputShutdown { - promise?.fail(ChannelError.outputClosed) + promise?.fail(ChannelError._outputClosed) return } diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index c99be2ccfd..c7c1f18dd4 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -322,7 +322,7 @@ public final class ServerBootstrap { public func withBoundSocket(_ socket: NIOBSDSocket.Handle) -> EventLoopFuture { func makeChannel(_ eventLoop: SelectableEventLoop, _ childEventLoopGroup: EventLoopGroup, _ enableMPTCP: Bool) throws -> ServerSocketChannel { if enableMPTCP { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } return try ServerSocketChannel(socket: socket, eventLoop: eventLoop, group: childEventLoopGroup) } @@ -429,7 +429,7 @@ public final class ServerBootstrap { future.flatMap { (_) -> EventLoopFuture in ctxEventLoop.assertInEventLoop() guard context.channel.isActive else { - return context.eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel) + return context.eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel) } context.fireChannelRead(data) return context.eventLoop.makeSucceededFuture(()) @@ -604,7 +604,7 @@ extension ServerBootstrap { return try await bind0( makeServerChannel: { eventLoop, childEventLoopGroup, enableMPTCP in if enableMPTCP { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } return try ServerSocketChannel( socket: socket, @@ -2014,7 +2014,7 @@ public final class NIOPipeBootstrap { case DWORD(FILE_TYPE_PIPE): break default: - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } #else var s: stat = .init() @@ -2023,7 +2023,7 @@ public final class NIOPipeBootstrap { } switch s.st_mode & S_IFMT { case S_IFREG, S_IFDIR, S_IFLNK, S_IFBLK: - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported default: () // Let's default to ok } diff --git a/Sources/NIOPosix/Errors+Any.swift b/Sources/NIOPosix/Errors+Any.swift new file mode 100644 index 0000000000..d8bc938b89 --- /dev/null +++ b/Sources/NIOPosix/Errors+Any.swift @@ -0,0 +1,38 @@ +//===----------------------------------------------------------------------===// +// +// 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 NIOCore + +// 'any Error' is unconditionally boxed, avoid allocating per use by statically boxing them. +extension ChannelError { + static let _alreadyClosed: any Error = ChannelError.alreadyClosed + static let _badInterfaceAddressFamily: any Error = ChannelError.badInterfaceAddressFamily + static let _badMulticastGroupAddressFamily: any Error = ChannelError.badMulticastGroupAddressFamily + static let _connectPending: any Error = ChannelError.connectPending + static let _eof: any Error = ChannelError.eof + static let _inappropriateOperationForState: any Error = ChannelError.inappropriateOperationForState + static let _inputClosed: any Error = ChannelError.inputClosed + static let _ioOnClosedChannel: any Error = ChannelError.ioOnClosedChannel + static let _operationUnsupported: any Error = ChannelError.operationUnsupported + static let _outputClosed: any Error = ChannelError.outputClosed + static let _unknownLocalAddress: any Error = ChannelError.unknownLocalAddress + static let _writeHostUnreachable: any Error = ChannelError.writeHostUnreachable + static let _writeMessageTooLarge: any Error = ChannelError.writeMessageTooLarge +} + +extension EventLoopError { + static let _shutdown: any Error = EventLoopError.shutdown + static let _unsupportedOperation: any Error = EventLoopError.unsupportedOperation +} diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index b9e73b16b5..81c0811896 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -304,7 +304,7 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup { private func _shutdownGracefully(queue: DispatchQueue, _ handler: @escaping ShutdownGracefullyCallback) { guard self.canBeShutDown else { queue.async { - handler(EventLoopError.unsupportedOperation) + handler(EventLoopError._unsupportedOperation) } return } diff --git a/Sources/NIOPosix/PendingDatagramWritesManager.swift b/Sources/NIOPosix/PendingDatagramWritesManager.swift index b34cb4a2b8..332add9f07 100644 --- a/Sources/NIOPosix/PendingDatagramWritesManager.swift +++ b/Sources/NIOPosix/PendingDatagramWritesManager.swift @@ -547,11 +547,11 @@ final class PendingDatagramWritesManager: PendingWritesManager { private func handleError(_ error: Error) throws -> OneWriteOperationResult { switch error { case let e as IOError where e.errnoCode == EMSGSIZE: - let (promise, result) = self.state.recoverableError(ChannelError.writeMessageTooLarge) + let (promise, result) = self.state.recoverableError(ChannelError._writeMessageTooLarge) self.fulfillPromise(promise) return result case let e as IOError where e.errnoCode == EHOSTUNREACH: - let (promise, result) = self.state.recoverableError(ChannelError.writeHostUnreachable) + let (promise, result) = self.state.recoverableError(ChannelError._writeHostUnreachable) self.fulfillPromise(promise) return result default: diff --git a/Sources/NIOPosix/PipeChannel.swift b/Sources/NIOPosix/PipeChannel.swift index 069fdfcc40..bd965c7bda 100644 --- a/Sources/NIOPosix/PipeChannel.swift +++ b/Sources/NIOPosix/PipeChannel.swift @@ -48,15 +48,15 @@ final class PipeChannel: BaseStreamSocketChannel { } override func connectSocket(to address: SocketAddress) throws -> Bool { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func connectSocket(to address: VsockAddress) throws -> Bool { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func finishConnectSocket() throws { - throw ChannelError.inappropriateOperationForState + throw ChannelError._inappropriateOperationForState } override func register(selector: Selector, interested: SelectorEventSet) throws { diff --git a/Sources/NIOPosix/PipePair.swift b/Sources/NIOPosix/PipePair.swift index cb9a92d4b3..d3b28bd63a 100644 --- a/Sources/NIOPosix/PipePair.swift +++ b/Sources/NIOPosix/PipePair.swift @@ -65,11 +65,11 @@ final class PipePair: SocketProtocol { } func connect(to address: SocketAddress) throws -> Bool { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func finishConnect() throws { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func write(pointer: UnsafeRawBufferPointer) throws -> IOResult { @@ -103,26 +103,26 @@ final class PipePair: SocketProtocol { storage: inout sockaddr_storage, storageLen: inout socklen_t, controlBytes: inout UnsafeReceivedControlBytes) throws -> IOResult { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } - + func sendmsg(pointer: UnsafeRawBufferPointer, destinationPtr: UnsafePointer?, destinationSize: socklen_t, controlBytes: UnsafeMutableRawBufferPointer) throws -> IOResult { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func sendFile(fd: CInt, offset: Int, count: Int) throws -> IOResult { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func recvmmsg(msgs: UnsafeMutableBufferPointer) throws -> IOResult { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func sendmmsg(msgs: UnsafeMutableBufferPointer) throws -> IOResult { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func shutdown(how: Shutdown) throws { @@ -142,7 +142,7 @@ final class PipePair: SocketProtocol { func close() throws { guard self.isOpen else { - throw ChannelError.alreadyClosed + throw ChannelError._alreadyClosed } let r1 = Result { if let inputFD = self.inputFD, inputFD.isOpen { @@ -159,22 +159,22 @@ final class PipePair: SocketProtocol { } func bind(to address: SocketAddress) throws { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func localAddress() throws -> SocketAddress { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func remoteAddress() throws -> SocketAddress { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func setOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option, value: T) throws { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } func getOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option) throws -> T { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index d262cf56ad..8da3951975 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -253,7 +253,7 @@ Further information: // Don't allow registration when we're closed. guard self.isOpen else { - throw EventLoopError.shutdown + throw EventLoopError._shutdown } try channel.register(selector: self._selector, interested: channel.interestedEvent) @@ -660,7 +660,7 @@ Further information: } // Fail all the scheduled tasks. for task in scheduledTasksCopy { - task.fail(EventLoopError.shutdown) + task.fail(EventLoopError._shutdown) } iterations += 1 @@ -767,7 +767,7 @@ Further information: } guard goAhead else { queue.async { - completionHandler(Result.failure(EventLoopError.shutdown)) + completionHandler(Result.failure(EventLoopError._shutdown)) } return } @@ -819,7 +819,7 @@ Further information: // This function is never called legally because the only possibly owner of an `SelectableEventLoop` is // `MultiThreadedEventLoopGroup` which calls `initiateClose` followed by `syncFinaliseClose`. queue.async { - callback(EventLoopError.unsupportedOperation) + callback(EventLoopError._unsupportedOperation) } } } diff --git a/Sources/NIOPosix/SelectorGeneric.swift b/Sources/NIOPosix/SelectorGeneric.swift index 68cf877cd6..73f8ef3e26 100644 --- a/Sources/NIOPosix/SelectorGeneric.swift +++ b/Sources/NIOPosix/SelectorGeneric.swift @@ -162,7 +162,7 @@ internal class Selector { assert(self.myThread != NIOThread.current) return try self.externalSelectorFDLock.withLock { guard self.selectorFD != -1 else { - throw EventLoopError.shutdown + throw EventLoopError._shutdown } return try body(self.selectorFD) } diff --git a/Sources/NIOPosix/SelectorKqueue.swift b/Sources/NIOPosix/SelectorKqueue.swift index 5b5c7acac1..bc10fea3f4 100644 --- a/Sources/NIOPosix/SelectorKqueue.swift +++ b/Sources/NIOPosix/SelectorKqueue.swift @@ -287,7 +287,7 @@ extension Selector: _SelectorBackendProtocol { assert(NIOThread.current != self.myThread) try self.externalSelectorFDLock.withLock { guard self.selectorFD >= 0 else { - throw EventLoopError.shutdown + throw EventLoopError._shutdown } var event = kevent() event.ident = 0 diff --git a/Sources/NIOPosix/SocketChannel.swift b/Sources/NIOPosix/SocketChannel.swift index 51fdc3a87e..67c5516378 100644 --- a/Sources/NIOPosix/SocketChannel.swift +++ b/Sources/NIOPosix/SocketChannel.swift @@ -54,7 +54,7 @@ final class SocketChannel: BaseStreamSocketChannel { var protocolSubtype = NIOBSDSocket.ProtocolSubtype.default if enableMPTCP { guard let subtype = NIOBSDSocket.ProtocolSubtype.mptcp else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } protocolSubtype = subtype } @@ -75,7 +75,7 @@ final class SocketChannel: BaseStreamSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -90,7 +90,7 @@ final class SocketChannel: BaseStreamSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -178,7 +178,7 @@ final class ServerSocketChannel: BaseSocketChannel { var protocolSubtype = NIOBSDSocket.ProtocolSubtype.default if enableMPTCP { guard let subtype = NIOBSDSocket.ProtocolSubtype.mptcp else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } protocolSubtype = subtype } @@ -212,7 +212,7 @@ final class ServerSocketChannel: BaseSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -227,7 +227,7 @@ final class ServerSocketChannel: BaseSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -249,12 +249,12 @@ final class ServerSocketChannel: BaseSocketChannel { self.eventLoop.assertInEventLoop() guard self.isOpen else { - promise?.fail(ChannelError.ioOnClosedChannel) + promise?.fail(ChannelError._ioOnClosedChannel) return } guard self.isRegistered else { - promise?.fail(ChannelError.inappropriateOperationForState) + promise?.fail(ChannelError._inappropriateOperationForState) return } @@ -282,18 +282,18 @@ final class ServerSocketChannel: BaseSocketChannel { } override func connectSocket(to address: SocketAddress) throws -> Bool { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func finishConnectSocket() throws { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func readFromSocket() throws -> ReadResult { var result = ReadResult.none for _ in 1...maxMessagesPerRead { guard self.isOpen else { - throw ChannelError.eof + throw ChannelError._eof } if let accepted = try self.socket.accept(setNonBlocking: true) { readPending = false @@ -352,7 +352,7 @@ final class ServerSocketChannel: BaseSocketChannel { ch.eventLoop.execute { ch.register().flatMapThrowing { guard ch.isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } ch.becomeActive0(promise: nil) }.whenFailure { error in @@ -366,7 +366,7 @@ final class ServerSocketChannel: BaseSocketChannel { } override func bufferPendingWrite(data: NIOAny, promise: EventLoopPromise?) { - promise?.fail(ChannelError.operationUnsupported) + promise?.fail(ChannelError._operationUnsupported) } override func markFlushPoint() { @@ -397,7 +397,7 @@ final class ServerSocketChannel: BaseSocketChannel { case let event as VsockChannelEvents.BindToAddress: self.bind0(to: .vsockAddress(event.address), promise: promise) default: - promise?.fail(ChannelError.operationUnsupported) + promise?.fail(ChannelError._operationUnsupported) } } } @@ -493,7 +493,7 @@ final class DatagramChannel: BaseSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -523,7 +523,7 @@ final class DatagramChannel: BaseSocketChannel { value: valueAsInt) default: // Explicit congestion notification is only supported for IP - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } case _ as ChannelOptions.Types.ReceivePacketInfo: let valueAsInt: CInt = value as! Bool ? 1 : 0 @@ -540,17 +540,17 @@ final class DatagramChannel: BaseSocketChannel { value: valueAsInt) default: // Receiving packet info is only supported for IP - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } case _ as ChannelOptions.Types.DatagramSegmentSize: guard System.supportsUDPSegmentationOffload else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } let segmentSize = value as! ChannelOptions.Types.DatagramSegmentSize.Value try self.socket.setUDPSegmentSize(segmentSize) case _ as ChannelOptions.Types.DatagramReceiveOffload: guard System.supportsUDPReceiveOffload else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } let enable = value as! ChannelOptions.Types.DatagramReceiveOffload.Value try self.socket.setUDPReceiveOffload(enable) @@ -563,7 +563,7 @@ final class DatagramChannel: BaseSocketChannel { self.eventLoop.assertInEventLoop() guard isOpen else { - throw ChannelError.ioOnClosedChannel + throw ChannelError._ioOnClosedChannel } switch option { @@ -583,7 +583,7 @@ final class DatagramChannel: BaseSocketChannel { name: .ipv6_recv_tclass) != 0) as! Option.Value default: // Explicit congestion notification is only supported for IP - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } case _ as ChannelOptions.Types.ReceivePacketInfo: switch self.localAddress?.protocol { @@ -595,16 +595,16 @@ final class DatagramChannel: BaseSocketChannel { name: .ipv6_recv_pktinfo) != 0) as! Option.Value default: // Receiving packet info is only supported for IP - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } case _ as ChannelOptions.Types.DatagramSegmentSize: guard System.supportsUDPSegmentationOffload else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } return try self.socket.getUDPSegmentSize() as! Option.Value case _ as ChannelOptions.Types.DatagramReceiveOffload: guard System.supportsUDPReceiveOffload else { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } return try self.socket.getUDPReceiveOffload() as! Option.Value default: @@ -636,12 +636,12 @@ final class DatagramChannel: BaseSocketChannel { } override func connectSocket(to address: VsockAddress) throws -> Bool { - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func finishConnectSocket() throws { // This is not required for connected datagram channels connect is a synchronous operation. - throw ChannelError.operationUnsupported + throw ChannelError._operationUnsupported } override func readFromSocket() throws -> ReadResult { @@ -665,7 +665,7 @@ final class DatagramChannel: BaseSocketChannel { for _ in 1...self.maxMessagesPerRead { guard self.isOpen else { - throw ChannelError.eof + throw ChannelError._eof } var controlBytes = UnsafeReceivedControlBytes(controlBytesBuffer: controlBytesBuffer) @@ -715,7 +715,7 @@ final class DatagramChannel: BaseSocketChannel { readLoop: for _ in 1...self.maxMessagesPerRead { guard self.isOpen else { - throw ChannelError.eof + throw ChannelError._eof } guard let vectorReadManager = self.vectorReadManager else { // The vector read manager went away. This happens if users unset the vector read manager @@ -868,7 +868,7 @@ final class DatagramChannel: BaseSocketChannel { override func bind0(to address: SocketAddress, promise: EventLoopPromise?) { self.eventLoop.assertInEventLoop() guard self.isRegistered else { - promise?.fail(ChannelError.inappropriateOperationForState) + promise?.fail(ChannelError._inappropriateOperationForState) return } do { @@ -1002,7 +1002,7 @@ extension DatagramChannel: MulticastChannel { self.eventLoop.assertInEventLoop() guard self.isActive else { - promise?.fail(ChannelError.inappropriateOperationForState) + promise?.fail(ChannelError._inappropriateOperationForState) return } @@ -1017,12 +1017,12 @@ extension DatagramChannel: MulticastChannel { // We need to check that we have the appropriate address types in all cases. They all need to overlap with // the address type of this channel, or this cannot work. guard let localAddress = self.localAddress else { - promise?.fail(ChannelError.unknownLocalAddress) + promise?.fail(ChannelError._unknownLocalAddress) return } guard localAddress.protocol == group.protocol else { - promise?.fail(ChannelError.badMulticastGroupAddressFamily) + promise?.fail(ChannelError._badMulticastGroupAddressFamily) return } @@ -1055,7 +1055,7 @@ extension DatagramChannel: MulticastChannel { try self.socket.setOption(level: .ipv6, name: operation.optionName(level: .ipv6), value: multicastRequest) case (.v4, .some(.v6)), (.v6, .some(.v4)), (.v4, .some(.unixDomainSocket)), (.v6, .some(.unixDomainSocket)): // Mismatched group and interface address: this is an error. - throw ChannelError.badInterfaceAddressFamily + throw ChannelError._badInterfaceAddressFamily } promise?.succeed(()) diff --git a/docker/docker-compose.2204.510.yaml b/docker/docker-compose.2204.510.yaml index 50a24d9d91..038b7fe4b3 100644 --- a/docker/docker-compose.2204.510.yaml +++ b/docker/docker-compose.2204.510.yaml @@ -24,8 +24,8 @@ services: - SWIFT_VERSION=5.10 - MAX_ALLOCS_ALLOWED_10000000_asyncsequenceproducer=21 - MAX_ALLOCS_ALLOWED_1000000_asyncwriter=1000050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers=45050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=38050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers=44050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=37050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlercontext=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlername=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlertype=8050 @@ -36,13 +36,13 @@ services: - MAX_ALLOCS_ALLOWED_1000_getHandlers=8050 - MAX_ALLOCS_ALLOWED_1000_getHandlers_sync=36 - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=26400 - - MAX_ALLOCS_ALLOWED_1000_rst_connections=147050 + - MAX_ALLOCS_ALLOWED_1000_rst_connections=145050 - MAX_ALLOCS_ALLOWED_1000_tcpbootstraps=3050 - - MAX_ALLOCS_ALLOWED_1000_tcpconnections=155050 + - MAX_ALLOCS_ALLOWED_1000_tcpconnections=152050 - MAX_ALLOCS_ALLOWED_1000_udp_reqs=6050 - MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050 - - MAX_ALLOCS_ALLOWED_1000_udpconnections=76050 - - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=393000 + - MAX_ALLOCS_ALLOWED_1000_udpconnections=75050 + - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=389000 - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050 - MAX_ALLOCS_ALLOWED_creating_10000_headers=0 - MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050 @@ -63,13 +63,13 @@ services: - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050 - MAX_ALLOCS_ALLOWED_modifying_1000_circular_buffer_elements=0 - MAX_ALLOCS_ALLOWED_modifying_byte_buffer_view=6050 - - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=343 + - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=319 - MAX_ALLOCS_ALLOWED_read_10000_chunks_from_file=110200 - - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=50100 + - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=40100 - MAX_ALLOCS_ALLOWED_schedule_and_run_10000_tasks=50050 - MAX_ALLOCS_ALLOWED_scheduling_10000_executions=89 - MAX_ALLOCS_ALLOWED_udp_1000_reqs_1_conn=6200 - - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=165050 + - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=162050 - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error # - SANITIZER_ARG=--sanitize=thread # TSan broken still diff --git a/docker/docker-compose.2204.58.yaml b/docker/docker-compose.2204.58.yaml index 35268e6c4e..ee41e86c72 100644 --- a/docker/docker-compose.2204.58.yaml +++ b/docker/docker-compose.2204.58.yaml @@ -24,8 +24,8 @@ services: - SWIFT_VERSION=5.8 - MAX_ALLOCS_ALLOWED_10000000_asyncsequenceproducer=22 - MAX_ALLOCS_ALLOWED_1000000_asyncwriter=1000050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers=45050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=38050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers=44050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=37050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlercontext=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlername=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlertype=8050 @@ -36,13 +36,13 @@ services: - MAX_ALLOCS_ALLOWED_1000_getHandlers=8050 - MAX_ALLOCS_ALLOWED_1000_getHandlers_sync=36 - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=26400 - - MAX_ALLOCS_ALLOWED_1000_rst_connections=149050 + - MAX_ALLOCS_ALLOWED_1000_rst_connections=147050 - MAX_ALLOCS_ALLOWED_1000_tcpbootstraps=4050 - - MAX_ALLOCS_ALLOWED_1000_tcpconnections=157050 + - MAX_ALLOCS_ALLOWED_1000_tcpconnections=154050 - MAX_ALLOCS_ALLOWED_1000_udp_reqs=6050 - MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050 - - MAX_ALLOCS_ALLOWED_1000_udpconnections=76050 - - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=402000 + - MAX_ALLOCS_ALLOWED_1000_udpconnections=75050 + - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=398000 - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050 - MAX_ALLOCS_ALLOWED_creating_10000_headers=0 - MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050 @@ -63,13 +63,13 @@ services: - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050 - MAX_ALLOCS_ALLOWED_modifying_1000_circular_buffer_elements=0 - MAX_ALLOCS_ALLOWED_modifying_byte_buffer_view=6050 - - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=341 + - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=334 - MAX_ALLOCS_ALLOWED_read_10000_chunks_from_file=110200 - - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=50100 + - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=40100 - MAX_ALLOCS_ALLOWED_schedule_and_run_10000_tasks=50050 - MAX_ALLOCS_ALLOWED_scheduling_10000_executions=89 - MAX_ALLOCS_ALLOWED_udp_1000_reqs_1_conn=6200 - - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=165050 + - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=162050 - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error # - SANITIZER_ARG=--sanitize=thread # TSan broken still diff --git a/docker/docker-compose.2204.59.yaml b/docker/docker-compose.2204.59.yaml index 76ee3ce812..2b53a85917 100644 --- a/docker/docker-compose.2204.59.yaml +++ b/docker/docker-compose.2204.59.yaml @@ -24,8 +24,8 @@ services: - SWIFT_VERSION=5.9 - MAX_ALLOCS_ALLOWED_10000000_asyncsequenceproducer=21 - MAX_ALLOCS_ALLOWED_1000000_asyncwriter=1000050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers=45050 - - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=38050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers=44050 + - MAX_ALLOCS_ALLOWED_1000_addHandlers_sync=37050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlercontext=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlername=8050 - MAX_ALLOCS_ALLOWED_1000_addRemoveHandlers_handlertype=8050 @@ -36,13 +36,13 @@ services: - MAX_ALLOCS_ALLOWED_1000_getHandlers=8050 - MAX_ALLOCS_ALLOWED_1000_getHandlers_sync=36 - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=26400 - - MAX_ALLOCS_ALLOWED_1000_rst_connections=149050 + - MAX_ALLOCS_ALLOWED_1000_rst_connections=147050 - MAX_ALLOCS_ALLOWED_1000_tcpbootstraps=4050 - - MAX_ALLOCS_ALLOWED_1000_tcpconnections=157050 + - MAX_ALLOCS_ALLOWED_1000_tcpconnections=154050 - MAX_ALLOCS_ALLOWED_1000_udp_reqs=6050 - MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050 - - MAX_ALLOCS_ALLOWED_1000_udpconnections=76050 - - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=402000 + - MAX_ALLOCS_ALLOWED_1000_udpconnections=75050 + - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=398000 - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050 - MAX_ALLOCS_ALLOWED_creating_10000_headers=0 - MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050 @@ -63,13 +63,13 @@ services: - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050 - MAX_ALLOCS_ALLOWED_modifying_1000_circular_buffer_elements=0 - MAX_ALLOCS_ALLOWED_modifying_byte_buffer_view=6050 - - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=350 + - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=334 - MAX_ALLOCS_ALLOWED_read_10000_chunks_from_file=110200 - - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=50100 + - MAX_ALLOCS_ALLOWED_schedule_10000_tasks=40100 - MAX_ALLOCS_ALLOWED_schedule_and_run_10000_tasks=50050 - MAX_ALLOCS_ALLOWED_scheduling_10000_executions=89 - MAX_ALLOCS_ALLOWED_udp_1000_reqs_1_conn=6200 - - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=165050 + - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=162050 - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error # - SANITIZER_ARG=--sanitize=thread # TSan broken still From e947421dd85f02e32fa90208a909833f63f77c84 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 8 Jul 2024 15:19:55 +0100 Subject: [PATCH 07/17] Allow in-place mutation of `NIOLoopBoundBox.value` (#2771) * Allow in-place mutation of `NIOLoopBoundBox.value` * Fix typos in docs --- Sources/NIOCore/NIOLoopBound.swift | 8 +++--- Tests/NIOPosixTests/CoWValue.swift | 31 +++++++++++++++++++++ Tests/NIOPosixTests/NIOLoopBoundTests.swift | 8 ++++++ 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 Tests/NIOPosixTests/CoWValue.swift diff --git a/Sources/NIOCore/NIOLoopBound.swift b/Sources/NIOCore/NIOLoopBound.swift index a3631f5b2f..6ce3c465dd 100644 --- a/Sources/NIOCore/NIOLoopBound.swift +++ b/Sources/NIOCore/NIOLoopBound.swift @@ -47,9 +47,9 @@ public struct NIOLoopBound: @unchecked Sendable { self._eventLoop.preconditionInEventLoop() return self._value } - set { + _modify { self._eventLoop.preconditionInEventLoop() - self._value = newValue + yield &self._value } } } @@ -136,9 +136,9 @@ public final class NIOLoopBoundBox: @unchecked Sendable { self._eventLoop.preconditionInEventLoop() return self._value } - set { + _modify { self._eventLoop.preconditionInEventLoop() - self._value = newValue + yield &self._value } } } diff --git a/Tests/NIOPosixTests/CoWValue.swift b/Tests/NIOPosixTests/CoWValue.swift new file mode 100644 index 0000000000..894dace81d --- /dev/null +++ b/Tests/NIOPosixTests/CoWValue.swift @@ -0,0 +1,31 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +/// A Copy on Write (CoW) type that can be used in tests to assert in-place mutation +struct CoWValue: @unchecked Sendable { + private final class UniquenessIndicator {} + + /// This reference is "copied" if not uniquely referenced + private var uniquenessIndicator = UniquenessIndicator() + + /// mutates `self` and returns a boolean whether it was mutated in place or not + /// - Returns: true if mutation happened in-place, false if Copy on Write (CoW) was triggered + mutating func mutateInPlace() -> Bool { + guard isKnownUniquelyReferenced(&self.uniquenessIndicator) else { + self.uniquenessIndicator = UniquenessIndicator() + return false + } + return true + } +} diff --git a/Tests/NIOPosixTests/NIOLoopBoundTests.swift b/Tests/NIOPosixTests/NIOLoopBoundTests.swift index 0c525125ad..cf4207ecfc 100644 --- a/Tests/NIOPosixTests/NIOLoopBoundTests.swift +++ b/Tests/NIOPosixTests/NIOLoopBoundTests.swift @@ -68,6 +68,14 @@ final class NIOLoopBoundTests: XCTestCase { }.wait()) } + func testInPlaceMutation() { + var loopBound = NIOLoopBound(CoWValue(), eventLoop: loop) + XCTAssertTrue(loopBound.value.mutateInPlace()) + + let loopBoundBox = NIOLoopBoundBox(CoWValue(), eventLoop: loop) + XCTAssertTrue(loopBoundBox.value.mutateInPlace()) + } + // MARK: - Helpers func sendableBlackhole(_ sendableThing: S) {} From 5cc05490c47a362426b2aab52dcbfdbbbaa52300 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Tue, 9 Jul 2024 08:56:26 +0200 Subject: [PATCH 08/17] [GHA] Unacceptable language check (#2766) * [GHA] Unacceptable language check # Motivation Next up replacing part of our soundness script that checked for unacceptable language. # Modification This PR adds a new GH action that checks for unacceptable language. # Result One more script replaced * PR review * Change homepage link to repo --- .github/workflows/reusable_pull_request.yml | 35 ++++++++++++++---- IntegrationTests/run-tests.sh | 4 +- IntegrationTests/tests_01_http/defines.sh | 11 +++--- .../tests_01_http/test_08_survive_signals.sh | 10 ++--- .../test_10_connection_drop_in_body_ok.sh | 4 +- .../tests_01_http/test_13_http_pipelining.sh | 2 +- .../test_18_close_with_no_keepalive.sh | 2 +- ...ion_drop_while_waiting_for_response_uds.sh | 2 +- ...ion_drop_while_waiting_for_response_tcp.sh | 2 +- .../test_21_connection_reset_tcp.sh | 4 +- NOTICE.txt | 18 ++------- README.md | 2 +- Sources/CNIOAtomics/src/cpp_magic.h | 2 +- Sources/CNIOLinux/shim.c | 2 +- Sources/CNIOSHA1/update_and_patch_sha1.sh | 2 +- Sources/NIO/Docs.docc/index.md | 2 +- Sources/NIOCore/SystemCallHelpers.swift | 2 +- Sources/NIOPosix/System.swift | 2 +- Tests/NIOPosixTests/ChannelTests.swift | 2 +- scripts/nio-diagnose | 2 +- scripts/soundness.sh | 4 +- scripts/unacceptable_language_check.sh | 37 +++++++++++++++++++ 22 files changed, 101 insertions(+), 52 deletions(-) create mode 100755 scripts/unacceptable_language_check.sh diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index 4a4b05bc5e..deadcb3067 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -3,18 +3,26 @@ name: Pull Request on: workflow_call: inputs: - enable_unit_tests: + unit_tests_enabled: type: boolean description: "Boolean to enable the unit tests job. Defaults to true." default: true - enable_api_breakage_check: + api_breakage_check_enabled: type: boolean description: "Boolean to enable the API breakage check job. Defaults to true." default: true - enable_docs_check: + docs_check_enabled: type: boolean description: "Boolean to enable the docs check job. Defaults to true." default: true + unacceptable_language_check_enabled: + type: boolean + description: "Boolean to enable the acceptable language check job. Defaults to true." + default: true + unacceptable_language_check_word_list: + type: string + description: "List of unacceptable words. Defaults to a sensible list of words." + default: "blacklist whitelist slave master sane sanity insane insanity kill killed killing hang hung hanged hanging" #ignore-unacceptable-language ## We are cancelling previously triggered workflow runs concurrency: @@ -24,7 +32,7 @@ concurrency: jobs: unit-tests: name: Unit tests - if: ${{ inputs.enable_unit_tests }} + if: ${{ inputs.unit_tests_enabled }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -46,7 +54,7 @@ jobs: api-breakage-check: name: API breakage check - if: ${{ inputs.enable_api_breakage_check }} + if: ${{ inputs.api_breakage_check_enabled }} runs-on: ubuntu-latest container: image: swift:5.10-noble @@ -65,7 +73,7 @@ jobs: docs-check: name: Documentation check - if: ${{ inputs.enable_docs_check }} + if: ${{ inputs.docs_check_enabled }} runs-on: ubuntu-latest container: image: swift:5.10-noble @@ -83,4 +91,17 @@ jobs: for target in "${targets[@]}"; do swift package plugin generate-documentation --target "$target" --warnings-as-errors --analyze --level detailed - done \ No newline at end of file + done + + unacceptable-language-check: + name: Unacceptable language check + if: ${{ inputs.unacceptable_language_check_enabled }} + runs-on: ubuntu-latest + timeout-minutes: 1 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run unacceptable language check + env: + UNACCEPTABLE_WORD_LIST: ${{ inputs.unacceptable_language_check_word_list}} + run: ./scripts/unacceptable_language_check.sh \ No newline at end of file diff --git a/IntegrationTests/run-tests.sh b/IntegrationTests/run-tests.sh index 9250b83ed9..3fcc18e82f 100755 --- a/IntegrationTests/run-tests.sh +++ b/IntegrationTests/run-tests.sh @@ -143,9 +143,9 @@ fi # report if [[ $cnt_fail > 0 ]]; then - # kill leftovers (the whole process group) + # terminate leftovers (the whole process group) trap '' TERM - kill 0 + kill 0 # ignore-unacceptable-language plugins_do summary_fail "$cnt_ok" "$cnt_fail" else diff --git a/IntegrationTests/tests_01_http/defines.sh b/IntegrationTests/tests_01_http/defines.sh index 8f4c287c25..14047d9463 100644 --- a/IntegrationTests/tests_01_http/defines.sh +++ b/IntegrationTests/tests_01_http/defines.sh @@ -141,17 +141,18 @@ function stop_server() { do_netstat "$token_type" assert_number_of_open_fds_for_pid_equals "$token_pid" "$token_open_fds" fi - kill -0 "$token_pid" # assert server is still running - ###kill -INT "$token_pid" # tell server to shut down gracefully - kill "$token_pid" # tell server to shut down gracefully + # assert server is still running + kill -0 "$token_pid" # ignore-unacceptable-language + # tell server to shut down gracefully + kill "$token_pid" # ignore-unacceptable-language for f in $(seq 20); do - if ! kill -0 "$token_pid" 2> /dev/null; then + if ! kill -0 "$token_pid" 2> /dev/null; then # ignore-unacceptable-language break # good, dead fi ps auxw | grep "$token_pid" || true sleep 0.1 done - if kill -0 "$token_pid" 2> /dev/null; then + if kill -0 "$token_pid" 2> /dev/null; then # ignore-unacceptable-language fail "server $token_pid still running" fi } diff --git a/IntegrationTests/tests_01_http/test_08_survive_signals.sh b/IntegrationTests/tests_01_http/test_08_survive_signals.sh index 33358cacc4..c906c879e0 100644 --- a/IntegrationTests/tests_01_http/test_08_survive_signals.sh +++ b/IntegrationTests/tests_01_http/test_08_survive_signals.sh @@ -23,15 +23,15 @@ echo FOO BAR > "$htdocs/some_file.txt" for f in $(seq 20); do # send some signals that are usually discarded - kill -CHLD "$server_pid" - kill -URG "$server_pid" - kill -CONT "$server_pid" - kill -WINCH "$server_pid" + kill -CHLD "$server_pid" # ignore-unacceptable-language + kill -URG "$server_pid" # ignore-unacceptable-language + kill -CONT "$server_pid" # ignore-unacceptable-language + kill -WINCH "$server_pid" # ignore-unacceptable-language do_curl "$token" "http://foobar.com/fileio/some_file.txt" > "$tmp/out.txt" & curl_pid=$! for g in $(seq 20); do - kill -URG "$server_pid" + kill -URG "$server_pid" # ignore-unacceptable-language done wait $curl_pid cmp "$htdocs/some_file.txt" "$tmp/out.txt" diff --git a/IntegrationTests/tests_01_http/test_10_connection_drop_in_body_ok.sh b/IntegrationTests/tests_01_http/test_10_connection_drop_in_body_ok.sh index 307a71d317..502e0b5864 100644 --- a/IntegrationTests/tests_01_http/test_10_connection_drop_in_body_ok.sh +++ b/IntegrationTests/tests_01_http/test_10_connection_drop_in_body_ok.sh @@ -21,7 +21,7 @@ htdocs=$(get_htdocs "$token") server_pid=$(get_server_pid "$token") socket=$(get_socket "$token") -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language ( echo -e 'POST /dynamic/echo HTTP/1.1\r\nContent-Length: 400000\r\n\r\nsome_bytes' for f in $(seq 5); do @@ -30,5 +30,5 @@ kill -0 $server_pid done ) | do_nc -U "$socket" sleep 0.1 -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language stop_server "$token" diff --git a/IntegrationTests/tests_01_http/test_13_http_pipelining.sh b/IntegrationTests/tests_01_http/test_13_http_pipelining.sh index 88c2d74a96..ad3d4330df 100644 --- a/IntegrationTests/tests_01_http/test_13_http_pipelining.sh +++ b/IntegrationTests/tests_01_http/test_13_http_pipelining.sh @@ -21,7 +21,7 @@ htdocs=$(get_htdocs "$token") server_pid=$(get_server_pid "$token") socket=$(get_socket "$token") -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language echo -e 'GET /dynamic/count-to-ten HTTP/1.1\r\n\r\nGET /dynamic/count-to-ten HTTP/1.1\r\n\r\n' | \ do_nc -U "$socket" > "$tmp/actual" diff --git a/IntegrationTests/tests_01_http/test_18_close_with_no_keepalive.sh b/IntegrationTests/tests_01_http/test_18_close_with_no_keepalive.sh index 719c0c35f6..508e22a8fc 100644 --- a/IntegrationTests/tests_01_http/test_18_close_with_no_keepalive.sh +++ b/IntegrationTests/tests_01_http/test_18_close_with_no_keepalive.sh @@ -21,7 +21,7 @@ htdocs=$(get_htdocs "$token") server_pid=$(get_server_pid "$token") socket=$(get_socket "$token") -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language echo -e 'GET /dynamic/count-to-ten HTTP/1.1\r\nConnection: close\r\n\r\n' | \ do_nc -U "$socket" > "$tmp/actual" diff --git a/IntegrationTests/tests_01_http/test_19_connection_drop_while_waiting_for_response_uds.sh b/IntegrationTests/tests_01_http/test_19_connection_drop_while_waiting_for_response_uds.sh index cc2e96c5c1..f4393dfcc7 100644 --- a/IntegrationTests/tests_01_http/test_19_connection_drop_while_waiting_for_response_uds.sh +++ b/IntegrationTests/tests_01_http/test_19_connection_drop_while_waiting_for_response_uds.sh @@ -20,7 +20,7 @@ start_server --disable-half-closure "$token" server_pid=$(get_server_pid "$token") socket=$(get_socket "$token") -kill -0 "$server_pid" +kill -0 "$server_pid" # ignore-unacceptable-language echo -e 'GET /dynamic/write-delay/10000 HTTP/1.1\r\n\r\n' | do_nc -w1 -U "$socket" sleep 0.2 diff --git a/IntegrationTests/tests_01_http/test_20_connection_drop_while_waiting_for_response_tcp.sh b/IntegrationTests/tests_01_http/test_20_connection_drop_while_waiting_for_response_tcp.sh index 202891dfd4..9a5e71de38 100644 --- a/IntegrationTests/tests_01_http/test_20_connection_drop_while_waiting_for_response_tcp.sh +++ b/IntegrationTests/tests_01_http/test_20_connection_drop_while_waiting_for_response_tcp.sh @@ -22,7 +22,7 @@ server_pid=$(get_server_pid "$token") ip=$(get_server_ip "$token") port=$(get_server_port "$token") -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language echo -e 'GET /dynamic/write-delay/10000 HTTP/1.1\r\n\r\n' | do_nc -w1 "$ip" "$port" sleep 0.2 stop_server "$token" diff --git a/IntegrationTests/tests_01_http/test_21_connection_reset_tcp.sh b/IntegrationTests/tests_01_http/test_21_connection_reset_tcp.sh index 033fb7cce1..a380a7b28e 100644 --- a/IntegrationTests/tests_01_http/test_21_connection_reset_tcp.sh +++ b/IntegrationTests/tests_01_http/test_21_connection_reset_tcp.sh @@ -22,11 +22,11 @@ server_pid=$(get_server_pid "$token") ip=$(get_server_ip "$token") port=$(get_server_port "$token") -kill -0 $server_pid +kill -0 $server_pid # ignore-unacceptable-language # try to simulate a TCP connection reset, works really well on Darwin but not on # Linux over loopback. On Linux however # `test_19_connection_drop_while_waiting_for_response_uds.sh` tests a very # similar situation. -yes "$( echo -e 'GET /dynamic/write-delay HTTP/1.1\r\n\r\n')" | nc "$ip" "$port" > /dev/null & sleep 0.5; kill -9 $! +yes "$( echo -e 'GET /dynamic/write-delay HTTP/1.1\r\n\r\n')" | nc "$ip" "$port" > /dev/null & sleep 0.5; kill -9 $! # ignore-unacceptable-language sleep 0.2 stop_server "$token" diff --git a/NOTICE.txt b/NOTICE.txt index 6caf903044..cbcc2e69bb 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -35,20 +35,10 @@ This product is heavily influenced by Netty. --- -This product contains a derivation of the Tony Stone's 'process_test_files.rb'. - - * LICENSE (Apache License 2.0): - * https://www.apache.org/licenses/LICENSE-2.0 - * HOMEPAGE: - * https://github.com/tonystone/build-tools/commit/6c417b7569df24597a48a9aa7b505b636e8f73a1 - * https://github.com/tonystone/build-tools/blob/master/source/xctest_tool.rb - ---- - This product contains NodeJS's llhttp. * LICENSE (MIT): - * https://github.com/nodejs/llhttp/blob/master/LICENSE-MIT + * https://github.com/nodejs/llhttp/blob/1e1c5b43326494e97cf8244ff57475eb72a1b62c/LICENSE-MIT * HOMEPAGE: * https://github.com/nodejs/llhttp @@ -57,7 +47,7 @@ This product contains NodeJS's llhttp. This product contains "cpp_magic.h" from Thomas Nixon & Jonathan Heathcote's uSHET * LICENSE (MIT): - * https://github.com/18sg/uSHET/blob/master/LICENSE + * https://github.com/18sg/uSHET/blob/c09e0acafd86720efe42dc15c63e0cc228244c32/lib/cpp_magic.h * HOMEPAGE: * https://github.com/18sg/uSHET @@ -68,14 +58,14 @@ This product contains "sha1.c" and "sha1.h" from FreeBSD (Copyright (C) 1995, 19 * LICENSE (BSD-3): * https://opensource.org/licenses/BSD-3-Clause * HOMEPAGE: - * https://github.com/freebsd/freebsd/tree/master/sys/crypto + * https://github.com/freebsd/freebsd-src --- This product contains a derivation of Fabian Fett's 'Base64.swift'. * LICENSE (Apache License 2.0): - * https://github.com/fabianfett/swift-base64-kit/blob/master/LICENSE + * https://github.com/swift-extras/swift-extras-base64/blob/b8af49699d59ad065b801715a5009619100245ca/LICENSE * HOMEPAGE: * https://github.com/fabianfett/swift-base64-kit diff --git a/README.md b/README.md index 889dd5a25b..39d2094975 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,7 @@ One major difference between writing concurrent code and writing synchronous cod An [`EventLoopFuture`][elf] is essentially a container for the return value of a function that will be populated *at some time in the future*. Each [`EventLoopFuture`][elf] has a corresponding [`EventLoopPromise`][elp], which is the object that the result will be put into. When the promise is succeeded, the future will be fulfilled. -If you had to poll the future to detect when it completed that would be quite inefficient, so [`EventLoopFuture`][elf] is designed to have managed callbacks. Essentially, you can hang callbacks off the future that will be executed when a result is available. The [`EventLoopFuture`][elf] will even carefully arrange the scheduling to ensure that these callbacks always execute on the event loop that initially created the promise, which helps ensure that you don't need too much synchronization around [`EventLoopFuture`][elf] callbacks. +If you had to poll the future to detect when it completed that would be quite inefficient, so [`EventLoopFuture`][elf] is designed to have managed callbacks. Essentially, you can chain callbacks off the future that will be executed when a result is available. The [`EventLoopFuture`][elf] will even carefully arrange the scheduling to ensure that these callbacks always execute on the event loop that initially created the promise, which helps ensure that you don't need too much synchronization around [`EventLoopFuture`][elf] callbacks. Another important topic for consideration is the difference between how the promise passed to `close` works as opposed to `closeFuture` on a [`Channel`][c]. For example, the promise passed into `close` will succeed after the [`Channel`][c] is closed down but before the [`ChannelPipeline`][cp] is completely cleared out. This will allow you to take action on the [`ChannelPipeline`][cp] before it is completely cleared out, if needed. If it is desired to wait for the [`Channel`][c] to close down and the [`ChannelPipeline`][cp] to be cleared out without any further action, then the better option would be to wait for the `closeFuture` to succeed. diff --git a/Sources/CNIOAtomics/src/cpp_magic.h b/Sources/CNIOAtomics/src/cpp_magic.h index 66f9f7899e..62afcce3b6 100644 --- a/Sources/CNIOAtomics/src/cpp_magic.h +++ b/Sources/CNIOAtomics/src/cpp_magic.h @@ -1,5 +1,5 @@ // -// this is https://github.com/18sg/uSHET/blob/master/lib/cpp_magic.h +// this is https://github.com/18sg/uSHET/blob/c09e0acafd86720efe42dc15c63e0cc228244c32/lib/cpp_magic.h // LICENSE: MIT // // diff --git a/Sources/CNIOLinux/shim.c b/Sources/CNIOLinux/shim.c index 832cef739e..4187317bf0 100644 --- a/Sources/CNIOLinux/shim.c +++ b/Sources/CNIOLinux/shim.c @@ -63,7 +63,7 @@ int CNIOLinux_pthread_setname_np(pthread_t thread, const char *name) { int CNIOLinux_pthread_getname_np(pthread_t thread, char *name, size_t len) { #ifdef __ANDROID__ - // https://android.googlesource.com/platform/bionic/+/master/libc/bionic/pthread_setname_np.cpp#51 + // https://android.googlesource.com/platform/bionic/+/8a18af52d9b9344497758ed04907a314a083b204/libc/bionic/pthread_setname_np.cpp#51 if (thread == pthread_self()) { return TEMP_FAILURE_RETRY(prctl(PR_GET_NAME, name)) == -1 ? -1 : 0; } diff --git a/Sources/CNIOSHA1/update_and_patch_sha1.sh b/Sources/CNIOSHA1/update_and_patch_sha1.sh index 8557928a34..a730717d4a 100755 --- a/Sources/CNIOSHA1/update_and_patch_sha1.sh +++ b/Sources/CNIOSHA1/update_and_patch_sha1.sh @@ -35,7 +35,7 @@ for f in sha1.c sha1.h; do echo " - use welcoming language (soundness check)" echo " - ensure BYTE_ORDER is defined" echo "*/" - curl -Ls "https://raw.githubusercontent.com/freebsd/freebsd/master/sys/crypto/$f" + curl -Ls "https://raw.githubusercontent.com/freebsd/freebsd/master/sys/crypto/$f" # ignore-unacceptable-language ) > "$here/c_nio_$f" for func in sha1_init sha1_pad sha1_loop sha1_result; do diff --git a/Sources/NIO/Docs.docc/index.md b/Sources/NIO/Docs.docc/index.md index 90f318e0e4..481ce3cd78 100644 --- a/Sources/NIO/Docs.docc/index.md +++ b/Sources/NIO/Docs.docc/index.md @@ -128,7 +128,7 @@ One major difference between writing concurrent code and writing synchronous cod An [`EventLoopFuture`][elf] is essentially a container for the return value of a function that will be populated *at some time in the future*. Each [`EventLoopFuture`][elf] has a corresponding [`EventLoopPromise`][elp], which is the object that the result will be put into. When the promise is succeeded, the future will be fulfilled. -If you had to poll the future to detect when it completed that would be quite inefficient, so [`EventLoopFuture`][elf] is designed to have managed callbacks. Essentially, you can hang callbacks off the future that will be executed when a result is available. The [`EventLoopFuture`][elf] will even carefully arrange the scheduling to ensure that these callbacks always execute on the event loop that initially created the promise, which helps ensure that you don't need too much synchronization around [`EventLoopFuture`][elf] callbacks. +If you had to poll the future to detect when it completed that would be quite inefficient, so [`EventLoopFuture`][elf] is designed to have managed callbacks. Essentially, you can chain callbacks off the future that will be executed when a result is available. The [`EventLoopFuture`][elf] will even carefully arrange the scheduling to ensure that these callbacks always execute on the event loop that initially created the promise, which helps ensure that you don't need too much synchronization around [`EventLoopFuture`][elf] callbacks. Another important topic for consideration is the difference between how the promise passed to `close` works as opposed to `closeFuture` on a [`Channel`][c]. For example, the promise passed into `close` will succeed after the [`Channel`][c] is closed down but before the [`ChannelPipeline`][cp] is completely cleared out. This will allow you to take action on the [`ChannelPipeline`][cp] before it is completely cleared out, if needed. If it is desired to wait for the [`Channel`][c] to close down and the [`ChannelPipeline`][cp] to be cleared out without any further action, then the better option would be to wait for the `closeFuture` to succeed. diff --git a/Sources/NIOCore/SystemCallHelpers.swift b/Sources/NIOCore/SystemCallHelpers.swift index b74092a139..cbf3b53a0f 100644 --- a/Sources/NIOCore/SystemCallHelpers.swift +++ b/Sources/NIOCore/SystemCallHelpers.swift @@ -122,7 +122,7 @@ enum SystemCalls { let err = errno #endif - // There is really nothing "sane" we can do when EINTR was reported on close. + // There is really nothing "good" we can do when EINTR was reported on close. // So just ignore it and "assume" everything is fine == we closed the file descriptor. // // For more details see: diff --git a/Sources/NIOPosix/System.swift b/Sources/NIOPosix/System.swift index 6fea9a56f8..8a96793c02 100644 --- a/Sources/NIOPosix/System.swift +++ b/Sources/NIOPosix/System.swift @@ -463,7 +463,7 @@ internal enum Posix { let err = errno #endif - // There is really nothing "sane" we can do when EINTR was reported on close. + // There is really nothing "good" we can do when EINTR was reported on close. // So just ignore it and "assume" everything is fine == we closed the file descriptor. // // For more details see: diff --git a/Tests/NIOPosixTests/ChannelTests.swift b/Tests/NIOPosixTests/ChannelTests.swift index e987d6f8ca..f947ad14f3 100644 --- a/Tests/NIOPosixTests/ChannelTests.swift +++ b/Tests/NIOPosixTests/ChannelTests.swift @@ -2991,7 +2991,7 @@ final class ReentrantWritabilityChangingHandler: ChannelInboundHandler { func channelActive(context: ChannelHandlerContext) { // We want to enqueue at least two pending writes before flushing. Neither of which - // should cause writability to change. However, we'll hang a callback off the first + // should cause writability to change. However, we'll chain a callback off the first // write which will make the channel unwritable and a writability change to be // emitted. The flush for that write should result in the writability flipping back // again. diff --git a/scripts/nio-diagnose b/scripts/nio-diagnose index e5ab7c0c50..21e6d40cf2 100755 --- a/scripts/nio-diagnose +++ b/scripts/nio-diagnose @@ -355,7 +355,7 @@ function analyse_process() { output "## Analysing pid $pid" output - if ! kill -0 "$pid" 2> /dev/null; then + if ! kill -0 "$pid" 2> /dev/null; then # ignore-unacceptable-language output "pid $pid is dead, skipping" return 0 fi diff --git a/scripts/soundness.sh b/scripts/soundness.sh index 1e6c2ca67b..c3a606ae2e 100755 --- a/scripts/soundness.sh +++ b/scripts/soundness.sh @@ -33,9 +33,9 @@ unacceptable_terms=( # We have to exclude the code of conduct as it gives examples of unacceptable # language. -if git grep --color=never -i "${unacceptable_terms[@]}" -- . ":(exclude)CODE_OF_CONDUCT.md" > /dev/null; then +if git grep --color=never -i "${unacceptable_terms[@]}" -- . ":(exclude).github/workflows/reusable_pull_request.yml" > /dev/null; then printf "\033[0;31mUnacceptable language found.\033[0m\n" - git grep -i "${unacceptable_terms[@]}" -- . ":(exclude)CODE_OF_CONDUCT.md" + git grep -i "${unacceptable_terms[@]}" -- . ":(exclude).github/workflows/reusable_pull_request.yml" exit 1 fi printf "\033[0;32mokay.\033[0m\n" diff --git a/scripts/unacceptable_language_check.sh b/scripts/unacceptable_language_check.sh new file mode 100755 index 0000000000..44c9861778 --- /dev/null +++ b/scripts/unacceptable_language_check.sh @@ -0,0 +1,37 @@ +#!/bin/bash +##===----------------------------------------------------------------------===## +## +## 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 +## +##===----------------------------------------------------------------------===## + +set -euo pipefail + +log() { printf -- "** %s\n" "$*" >&2; } +error() { printf -- "** ERROR: %s\n" "$*" >&2; } +fatal() { error "$@"; exit 1; } + +test -n "${UNACCEPTABLE_WORD_LIST:-}" || fatal "UNACCEPTABLE_WORD_LIST unset" + +log "Checking for unacceptable language..." +unacceptable_language_lines=$(git grep \ + -i -I -w \ + -H -n --column \ + -E "${UNACCEPTABLE_WORD_LIST// /|}" | grep -v "ignore-unacceptable-language" +) || true | /usr/bin/paste -s -d " " - + +if [ -n "${unacceptable_language_lines}" ]; then + fatal " ❌ Found unacceptable language: +${unacceptable_language_lines} +" +fi + +log "✅ Found no unacceptable language." \ No newline at end of file From abbb144f6678d0d830899b8bda5eea5924a61c0d Mon Sep 17 00:00:00 2001 From: George Barnett Date: Tue, 9 Jul 2024 13:27:35 +0100 Subject: [PATCH 09/17] Avoid creating a yield ID counter per async writer (#2768) Motivation: The NIOAsyncWriter uses an atomic to generate yield IDs. Each writer has its own atomic. We can save an allocatiuon per writer but using a shared atomic. Each load then wrapping increment operation with relaxed ordering takes approx 3ns on my machine. For a UInt64 this would take approx 188 years to wrap around if run in a tight loop. Modification: - Share a single yield ID counter for NIOAsyncWriter Result: Fewer allocations --- Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift index f0fc90378d..fe7e6af235 100644 --- a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift +++ b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift @@ -17,6 +17,9 @@ import DequeModule import NIOConcurrencyHelpers import _NIODataStructures +@usableFromInline +let _asyncWriterYieldIDCounter = ManagedAtomic(0) + /// The delegate of the ``NIOAsyncWriter``. It is the consumer of the yielded writes to the ``NIOAsyncWriter``. /// Furthermore, the delegate gets informed when the ``NIOAsyncWriter`` terminated. /// @@ -434,14 +437,11 @@ extension NIOAsyncWriter { } } - @usableFromInline - /* private */ internal let _yieldIDCounter = ManagedAtomic(0) - @inlinable func generateUniqueYieldID() -> YieldID { // Using relaxed is fine here since we do not need any strict ordering just a // unique ID for every yield. - .init(value: self._yieldIDCounter.loadThenWrappingIncrement(ordering: .relaxed)) + .init(value: _asyncWriterYieldIDCounter.loadThenWrappingIncrement(ordering: .relaxed)) } } From 4d87167a422c64b207a6ab81ddb7d3be99083eaa Mon Sep 17 00:00:00 2001 From: George Barnett Date: Wed, 10 Jul 2024 10:34:04 +0100 Subject: [PATCH 10/17] Add benchmark for creating NIOAsyncChannel (#2774) Motivation: NIOAsyncChannel has a number of allocs associated with it. We should have a benchmark which tracks the number of allocs. Modifications: - Add NIOCoreBenchmarks with a single benchmark Result: Better insight --- .../NIOCoreBenchmarks/Benchmarks.swift | 44 +++++++++++++++++++ Benchmarks/Package.swift | 12 +++++ ...reBenchmarks.NIOAsyncChannel.init.p90.json | 3 ++ ...reBenchmarks.NIOAsyncChannel.init.p90.json | 3 ++ ...reBenchmarks.NIOAsyncChannel.init.p90.json | 3 ++ ...reBenchmarks.NIOAsyncChannel.init.p90.json | 3 ++ 6 files changed, 68 insertions(+) create mode 100644 Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift create mode 100644 Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json create mode 100644 Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json create mode 100644 Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json create mode 100644 Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json diff --git a/Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift new file mode 100644 index 0000000000..39fa33c7df --- /dev/null +++ b/Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift @@ -0,0 +1,44 @@ +//===----------------------------------------------------------------------===// +// +// 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 Benchmark +import NIOCore +import NIOEmbedded + +let benchmarks = { + let defaultMetrics: [BenchmarkMetric] = [ + .mallocCountTotal, + ] + + Benchmark( + "NIOAsyncChannel.init", + configuration: Benchmark.Configuration(metrics: defaultMetrics) + ) { benchmark in + // Elide the cost of the 'EmbeddedChannel'. It's only used for its pipeline. + var channels: [EmbeddedChannel] = [] + channels.reserveCapacity(benchmark.scaledIterations.count) + for _ in 0 ..< benchmark.scaledIterations.count { + channels.append(EmbeddedChannel()) + } + + benchmark.startMeasurement() + defer { + benchmark.stopMeasurement() + } + for channel in channels { + let asyncChanel = try NIOAsyncChannel(wrappingChannelSynchronously: channel) + blackHole(asyncChanel) + } + } +} diff --git a/Benchmarks/Package.swift b/Benchmarks/Package.swift index 889dabe3b2..507159a7b1 100644 --- a/Benchmarks/Package.swift +++ b/Benchmarks/Package.swift @@ -37,5 +37,17 @@ let package = Package( .plugin(name: "BenchmarkPlugin", package: "package-benchmark") ] ), + .executableTarget( + name: "NIOCoreBenchmarks", + dependencies: [ + .product(name: "Benchmark", package: "package-benchmark"), + .product(name: "NIOCore", package: "swift-nio"), + .product(name: "NIOEmbedded", package: "swift-nio"), + ], + path: "Benchmarks/NIOCoreBenchmarks", + plugins: [ + .plugin(name: "BenchmarkPlugin", package: "package-benchmark") + ] + ), ] ) diff --git a/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json new file mode 100644 index 0000000000..4a3c90c5f3 --- /dev/null +++ b/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 12 +} diff --git a/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json new file mode 100644 index 0000000000..4a3c90c5f3 --- /dev/null +++ b/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 12 +} diff --git a/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json new file mode 100644 index 0000000000..4a3c90c5f3 --- /dev/null +++ b/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 12 +} diff --git a/Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json new file mode 100644 index 0000000000..4a3c90c5f3 --- /dev/null +++ b/Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 12 +} From fd31c9504c2c7421ddecbe8732cb6e21dbe97255 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Wed, 10 Jul 2024 16:51:45 +0200 Subject: [PATCH 11/17] [GHA] Move docs check to script (#2776) # Motivation For longer scripts we want to use a standalone shell script instead of using a multiline script inside the workflow yml. # Modification This PR uses the existing doc script instead of an inline script in the workflow # Result Better maintainability of the scripts. --- .github/workflows/reusable_pull_request.yml | 21 ++++++------------- ...heck.sh => check-unacceptable-language.sh} | 0 2 files changed, 6 insertions(+), 15 deletions(-) rename scripts/{unacceptable_language_check.sh => check-unacceptable-language.sh} (100%) diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index deadcb3067..751da8ec64 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -3,9 +3,9 @@ name: Pull Request on: workflow_call: inputs: - unit_tests_enabled: + unit_tests_linux_enabled: type: boolean - description: "Boolean to enable the unit tests job. Defaults to true." + description: "Boolean to enable the unit tests linux job. Defaults to true." default: true api_breakage_check_enabled: type: boolean @@ -30,9 +30,9 @@ concurrency: cancel-in-progress: true jobs: - unit-tests: + unit-tests-linux: name: Unit tests - if: ${{ inputs.unit_tests_enabled }} + if: ${{ inputs.unit_tests_linux_enabled }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -82,16 +82,7 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - name: Run documentation check - shell: bash - run: | - set -eu - - raw_targets=$(sed -E -n -e 's/^.* - documentation_targets: \[(.*)\].*$/\1/p' .spi.yml) - targets=(${raw_targets//,/ }) - - for target in "${targets[@]}"; do - swift package plugin generate-documentation --target "$target" --warnings-as-errors --analyze --level detailed - done + run: ./scripts/check-docs.sh unacceptable-language-check: name: Unacceptable language check @@ -104,4 +95,4 @@ jobs: - name: Run unacceptable language check env: UNACCEPTABLE_WORD_LIST: ${{ inputs.unacceptable_language_check_word_list}} - run: ./scripts/unacceptable_language_check.sh \ No newline at end of file + run: ./scripts/check-unacceptable-language.sh \ No newline at end of file diff --git a/scripts/unacceptable_language_check.sh b/scripts/check-unacceptable-language.sh similarity index 100% rename from scripts/unacceptable_language_check.sh rename to scripts/check-unacceptable-language.sh From 86316020a55e1a65f7a673a58daaac2a96216912 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Thu, 11 Jul 2024 12:23:24 +0200 Subject: [PATCH 12/17] [GHA] Benchmark job (#2780) --- .github/workflows/pull_request.yml | 4 +- .github/workflows/reusable_pull_request.yml | 42 ++++++++++++++++++- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 0 .../NIOPosixBenchmarks.TCPEcho.p90.json | 0 ...sixBenchmarks.TCPEchoAsyncChannel.p90.json | 0 ...reBenchmarks.NIOAsyncChannel.init.p90.json | 3 ++ .../NIOPosixBenchmarks.TCPEcho.p90.json | 3 ++ ...sixBenchmarks.TCPEchoAsyncChannel.p90.json | 3 ++ 8 files changed, 53 insertions(+), 2 deletions(-) rename Benchmarks/Thresholds/{main => nightly-main}/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json (100%) rename Benchmarks/Thresholds/{main => nightly-main}/NIOPosixBenchmarks.TCPEcho.p90.json (100%) rename Benchmarks/Thresholds/{main => nightly-main}/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json (100%) create mode 100644 Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json create mode 100644 Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEcho.p90.json create mode 100644 Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 5ebeeca100..b2e62dbd26 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,4 +7,6 @@ on: jobs: call-reusable-pull-request-workflow: name: Checks - uses: ./.github/workflows/reusable_pull_request.yml \ No newline at end of file + uses: ./.github/workflows/reusable_pull_request.yml + with: + benchmarks_linux_package_path: "Benchmarks" \ No newline at end of file diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index 751da8ec64..f6ae37340f 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -7,6 +7,14 @@ on: type: boolean description: "Boolean to enable the unit tests linux job. Defaults to true." default: true + benchmarks_linux_enabled: + type: boolean + description: "Boolean to enable the benchmarks linux job. Defaults to true." + default: true + benchmarks_linux_package_path: + type: string + description: "Path to the package containing the benchmarks. Defaults to the repository root." + default: "." api_breakage_check_enabled: type: boolean description: "Boolean to enable the API breakage check job. Defaults to true." @@ -40,7 +48,7 @@ jobs: swift: - image: swift:5.8-jammy - image: swift:5.9-jammy - - image: swift:5.10-noble + - image: swift:5.10-jammy - image: swiftlang/swift:nightly-6.0-jammy - image: swiftlang/swift:nightly-main-jammy container: @@ -51,6 +59,38 @@ jobs: uses: actions/checkout@v4 - name: Run tests run: swift test + + benchmarks-linux: + name: Benchmarks + if: ${{ inputs.benchmarks_linux_enabled }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + swift: + - image: swift:5.8-jammy + swift_version: "5.8" + - image: swift:5.9-jammy + swift_version: "5.9" + - image: swift:5.10-jammy + swift_version: "5.10" + - image: swiftlang/swift:nightly-6.0-jammy + swift_version: "nightly-next" + - image: swiftlang/swift:nightly-main-jammy + swift_version: "nightly-main" + container: + image: ${{ matrix.swift.image }} + timeout-minutes: 20 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run tests + env: + PACKAGE_PATH: ${{ inputs.benchmarks_linux_package_path }} + SWIFT_VERSION: ${{ matrix.swift.swift_version }} + run: | + apt-get update -y -q && apt-get install -y -q libjemalloc-dev + swift package --package-path ${PACKAGE_PATH} --disable-sandbox benchmark baseline check --check-absolute-path ${PACKAGE_PATH}/Thresholds/${SWIFT_VERSION}/ api-breakage-check: name: API breakage check diff --git a/Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json similarity index 100% rename from Benchmarks/Thresholds/main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json rename to Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json diff --git a/Benchmarks/Thresholds/main/NIOPosixBenchmarks.TCPEcho.p90.json b/Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEcho.p90.json similarity index 100% rename from Benchmarks/Thresholds/main/NIOPosixBenchmarks.TCPEcho.p90.json rename to Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEcho.p90.json diff --git a/Benchmarks/Thresholds/main/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json b/Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json similarity index 100% rename from Benchmarks/Thresholds/main/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json rename to Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json diff --git a/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json new file mode 100644 index 0000000000..4a3c90c5f3 --- /dev/null +++ b/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 12 +} diff --git a/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEcho.p90.json b/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEcho.p90.json new file mode 100644 index 0000000000..c6a93680d0 --- /dev/null +++ b/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEcho.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 108 +} diff --git a/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json b/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json new file mode 100644 index 0000000000..361ee44ab6 --- /dev/null +++ b/Benchmarks/Thresholds/nightly-next/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json @@ -0,0 +1,3 @@ +{ + "mallocCountTotal" : 165000 +} From 0f4c110bd75f3ad31c9f2f9e2789ff66c85480ad Mon Sep 17 00:00:00 2001 From: George Barnett Date: Fri, 12 Jul 2024 09:02:04 +0100 Subject: [PATCH 13/17] Combine the two NIOAsyncChannel channel handlers (#2779) Motivation: The NIOAsyncChannel allocates 12 times on init. 4 of these allocations come from creating two channel handlers and two channel handler contexts. There's no inherent reason that these channel handlers can't be combined to eliminate two allocations (one handler and one context). Modifications: - Combine `NIOAsyncChannelInboundStreamChannelHandler` and `NIOAsyncChannelOutboundWriterHandler` into a single `NIOAsyncChannelHandler`. Most of this was straightforward as only a few handler operations were duplicated across both. - Add a 'NIOAsyncChannelHandlerWriterDelegate' in place of the 'NIOAsyncChannelOutboundWriterHandler.Delegate'. One knock on from this is that the new delegate stores callbacks rather than the concrete type of the handler. This is necessary to prevent the generics from the new channel handler bubbling up to the outbound writer (which would break API and be somewhat odd). Result: Fewer allocations --- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 2 +- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 2 +- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 2 +- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 2 +- ...reBenchmarks.NIOAsyncChannel.init.p90.json | 2 +- .../NIOCore/AsyncChannel/AsyncChannel.swift | 37 ++- ...andler.swift => AsyncChannelHandler.swift} | 311 +++++++++++++----- .../AsyncChannelInboundStream.swift | 61 +--- .../AsyncChannelOutboundWriter.swift | 17 +- .../AsyncChannelOutboundWriterHandler.swift | 232 ------------- .../AsyncChannel/AsyncChannelTests.swift | 2 +- 11 files changed, 292 insertions(+), 378 deletions(-) rename Sources/NIOCore/AsyncChannel/{AsyncChannelInboundStreamChannelHandler.swift => AsyncChannelHandler.swift} (56%) delete mode 100644 Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift diff --git a/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json index 4a3c90c5f3..2554cd2c39 100644 --- a/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json +++ b/Benchmarks/Thresholds/5.10/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal" : 12 + "mallocCountTotal" : 10 } diff --git a/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json index 4a3c90c5f3..2554cd2c39 100644 --- a/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json +++ b/Benchmarks/Thresholds/5.8/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal" : 12 + "mallocCountTotal" : 10 } diff --git a/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json index 4a3c90c5f3..2554cd2c39 100644 --- a/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json +++ b/Benchmarks/Thresholds/5.9/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal" : 12 + "mallocCountTotal" : 10 } diff --git a/Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json index 4a3c90c5f3..2554cd2c39 100644 --- a/Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json +++ b/Benchmarks/Thresholds/nightly-main/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal" : 12 + "mallocCountTotal" : 10 } diff --git a/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json b/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json index 4a3c90c5f3..2554cd2c39 100644 --- a/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json +++ b/Benchmarks/Thresholds/nightly-next/NIOCoreBenchmarks.NIOAsyncChannel.init.p90.json @@ -1,3 +1,3 @@ { - "mallocCountTotal" : 12 + "mallocCountTotal" : 10 } diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannel.swift b/Sources/NIOCore/AsyncChannel/AsyncChannel.swift index 27ef318293..eda10858ab 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannel.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannel.swift @@ -315,16 +315,27 @@ extension Channel { ) throws -> (NIOAsyncChannelInboundStream, NIOAsyncChannelOutboundWriter) { self.eventLoop.assertInEventLoop() - let inboundStream = try NIOAsyncChannelInboundStream.makeWrappingHandler( - channel: self, + let handler = NIOAsyncChannelHandler( + eventLoop: self.eventLoop, + transformation: .syncWrapping { $0 }, + isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled + ) + + let inboundStream = try NIOAsyncChannelInboundStream( + eventLoop: self.eventLoop, + handler: handler, backPressureStrategy: backPressureStrategy, closeOnDeinit: closeOnDeinit ) + let writer = try NIOAsyncChannelOutboundWriter( - channel: self, + eventLoop: self.eventLoop, + handler: handler, isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled, closeOnDeinit: closeOnDeinit ) + + try self.pipeline.syncOperations.addHandler(handler) return (inboundStream, writer) } @@ -338,17 +349,27 @@ extension Channel { ) throws -> (NIOAsyncChannelInboundStream, NIOAsyncChannelOutboundWriter) { self.eventLoop.assertInEventLoop() - let inboundStream = try NIOAsyncChannelInboundStream.makeTransformationHandler( - channel: self, + let handler = NIOAsyncChannelHandler( + eventLoop: self.eventLoop, + transformation: .transformation(channelReadTransformation: channelReadTransformation), + isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled + ) + + let inboundStream = try NIOAsyncChannelInboundStream( + eventLoop: self.eventLoop, + handler: handler, backPressureStrategy: backPressureStrategy, - closeOnDeinit: closeOnDeinit, - channelReadTransformation: channelReadTransformation + closeOnDeinit: closeOnDeinit ) + let writer = try NIOAsyncChannelOutboundWriter( - channel: self, + eventLoop: self.eventLoop, + handler: handler, isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled, closeOnDeinit: closeOnDeinit ) + + try self.pipeline.syncOperations.addHandler(handler) return (inboundStream, writer) } } diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStreamChannelHandler.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelHandler.swift similarity index 56% rename from Sources/NIOCore/AsyncChannel/AsyncChannelInboundStreamChannelHandler.swift rename to Sources/NIOCore/AsyncChannel/AsyncChannelHandler.swift index dff04c3f37..af0f659c38 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStreamChannelHandler.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannelHandler.swift @@ -2,7 +2,7 @@ // // This source file is part of the SwiftNIO open source project // -// Copyright (c) 2022-2023 Apple Inc. and the SwiftNIO project authors +// Copyright (c) 2022-2024 Apple Inc. and the SwiftNIO project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -12,11 +12,15 @@ // //===----------------------------------------------------------------------===// +import DequeModule + /// A ``ChannelHandler`` that is used to transform the inbound portion of a NIO -/// ``Channel`` into an asynchronous sequence that supports back-pressure. +/// ``Channel`` into an asynchronous sequence that supports back-pressure. It's also used +/// to write the outbound portion of a NIO ``Channel`` from Swift Concurrency with back-pressure +/// support. @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) @usableFromInline -internal final class NIOAsyncChannelInboundStreamChannelHandler: ChannelDuplexHandler { +internal final class NIOAsyncChannelHandler { @usableFromInline enum _ProducingState { // Not .stopProducing @@ -29,18 +33,12 @@ internal final class NIOAsyncChannelInboundStreamChannelHandler.Source /// The source of the asynchronous sequence. @@ -77,49 +75,103 @@ internal final class NIOAsyncChannelInboundStreamChannelHandler + > + + @usableFromInline + typealias Sink = Writer.Sink + + /// The sink of the ``NIOAsyncWriter``. + @usableFromInline + var sink: Sink? + + /// The writer of the ``NIOAsyncWriter``. + /// + /// The reference is retained until `channelActive` is fired. This avoids situations + /// where `deinit` is called on the unfinished writer because the `Channel` was never returned + /// to the caller (e.g. because a connect failed or or happy-eyeballs created multiple + /// channels). + /// + /// Effectively `channelActive` is used at the point in time at which NIO cedes ownership of + /// the writer to the caller. + @usableFromInline + var writer: Writer? + + @usableFromInline + let isOutboundHalfClosureEnabled: Bool + @inlinable init( eventLoop: EventLoop, - transformation: Transformation + transformation: Transformation, + isOutboundHalfClosureEnabled: Bool ) { self.eventLoop = eventLoop self.transformation = transformation + self.isOutboundHalfClosureEnabled = isOutboundHalfClosureEnabled } +} + +@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) +extension NIOAsyncChannelHandler: ChannelInboundHandler { + @usableFromInline + typealias InboundIn = InboundIn - /// Creates a new ``NIOAsyncChannelInboundStreamChannelHandler`` which is used when the pipeline got synchronously wrapped. @inlinable - static func makeHandler( - eventLoop: EventLoop - ) -> NIOAsyncChannelInboundStreamChannelHandler where InboundIn == ProducerElement { - return .init( - eventLoop: eventLoop, - transformation: .syncWrapping { $0 } - ) + func handlerAdded(context: ChannelHandlerContext) { + self.context = context } - /// Creates a new ``NIOAsyncChannelInboundStreamChannelHandler`` which has hooks for transformations. @inlinable - static func makeHandlerWithTransformations( - eventLoop: EventLoop, - channelReadTransformation: @Sendable @escaping (InboundIn) -> EventLoopFuture - ) -> NIOAsyncChannelInboundStreamChannelHandler where InboundIn == Channel { - return .init( - eventLoop: eventLoop, - transformation: .transformation( - channelReadTransformation: channelReadTransformation - ) - ) + func handlerRemoved(context: ChannelHandlerContext) { + self._finishSource(context: context) + self.sink?.finish(error: ChannelError._ioOnClosedChannel) + self.context = nil + self.writer = nil } @inlinable - func handlerAdded(context: ChannelHandlerContext) { - self.context = context + func channelActive(context: ChannelHandlerContext) { + // Drop the writer ref, the caller is responsible for it now. + self.writer = nil + context.fireChannelActive() } @inlinable - func handlerRemoved(context: ChannelHandlerContext) { + func channelInactive(context: ChannelHandlerContext) { self._finishSource(context: context) - self.context = nil + self.sink?.finish(error: ChannelError._ioOnClosedChannel) + context.fireChannelInactive() + } + + + @inlinable + func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) { + switch event { + case ChannelEvent.inputClosed: + self._finishSource(context: context) + case ChannelEvent.outputClosed: + self.sink?.finish() + default: + break + } + + context.fireUserInboundEventTriggered(event) + } + + @inlinable + func channelWritabilityChanged(context: ChannelHandlerContext) { + self.sink?.setWritability(to: context.channel.isWritable) + context.fireChannelWritabilityChanged() + } + + @inlinable + func errorCaught(context: ChannelHandlerContext, error: Error) { + self._finishSource(with: error, context: context) + context.fireErrorCaught(error) } @inlinable @@ -153,43 +205,20 @@ internal final class NIOAsyncChannelInboundStreamChannelHandler - ) { - context.eventLoop.preconditionInEventLoop() - - switch result { - case .success(let transformed): - self.buffer.append(transformed) - // We are delivering out of band here since the future can complete at any point - self._deliverReads(context: context) - - case .failure: - // Transformation failed. Nothing to really do here this must be handled in the transformation - // futures themselves. - break - } - } - @inlinable func channelReadComplete(context: ChannelHandlerContext) { self._deliverReads(context: context) context.fireChannelReadComplete() } +} - @inlinable - func channelInactive(context: ChannelHandlerContext) { - self._finishSource(context: context) - context.fireChannelInactive() - } +@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) +extension NIOAsyncChannelHandler: ChannelOutboundHandler { + @usableFromInline + typealias OutboundIn = Any - @inlinable - func errorCaught(context: ChannelHandlerContext, error: Error) { - self._finishSource(with: error, context: context) - context.fireErrorCaught(error) - } + @usableFromInline + typealias OutboundOut = OutboundOut @inlinable func read(context: ChannelHandlerContext) { @@ -202,17 +231,28 @@ internal final class NIOAsyncChannelInboundStreamChannelHandler + ) { + context.eventLoop.preconditionInEventLoop() + + switch result { + case .success(let transformed): + self.buffer.append(transformed) + // We are delivering out of band here since the future can complete at any point + self._deliverReads(context: context) + + case .failure: + // Transformation failed. Nothing to really do here this must be handled in the transformation + // futures themselves. break } - - context.fireUserInboundEventTriggered(event) } @inlinable @@ -258,8 +298,9 @@ internal final class NIOAsyncChannelInboundStreamChannelHandler Void @inlinable - init(handler: NIOAsyncChannelInboundStreamChannelHandler) { + init(handler: NIOAsyncChannelHandler) { self.eventLoop = handler.eventLoop self._didTerminate = handler._didTerminate self._produceMore = handler._produceMore @@ -329,6 +370,126 @@ struct NIOAsyncChannelInboundStreamChannelHandlerProducerDelegate: @unchecked Se } } +@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) +@usableFromInline +struct NIOAsyncChannelHandlerWriterDelegate: NIOAsyncWriterSinkDelegate, @unchecked Sendable { + @usableFromInline + let eventLoop: EventLoop + + @usableFromInline + let _didYieldContentsOf: (Deque) -> Void + + @usableFromInline + let _didYield: (Element) -> Void + + @usableFromInline + let _didTerminate: ((any Error)?) -> Void + + @inlinable + init(handler: NIOAsyncChannelHandler) { + self.eventLoop = handler.eventLoop + self._didYieldContentsOf = handler._didYield(sequence:) + self._didYield = handler._didYield(element:) + self._didTerminate = handler._didTerminate(error:) + } + + @inlinable + func didYield(contentsOf sequence: Deque) { + if self.eventLoop.inEventLoop { + self._didYieldContentsOf(sequence) + } else { + self.eventLoop.execute { + self._didYieldContentsOf(sequence) + } + } + } + + @inlinable + func didYield(_ element: Element) { + if self.eventLoop.inEventLoop { + self._didYield(element) + } else { + self.eventLoop.execute { + self._didYield(element) + } + } + } + + @inlinable + func didTerminate(error: (any Error)?) { + if self.eventLoop.inEventLoop { + self._didTerminate(error) + } else { + self.eventLoop.execute { + self._didTerminate(error) + } + } + } +} + +@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) +extension NIOAsyncChannelHandler { + @inlinable + func _didYield(sequence: Deque) { + // This is always called from an async context, so we must loop-hop. + // Because we always loop-hop, we're always at the top of a stack frame. As this + // is the only source of writes for us, and as this channel handler doesn't implement + // func write(), we cannot possibly re-entrantly write. That means we can skip many of the + // awkward re-entrancy protections NIO usually requires, and can safely just do an iterative + // write. + self.eventLoop.preconditionInEventLoop() + guard let context = self.context else { + // Already removed from the channel by now, we can stop. + return + } + + self._doOutboundWrites(context: context, writes: sequence) + } + + @inlinable + func _didYield(element: OutboundOut) { + // This is always called from an async context, so we must loop-hop. + // Because we always loop-hop, we're always at the top of a stack frame. As this + // is the only source of writes for us, and as this channel handler doesn't implement + // func write(), we cannot possibly re-entrantly write. That means we can skip many of the + // awkward re-entrancy protections NIO usually requires, and can safely just do an iterative + // write. + self.eventLoop.preconditionInEventLoop() + guard let context = self.context else { + // Already removed from the channel by now, we can stop. + return + } + + self._doOutboundWrite(context: context, write: element) + } + + @inlinable + func _didTerminate(error: Error?) { + self.eventLoop.preconditionInEventLoop() + + if self.isOutboundHalfClosureEnabled { + self.context?.close(mode: .output, promise: nil) + } + + self.sink = nil + } + + @inlinable + func _doOutboundWrites(context: ChannelHandlerContext, writes: Deque) { + for write in writes { + context.write(self.wrapOutboundOut(write), promise: nil) + } + + context.flush() + } + + @inlinable + func _doOutboundWrite(context: ChannelHandlerContext, write: OutboundOut) { + context.write(self.wrapOutboundOut(write), promise: nil) + context.flush() + } +} + @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) @available(*, unavailable) -extension NIOAsyncChannelInboundStreamChannelHandler: Sendable {} +extension NIOAsyncChannelHandler: Sendable {} diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStream.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStream.swift index 0a672dc3ed..7134359a2c 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStream.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannelInboundStream.swift @@ -18,7 +18,12 @@ @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public struct NIOAsyncChannelInboundStream: Sendable { @usableFromInline - typealias Producer = NIOThrowingAsyncSequenceProducer + typealias Producer = NIOThrowingAsyncSequenceProducer< + Inbound, + Error, + NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark, + NIOAsyncChannelHandlerProducerDelegate + > /// A source used for driving a ``NIOAsyncChannelInboundStream`` during tests. public struct TestSource { @@ -77,13 +82,13 @@ public struct NIOAsyncChannelInboundStream: Sendable { } @inlinable - init( - channel: Channel, + init( + eventLoop: any EventLoop, + handler: NIOAsyncChannelHandler, backPressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark?, - closeOnDeinit: Bool, - handler: NIOAsyncChannelInboundStreamChannelHandler + closeOnDeinit: Bool ) throws { - channel.eventLoop.preconditionInEventLoop() + eventLoop.preconditionInEventLoop() let strategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark if let userProvided = backPressureStrategy { @@ -97,52 +102,12 @@ public struct NIOAsyncChannelInboundStream: Sendable { let sequence = Producer.makeSequence( backPressureStrategy: strategy, finishOnDeinit: closeOnDeinit, - delegate: NIOAsyncChannelInboundStreamChannelHandlerProducerDelegate(handler: handler) + delegate: NIOAsyncChannelHandlerProducerDelegate(handler: handler) ) + handler.source = sequence.source - try channel.pipeline.syncOperations.addHandler(handler) self._backing = .producer(sequence.sequence) } - - /// Creates a new ``NIOAsyncChannelInboundStream`` which is used when the pipeline got synchronously wrapped. - @inlinable - static func makeWrappingHandler( - channel: Channel, - backPressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark?, - closeOnDeinit: Bool - ) throws -> NIOAsyncChannelInboundStream { - let handler = NIOAsyncChannelInboundStreamChannelHandler.makeHandler( - eventLoop: channel.eventLoop - ) - - return try .init( - channel: channel, - backPressureStrategy: backPressureStrategy, - closeOnDeinit: closeOnDeinit, - handler: handler - ) - } - - /// Creates a new ``NIOAsyncChannelInboundStream`` which has hooks for transformations. - @inlinable - static func makeTransformationHandler( - channel: Channel, - backPressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark?, - closeOnDeinit: Bool, - channelReadTransformation: @Sendable @escaping (Channel) -> EventLoopFuture - ) throws -> NIOAsyncChannelInboundStream { - let handler = NIOAsyncChannelInboundStreamChannelHandler.makeHandlerWithTransformations( - eventLoop: channel.eventLoop, - channelReadTransformation: channelReadTransformation - ) - - return try .init( - channel: channel, - backPressureStrategy: backPressureStrategy, - closeOnDeinit: closeOnDeinit, - handler: handler - ) - } } @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriter.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriter.swift index 37731b6f37..8ef5f12cf8 100644 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriter.swift +++ b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriter.swift @@ -20,7 +20,10 @@ @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public struct NIOAsyncChannelOutboundWriter: Sendable { @usableFromInline - typealias _Writer = NIOAsyncChannelOutboundWriterHandler.Writer + typealias _Writer = NIOAsyncWriter< + OutboundOut, + NIOAsyncChannelHandlerWriterDelegate + > /// An `AsyncSequence` backing a ``NIOAsyncChannelOutboundWriter`` for testing purposes. public struct TestSink: AsyncSequence { @@ -82,15 +85,13 @@ public struct NIOAsyncChannelOutboundWriter: Sendable { } @inlinable - init( - channel: Channel, + init( + eventLoop: any EventLoop, + handler: NIOAsyncChannelHandler, isOutboundHalfClosureEnabled: Bool, closeOnDeinit: Bool ) throws { - let handler = NIOAsyncChannelOutboundWriterHandler( - eventLoop: channel.eventLoop, - isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled - ) + eventLoop.preconditionInEventLoop() let writer = _Writer.makeWriter( elementType: OutboundOut.self, isWritable: true, @@ -101,8 +102,6 @@ public struct NIOAsyncChannelOutboundWriter: Sendable { handler.sink = writer.sink handler.writer = writer.writer - try channel.pipeline.syncOperations.addHandler(handler) - self._backing = .writer(writer.writer) } diff --git a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift b/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift deleted file mode 100644 index 0e73fde347..0000000000 --- a/Sources/NIOCore/AsyncChannel/AsyncChannelOutboundWriterHandler.swift +++ /dev/null @@ -1,232 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the SwiftNIO open source project -// -// Copyright (c) 2022-2023 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 DequeModule - -/// A ``ChannelHandler`` that is used to write the outbound portion of a NIO -/// ``Channel`` from Swift Concurrency with back-pressure support. -@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -@usableFromInline -internal final class NIOAsyncChannelOutboundWriterHandler: ChannelDuplexHandler { - @usableFromInline typealias InboundIn = Any - @usableFromInline typealias InboundOut = Any - @usableFromInline typealias OutboundIn = Any - @usableFromInline typealias OutboundOut = OutboundOut - - @usableFromInline - typealias Writer = NIOAsyncWriter< - OutboundOut, - NIOAsyncChannelOutboundWriterHandler.Delegate - > - - @usableFromInline - typealias Sink = Writer.Sink - - /// The sink of the ``NIOAsyncWriter``. - @usableFromInline - var sink: Sink? - - /// The writer of the ``NIOAsyncWriter``. - /// - /// The reference is retained until `channelActive` is fired. This avoids situations - /// where `deinit` is called on the unfinished writer because the `Channel` was never returned - /// to the caller (e.g. because a connect failed or or happy-eyeballs created multiple - /// channels). - /// - /// Effectively `channelActive` is used at the point in time at which NIO cedes ownership of - /// the writer to the caller. - @usableFromInline - var writer: Writer? - - /// The channel handler context. - @usableFromInline - var context: ChannelHandlerContext? - - /// The event loop. - @usableFromInline - let eventLoop: EventLoop - - @usableFromInline - let isOutboundHalfClosureEnabled: Bool - - @inlinable - init( - eventLoop: EventLoop, - isOutboundHalfClosureEnabled: Bool - ) { - self.eventLoop = eventLoop - self.isOutboundHalfClosureEnabled = isOutboundHalfClosureEnabled - } - - @inlinable - func _didYield(sequence: Deque) { - // This is always called from an async context, so we must loop-hop. - // Because we always loop-hop, we're always at the top of a stack frame. As this - // is the only source of writes for us, and as this channel handler doesn't implement - // func write(), we cannot possibly re-entrantly write. That means we can skip many of the - // awkward re-entrancy protections NIO usually requires, and can safely just do an iterative - // write. - self.eventLoop.preconditionInEventLoop() - guard let context = self.context else { - // Already removed from the channel by now, we can stop. - return - } - - self._doOutboundWrites(context: context, writes: sequence) - } - - @inlinable - func _didYield(element: OutboundOut) { - // This is always called from an async context, so we must loop-hop. - // Because we always loop-hop, we're always at the top of a stack frame. As this - // is the only source of writes for us, and as this channel handler doesn't implement - // func write(), we cannot possibly re-entrantly write. That means we can skip many of the - // awkward re-entrancy protections NIO usually requires, and can safely just do an iterative - // write. - self.eventLoop.preconditionInEventLoop() - guard let context = self.context else { - // Already removed from the channel by now, we can stop. - return - } - - self._doOutboundWrite(context: context, write: element) - } - - @inlinable - func _didTerminate(error: Error?) { - self.eventLoop.preconditionInEventLoop() - - if self.isOutboundHalfClosureEnabled { - self.context?.close(mode: .output, promise: nil) - } - - self.sink = nil - } - - @inlinable - func _doOutboundWrites(context: ChannelHandlerContext, writes: Deque) { - for write in writes { - context.write(self.wrapOutboundOut(write), promise: nil) - } - - context.flush() - } - - @inlinable - func _doOutboundWrite(context: ChannelHandlerContext, write: OutboundOut) { - context.write(self.wrapOutboundOut(write), promise: nil) - context.flush() - } - - @inlinable - func handlerAdded(context: ChannelHandlerContext) { - self.context = context - } - - @inlinable - func handlerRemoved(context: ChannelHandlerContext) { - self.context = nil - self.sink?.finish(error: ChannelError._ioOnClosedChannel) - self.writer = nil - } - - @inlinable - func channelActive(context: ChannelHandlerContext) { - // Drop the writer ref, the caller is responsible for it now. - self.writer = nil - context.fireChannelActive() - } - - @inlinable - func channelInactive(context: ChannelHandlerContext) { - self.sink?.finish(error: ChannelError._ioOnClosedChannel) - context.fireChannelInactive() - } - - @inlinable - func channelWritabilityChanged(context: ChannelHandlerContext) { - self.sink?.setWritability(to: context.channel.isWritable) - context.fireChannelWritabilityChanged() - } - - @inlinable - func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) { - switch event { - case ChannelEvent.outputClosed: - self.sink?.finish() - default: - break - } - - context.fireUserInboundEventTriggered(event) - } -} - -@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -extension NIOAsyncChannelOutboundWriterHandler { - @usableFromInline - struct Delegate: @unchecked Sendable, NIOAsyncWriterSinkDelegate { - @usableFromInline - typealias Element = OutboundOut - - @usableFromInline - let eventLoop: EventLoop - - @usableFromInline - let handler: NIOAsyncChannelOutboundWriterHandler - - @inlinable - init(handler: NIOAsyncChannelOutboundWriterHandler) { - self.eventLoop = handler.eventLoop - self.handler = handler - } - - @inlinable - func didYield(contentsOf sequence: Deque) { - if self.eventLoop.inEventLoop { - self.handler._didYield(sequence: sequence) - } else { - self.eventLoop.execute { - self.handler._didYield(sequence: sequence) - } - } - } - - @inlinable - func didYield(_ element: OutboundOut) { - if self.eventLoop.inEventLoop { - self.handler._didYield(element: element) - } else { - self.eventLoop.execute { - self.handler._didYield(element: element) - } - } - } - - @inlinable - func didTerminate(error: Error?) { - if self.eventLoop.inEventLoop { - self.handler._didTerminate(error: error) - } else { - self.eventLoop.execute { - self.handler._didTerminate(error: error) - } - } - } - } -} - -@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -@available(*, unavailable) -extension NIOAsyncChannelOutboundWriterHandler: Sendable {} diff --git a/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift b/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift index cbaec06640..4115b9dee8 100644 --- a/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift +++ b/Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift @@ -251,7 +251,7 @@ final class AsyncChannelTests: XCTestCase { do { let strongSentinel: Sentinel? = Sentinel() sentinel = strongSentinel! - try await XCTAsyncAssertNotNil(await channel.pipeline.handler(type: NIOAsyncChannelInboundStreamChannelHandler.self).get()) + try await XCTAsyncAssertNotNil(await channel.pipeline.handler(type: NIOAsyncChannelHandler.self).get()) try await channel.writeInbound(strongSentinel!) _ = try await channel.readInbound(as: Sentinel.self) } From 2798a6e92f0bbc2e830904132c76ba19435c5485 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Fri, 12 Jul 2024 10:41:40 +0200 Subject: [PATCH 14/17] [GHA] Download the scripts to make workflow reusable (#2785) # Motivation The reusable workflow is intended to be called from other repos; however, the workflow contains jobs that execute shell scripts. Those scripts are part of the NIO repo. When another repo calls the reusable workflow those scripts aren't present on the machine. # Modification This PR downloads the scripts via curl and executes them. # Result Makes the reusable workflow truly reusable --- .github/workflows/reusable_pull_request.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index f6ae37340f..5fe2d67475 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -122,7 +122,9 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - name: Run documentation check - run: ./scripts/check-docs.sh + run: | + apt-get -qq update && apt-get -qq -y install curl + curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-docs.sh | bash unacceptable-language-check: name: Unacceptable language check @@ -135,4 +137,4 @@ jobs: - name: Run unacceptable language check env: UNACCEPTABLE_WORD_LIST: ${{ inputs.unacceptable_language_check_word_list}} - run: ./scripts/check-unacceptable-language.sh \ No newline at end of file + run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-unacceptable-language.sh | bash \ No newline at end of file From 5e9a9ccc10d7636e00452171c58b1742b9d0338e Mon Sep 17 00:00:00 2001 From: Dimitri Bouniol Date: Fri, 12 Jul 2024 02:20:14 -0700 Subject: [PATCH 15/17] Improved documentation for HTTP Parts to clarify how often each part is received (#2775) --- Sources/NIOHTTP1/HTTPTypes.swift | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/Sources/NIOHTTP1/HTTPTypes.swift b/Sources/NIOHTTP1/HTTPTypes.swift index fe46d6074e..0237074137 100644 --- a/Sources/NIOHTTP1/HTTPTypes.swift +++ b/Sources/NIOHTTP1/HTTPTypes.swift @@ -125,15 +125,30 @@ public struct HTTPRequestHead: Equatable { extension HTTPRequestHead: @unchecked Sendable {} -/// The parts of a complete HTTP message, either request or response. -/// -/// A HTTP message is made up of a request or status line with several headers, -/// encoded by `.head`, zero or more body parts, and optionally some trailers. To -/// indicate that a complete HTTP message has been sent or received, we use `.end`, -/// which may also contain any trailers that make up the message. +/// The parts of a complete HTTP message, representing either a request or a response. +/// +/// An HTTP message is made up of: +/// - a request or status line with several headers, encoded by a single ``HTTPPart/head(_:)`` part, +/// - zero or more ``HTTPPart/body(_:)`` parts, +/// - and some optional trailers (represented as headers) in a single ``HTTPPart/end(_:)`` part. +/// +/// To indicate that a complete HTTP message has been sent or received, +/// an ``HTTPPart/end(_:)`` part must be used, even when no trailers are included. public enum HTTPPart { + /// The headers of an HTTP request or response. + /// + /// A single part is always used to encode all headers. case head(HeadT) + + /// A part of an HTTP request or response's body. + /// + /// Zero or more body parts can be sent or received. The stream is finished when + /// an ``HTTPPart/end(_:)`` part is received. case body(BodyT) + + /// The end of an HTTP request or response, optionally containing trailers. + /// + /// A single part is always used to encode all trailers. case end(HTTPHeaders?) } From 654bf41c03e99791f2e2c654e3974cf80aadeb0c Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Fri, 12 Jul 2024 12:07:06 +0200 Subject: [PATCH 16/17] [GHA] Add license header check (#2781) * [GHA] Add license header check # Motivation We need to make sure all code files have the appropriate license headers in place. This is currently part of the soundness check and we need to replace this with a GH action. # Modification Adds a new job to the reusable workflow the check all files for license headers. Since some files are ignored every repo can specify a `.licenseignore` file. # Result Last part of the soundness script migrated. * Review * Remove default excludes --- .github/workflows/pull_request.yml | 3 +- .github/workflows/reusable_pull_request.yml | 23 ++++++- .licenseignore | 47 +++++++++++++ Benchmarks/Package.swift | 13 ---- scripts/check-license-header.sh | 74 +++++++++++++++++++++ 5 files changed, 145 insertions(+), 15 deletions(-) create mode 100644 .licenseignore create mode 100755 scripts/check-license-header.sh diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index b2e62dbd26..6d19bb7b49 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -9,4 +9,5 @@ jobs: name: Checks uses: ./.github/workflows/reusable_pull_request.yml with: - benchmarks_linux_package_path: "Benchmarks" \ No newline at end of file + benchmarks_linux_package_path: "Benchmarks" + license_header_check_project_name: "SwiftNIO" diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index 5fe2d67475..b34e0c6c77 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -31,6 +31,14 @@ on: type: string description: "List of unacceptable words. Defaults to a sensible list of words." default: "blacklist whitelist slave master sane sanity insane insanity kill killed killing hang hung hanged hanging" #ignore-unacceptable-language + license_header_check_enabled: + type: boolean + description: "Boolean to enable the license header check job. Defaults to true." + default: true + license_header_check_project_name: + type: string + description: "Name of the project called out in the license header." + required: true ## We are cancelling previously triggered workflow runs concurrency: @@ -137,4 +145,17 @@ jobs: - name: Run unacceptable language check env: UNACCEPTABLE_WORD_LIST: ${{ inputs.unacceptable_language_check_word_list}} - run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-unacceptable-language.sh | bash \ No newline at end of file + run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-unacceptable-language.sh | bash + + license-header-check: + name: License headers check + if: ${{ inputs.license_header_check_enabled }} + runs-on: ubuntu-latest + timeout-minutes: 1 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run license header check + env: + PROJECT_NAME: ${{ inputs.license_header_check_project_name }} + run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-license-header.sh | bash diff --git a/.licenseignore b/.licenseignore new file mode 100644 index 0000000000..7e304d71fa --- /dev/null +++ b/.licenseignore @@ -0,0 +1,47 @@ +.gitignore +**/.gitignore +.licenseignore +.gitattributes +.mailfilter +.mailmap +.spi.yml +.swift-format +.github/* +*.md +*.txt +*.yml +*.yaml +*.json +Package.swift +**/Package.swift +Package@-*.swift +**/Package@-*.swift +Package.resolved +**/Package.resolved +Makefile +*.modulemap +**/*.modulemap +**/*.docc/* +*.xcprivacy +**/*.xcprivacy +*.symlink +**/*.symlink +Dockerfile +**/Dockerfile +Snippets/* +Sources/CNIOAtomics/src/cpp_magic.h +Sources/CNIOLLHTTP/LICENSE-MIT +Sources/CNIOLLHTTP/c_nio_api.c +Sources/CNIOLLHTTP/c_nio_http.c +Sources/CNIOLLHTTP/c_nio_llhttp.c +Sources/CNIOLLHTTP/include/c_nio_llhttp.h +Sources/CNIOSHA1/c_nio_sha1.c +Sources/CNIOSHA1/include/CNIOSHA1.h +dev/alloc-limits-from-test-output +dev/boxed-existentials.d +dev/git.commit.template +dev/lldb-smoker +dev/make-single-file-spm +dev/malloc-aggregation.d +dev/update-alloc-limits-to-last-completed-ci-build +scripts/nio-diagnose diff --git a/Benchmarks/Package.swift b/Benchmarks/Package.swift index 507159a7b1..6748b30a41 100644 --- a/Benchmarks/Package.swift +++ b/Benchmarks/Package.swift @@ -1,17 +1,4 @@ // swift-tools-version: 5.7 -//===----------------------------------------------------------------------===// -// -// This source file is part of the SwiftCertificates open source project -// -// Copyright (c) 2023 Apple Inc. and the SwiftCertificates project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// See CONTRIBUTORS.txt for the list of SwiftCertificates project authors -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// import PackageDescription diff --git a/scripts/check-license-header.sh b/scripts/check-license-header.sh new file mode 100755 index 0000000000..1522fc8eb6 --- /dev/null +++ b/scripts/check-license-header.sh @@ -0,0 +1,74 @@ +#!/bin/bash +##===----------------------------------------------------------------------===## +## +## 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 +## +##===----------------------------------------------------------------------===## +set -euo pipefail + +log() { printf -- "** %s\n" "$*" >&2; } +error() { printf -- "** ERROR: %s\n" "$*" >&2; } +fatal() { error "$@"; exit 1; } + +test -n "${PROJECT_NAME:-}" || fatal "PROJECT_NAME unset" + +expected_file_header_template="@@===----------------------------------------------------------------------===@@ +@@ +@@ This source file is part of the ${PROJECT_NAME} open source project +@@ +@@ Copyright (c) YEARS Apple Inc. and the ${PROJECT_NAME} project authors +@@ Licensed under Apache License v2.0 +@@ +@@ See LICENSE.txt for license information +@@ See CONTRIBUTORS.txt for the list of ${PROJECT_NAME} project authors +@@ +@@ SPDX-License-Identifier: Apache-2.0 +@@ +@@===----------------------------------------------------------------------===@@" + +paths_with_missing_license=( ) + +file_paths=$(git ls-files $(cat .licenseignore | xargs -I% printf ":(exclude)% ")) + +while IFS= read -r file_path; do + file_basename=$(basename -- "${file_path}") + file_extension="${file_basename##*.}" + + case "${file_extension}" in + swift) expected_file_header=$(sed -e 's|@@|//|g' <<<"${expected_file_header_template}") ;; + h) expected_file_header=$(sed -e 's|@@|//|g' <<<"${expected_file_header_template}") ;; + c) expected_file_header=$(sed -e 's|@@|//|g' <<<"${expected_file_header_template}") ;; + sh) expected_file_header=$(cat <(echo '#!/bin/bash') <(sed -e 's|@@|##|g' <<<"${expected_file_header_template}")) ;; + py) expected_file_header=$(cat <(echo '#!/usr/bin/env python3') <(sed -e 's|@@|##|g' <<<"${expected_file_header_template}")) ;; + rb) expected_file_header=$(cat <(echo '#!/usr/bin/env ruby') <(sed -e 's|@@|##|g' <<<"${expected_file_header_template}")) ;; + *) fatal "Unsupported file extension for file (exclude or update this script): ${file_path}" ;; + esac + expected_file_header_linecount=$(wc -l <<<"${expected_file_header}") + + file_header=$(head -n "${expected_file_header_linecount}" "${file_path}") + normalized_file_header=$( + echo "${file_header}" \ + | sed -e 's/20[12][0123456789]-20[12][0123456789]/YEARS/' -e 's/20[12][0123456789]/YEARS/' \ + ) + + if ! diff -u \ + --label "Expected header" <(echo "${expected_file_header}") \ + --label "${file_path}" <(echo "${normalized_file_header}") + then + paths_with_missing_license+=("${file_path} ") + fi +done <<< "$file_paths" + +if [ "${#paths_with_missing_license[@]}" -gt 0 ]; then + fatal "❌ Found missing license header in files: ${paths_with_missing_license[*]}." +fi + +log "✅ Found no files with missing license header." From eea0f74cb2f4d980d8829c8e1fa67785e3be6782 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Fri, 12 Jul 2024 13:58:37 +0200 Subject: [PATCH 17/17] [GHA] Broken symlink and format check (#2787) # Motivation The next two reusable checks that we need to create are for broken symlinks and formatting. The latter is disabled for this repo for now since we need to discuss the formatting rules separately. # Modification This PR adds two new checks - broken symlinks and formatting. # Result Closer to having everything on GitHub actions. --- .github/workflows/pull_request.yml | 1 + .github/workflows/reusable_pull_request.yml | 32 +++++++++++++++++++ scripts/check-broken-symlinks.sh | 34 +++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100755 scripts/check-broken-symlinks.sh diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 6d19bb7b49..23c4ce3b64 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -11,3 +11,4 @@ jobs: with: benchmarks_linux_package_path: "Benchmarks" license_header_check_project_name: "SwiftNIO" + format_check_enabled: false diff --git a/.github/workflows/reusable_pull_request.yml b/.github/workflows/reusable_pull_request.yml index b34e0c6c77..d1220140cd 100644 --- a/.github/workflows/reusable_pull_request.yml +++ b/.github/workflows/reusable_pull_request.yml @@ -39,6 +39,14 @@ on: type: string description: "Name of the project called out in the license header." required: true + broken_symlink_check_enabled: + type: boolean + description: "Boolean to enable the broken symlink check job. Defaults to true." + default: true + format_check_enabled: + type: boolean + description: "Boolean to enable the format check job. Defaults to true." + default: true ## We are cancelling previously triggered workflow runs concurrency: @@ -159,3 +167,27 @@ jobs: env: PROJECT_NAME: ${{ inputs.license_header_check_project_name }} run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-license-header.sh | bash + + broken-symlink-check: + name: Broken symlinks check + if: ${{ inputs.broken_symlink_check_enabled }} + runs-on: ubuntu-latest + timeout-minutes: 1 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run broken symlinks check + run: curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-broken-symlinks.sh | bash + + format-check: + name: Format check + if: ${{ inputs.format_check_enabled }} + runs-on: ubuntu-latest + container: + image: swiftlang/swift:nightly-6.0-jammy + timeout-minutes: 5 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run format check + run: swift format lint --parallel --recursive --strict diff --git a/scripts/check-broken-symlinks.sh b/scripts/check-broken-symlinks.sh new file mode 100755 index 0000000000..0c65d99df2 --- /dev/null +++ b/scripts/check-broken-symlinks.sh @@ -0,0 +1,34 @@ +#!/bin/bash +##===----------------------------------------------------------------------===## +## +## 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 +## +##===----------------------------------------------------------------------===## +set -euo pipefail + +log() { printf -- "** %s\n" "$*" >&2; } +error() { printf -- "** ERROR: %s\n" "$*" >&2; } +fatal() { error "$@"; exit 1; } + +log "Checking for broken symlinks..." +num_broken_symlinks=0 +while read -r -d '' file; do + if ! test -e "./${file}"; then + error "Broken symlink: ${file}" + ((num_broken_symlinks++)) + fi +done < <(git ls-files -z) + +if [ "${num_broken_symlinks}" -gt 0 ]; then + fatal "❌ Found ${num_broken_symlinks} symlinks." +fi + +log "✅ Found 0 symlinks."