### Motivation:
Channels based on `BaseSocketChannel` throw in both `getOption` and
`setOption` if the channel has been closed, since the `setsockopt` will
fail. However, the current behavior of `EmbeddedChannel` is options
remain writable and readable on closed channels.
There are situations where we'd like to be able to model the runtime
behaviour of the real channel in tests, e.g. to test this fix:
https://github.com/apple/swift-nio-extras/pull/304.
### Modifications:
- Add API to enable throwing in `EmbeddedChannel.getOption` and
`.setOption` if channel is closed.
- Add a test for this new behavior.
### Result:
- New API to enable throwing in `EmbeddedChannel.getOption` and
`.setOption` if channel is closed.
- No observable change for existing users of `EmbeddedChannel`.
### Motivation:
Currently, pending consumer closures remain in the
`{in}{out}boundBufferConsumer` queues of `EmbeddedChannelCore` even
after the channel closes. It is also possible to enqueue consumers
*after* the channel closes. In these cases, the consumer closures will
never be invoked and this can lead to unfavourable behaviour, as
observed in `NIOAsyncTestingChannel`'s `waitFor{In}{Out}boundWrite`
methods (the only places these queues are currently used).
`NIOAsyncTestingChannel`'s `waitFor{In}{Out}boundWrite` methods complete
a continuation *inside* the consumer closure. In the cases described
above, the continuation never completes and therefore
`waitFor{In}{Out}boundWrite` never returns.
### Modifications:
- Updated the element type in `EmbeddedChannelCore`'s
`{in}{out}boundBufferConsumer` from `(NIOAny) -> Void` to
`(Result<NIOAny, Error>) -> Void`.
- This is so that the `.failure` case can be used to notify the consumer
closure that the channel has closed.
- Changed the visibility of the `{in}{out}boundBufferConsumer`
properties from `internal` to `private` in order to prevent the queues
from being accessed and being appended to without the call site
considering whether the channel has been closed.
- Added new `internal` methods named
`enqueue{In}{Out}boundBufferConsumer(_:)` which take the consumer
closure as an argument and only append to the corresponding queue if the
channel isn't closed.
- If the channel is closed, the consumer closure is invoked immediately
with `.failure(ChannelError.ioOnClosedChannel)`.
- Updated `EmbeddedChannelCore`'s `close0` method to return a
`.failure(ChannelError.ioOnClosedChannel)` result to each closure in
`{in}{out}boundBufferConsumer` and empty both buffers.
- Updated `NIOAsyncTestingChannel`'s `waitFor{In}{Out}boundWrite` to
throw an error in the continuation upon receiving a `.failure` result.
- Added associated test cases.
### Result:
`EmbeddedChannelCore`'s `{in}{out}boundBufferConsumer` queues can be
used more safely: all pending closures will be invoked upon channel
close. As a result, `NIOAsyncTestingChannel`'s
`waitFor{In}{Out}boundWrite` no longer indefinitely blocks when the
channel closes.
### Motivation:
`NIOAsyncTestingChannel` stored its `localAddress` and `remoteAddress`
in a locked storage on itself for thread safety, however in doing so
left us open to bugs because a handler grabbing the addresses of the
context had no visibility of the values.
### Modifications:
Reach into `EmbeddedChannelCore` for the addresses instead of storing
them on the `NIOAsyncTestinghannel`. I also considered a delegate
approach where the `EmbeddedChannelCore` could offload the
responsibility for storing the values back to the
`NIOAsyncTestingChannel` but it was complicated and of questionable
value.
### Result:
* The correct address values are seen no matter how they are obtained.
* We probably take a performance hit locking the values in this way but
this is testing code so probably not the end of the world.
Support options on AsyncTestingChannel and EmbeddedChannel
Fixes#3305
### Motivation:
The channels are intended for testing purposes so while most options
have no practical use setting and getting them can be useful for
testing.
For example a traceroute implementation will set TTL to the current hop
number. Testing such an implementation requires the channels to pretend
to support the TTL option.
### Modifications:
Added option storage to AsyncTestingChannel and EmbedededChannel and
made `getOptionSync` and `setOptionSync` read and write from that
storage.
### Result:
EmbeddedChannel and AsyncTestingChannel support changing their options.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
Embedded channels should set local and remote address always
### Motivation:
Currently, if connect or bind are called without a promise, the
remote/local address does not get set because those were getting set in
the whenSuccess of the promise.
### Modifications:
If the user didn't provide a promise, create one ourselves, so we have
something to listen for.
### Result:
Calling connect/bind will always result in remote/local address getting
set, regardless of whether you pass a promise
Since nio 2.78, adding handlers to a pipeline requires the handlers to
be sendable.
That makes the
[NIOAsyncTestingChannel.init(handlers➿)](https://swiftpackageindex.com/apple/swift-nio/2.78.0/documentation/nioembedded/nioasynctestingchannel/init(handlers➿))
function cumbersome, because you cannot create handlers and then call
the function (unless your handlers are Sendable) even if you never use
the handlers elsewhere.
This PR adds a new initializer which takes a closure. The closure is run
on-loop before the channel is registered. This means we can do:
```swift
let channel = try await NIOAsyncTestingChannel {
let handler = MyUnsendableHandler()
try $0.pipeline.syncOperations.addHandler(handler)
}
```
…ledOrNotScheduledAnymore
# Motivation
This task was flaky and hit the assertion with the count being 1
# Modification
This changes the expectation to greater than 0 since we can always
guarantee that one scheduled task is run but due to timing windows it
might be that we shutdown the EL before the second task is enqueued
which meant it was immediately failed.
# Result
One less flaky test
Motivation:
NIOEmbedded is used all over NIO-land for testing various pieces of the
infrastructure, and so requires a substantial audit for strict
concurrency.
Modifications:
- Mark a few things Sendable.
- Fix the tests, which actually did have some nasty bugs
Result:
Sendable-clean NIOEmbedded
Motivation:
6.0.3 added new warnings about Sendable issues, so let's fix those to
keep things warnings free.
Modifications:
A bunch of Sendable usage fixes.
Result:
Warnings free builds again!
### Motivation
Code that tries to work with `NIODeadline` can be challenging to test
using `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` because they
have their own, fake clock.
These testing event loops do implement `scheduleTask(in:_)` to submit
work in the future, relative to the event loop clock, but there is no
way to get the event loop's current notion of "now", as a `NIODeadline`,
and users sometimes find that `NIODeadline.now`, which returns the time
of the real clock, to be surprising.
### Modifications
Add `EventLoop.now` to get the current time of the event loop clock.
### Result
New APIs to support writing code that's easier to test.
### Motivation:
Get this repo building again for Android with NDK 27
### Modifications:
- Update some networking declarations for newly added nullability
annotations
- Import the new Android overlay instead in some tests
- Add two force-unwraps on all platforms, that are needed for Android
### Result:
This repo and its tests build for Android again
I've been [using these patches on my Android
CI](https://github.com/finagolfin/swift-android-sdk/blob/main/swift-nio-ndk27.patch)
and natively on Android for a couple months now. I didn't bother keeping
this patch building for Android with Swift 5 anymore, as my Android CI
no longer tests Swift 5.
I built this pull and ran the tests on linux x86_64 to make sure there
was no regression.
### Motivation:
Opening the `swift-nio` repository made me warning blind because there
were always so many trivially fixable warnings about things that were
correct but cannot be understood by the compiler.
### Modifications:
Fix all the sendable warnings that popped up, except for one test where
`NIOLockedValueBox<Thread?>` isn't sendable because `Foundation.Thread`
seemingly isn't `Sendable` which is odd. Guessing that'll be fixed on
their end.
### Result:
- Fewer warnings
- Less warning-blindness
- More checks
Motivation:
`IOData` is a legacy but alas also core type that needs to be
`Sendable`. Before this PR however it can't be `Sendable` because it
holds a `FileRegion` which holds a `NIOFileDescriptor`. So let's make
all of these `Sendable` but let's also start the deprecation journey for
the following types:
- `IOData`, now soft-deprecated (no warnings) because on its reliance on
`FileRegion`
- `FileRegion`, now soft-deprecated (no warnings) because on its
reliance on `NIOFileHandle`
- `NIOFileHandle`, now soft-deprecated (warnings on the
`NIOFileHandle(descriptor:)` constructor but with a
`NIOFileHandle(_deprecatedTakingOwnershipOfDescriptor:)` alternative
- `NonBlockingFileIO`, now soft-deprecated (warnings on the `openFile`
functions (but with `_deprecated` alternatives) because of their
reliance on `NIOFileHandle)
Modification:
- Make `NIOFileDescriptor`, `FileRegion` and `IOData` `Sendable` by
tracking the fd number and the usage state in an atomic
- Enforce singular access by making the `withFileDescriptor { fd ... }`
function atomically exchange the fd number for a "I'm busy" sentinel
value
- Start deprecating `IOData`, `NIOFileHandle`, `NonBlockingFileIO`,
`FileRegion`
Result:
- `NIOFileDescriptor`, `FileRegion` and `IOData` can be `Sendable`
Motivation:
The ChannelInvoker protocols are an awkward beast. They aren't really
something that people can do generic programming against. Instead, they
were designed to do API sharing. Of course, they didn't do that very
well, and the strict concurrency checking world has revealed this.
Much of the API surface on ChannelInvoker is confused. There are
NIOAnys, which aren't Sendable. We allow sending user events without
requiring Sendable. And our two main conforming types are
ChannelPipeline and ChannelHandlerContext, two types with wildly
differing thread-safety semantics.
This PR aims to clean that up.
Modifications:
- Deprecated all API surface on ChannelInvoker protocols that uses
NIOAny.
ChannelInvoker has to be assumed to be a cross-thread protocol,
and that requires that it only use Sendable types. NIOAny isn't,
so these methods are no longer sound.
- Re-add non-deprecated versions on ChannelHandlerContext.
While it's not safe to use the NIOAny methods on Channel or
ChannelPipeline, it's totally safe to use them on
ChannelHandlerContext. So we keep those available and
undeprecated.
- Provide typed generic replacements on ChannelPipeline and on Channel
To replace the NIOAny methods on ChannelPipeline and Channel
we can use some typed generic ones instead. These are not
defined on ChannelInvoker, as the methods are useless on
ChannelHandlerContext. This begins the acknowledgement that
ChannelHandlerContext should not have conformed to these
protocols at all.
- Add Sendable constraints to the user event witnesses on ChannelInvoker
Again, these were missing, but must be there for Channel and
ChannelPipeline.
- Provide non-Sendable overloads on ChannelHandlerContext
ChannelHandlerContext is thread-bound, and so may safely pass
non-Sendable user events.
Result:
One step closer to strict concurrency cleanliness for NIOCore.
Add ability to get the amount of buffered outbound data from `Channel`
### Motivation:
Right now, SwiftNIO does not have the API to answer the question "how
much data is buffered in the Channel". Applications focusing on
performance may need to fine-tune the amount of outbound data that will
be sent to optimize data throughput, adjust sending rate to avoid
overflow, and potentially reduce latency.
SwiftNIO currently provides some backpressure mechanism. This new API
will be a good addition. By knowing how much data is buffered directly,
applications can make informed decision to adjust for optimal buffer
sizes and send rates.
### Modifications:
- Expose current buffer size through ChannelOptions so that users can
read the value out. StreamSocketChannel, DatagramSocketChannel,
EmbeddedChannel, and AsyncTestingChannel have the same API interface.
- Various modifications to the existing tests to make sure the new API
is working correctly.
- Add a new `so_sndbuf` socket option so that users can easily adjust
the send buffer size.
### Result:
Users can get the amount of outbound bytes currently buffered in the
`Channel` through the new `BufferedWritableBytesOption` channel option.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
### Motivation:
We're seeing some very rare failures from this test in CI runs but I
cannot reproduce locally. The test relies on timing to some degree and
the interaction of scheduling tasks at shutdown.
### Modifications:
The `AsyncTestingEventLoop` has an `executeInContext` function which
puts the closure on the backing dispatch queue and blocks. In this
instance it might be a good way to make sure that all the previous work
has happened.
### Result:
(Hopefully) less flakey test.
Motivation:
NIOAsyncTestingEventLoopTests.testTasksScheduledDuringShutdownAreAutomaticallyCancelled
is flaky.
The recursivesly schedules tasks to run on the event loop and then,
after a small pause, shuts down the event loop. It then asserts that
more then 1 task was scheduled (i.e. at least 1 recursive task was run).
This assertion occasionally fails as exactly 1 task was run.
I haven't been able to reproduce this locally but I believe the root of
the flakiness is that the child tasks to shutdown the loop and advance
the time (to trigger the recursive scheduling) can race. If the shutdown
wins then only the root task will run.
Modifications:
- Remove the array of scheduled tasks, it wasn't being used
- Fix atomic ordering
- Increase sleep time before shutting down
- Advance time in the task group rather than a child task
Result:
Less flaky test (hopefully)
Motivation:
Since Swift 5.5 and [SE-0299](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0299-extend-generic-static-member-lookup.md) it is possible to add static members to protocols which are discoverable through the shorthand dot syntax.
This change reduces type repetition and improves call-site legibility.
Modifications:
Added extensions for ChannelOption with static members where Self is bound to a concrete type.
Result:
ChannelOption types can be used with the leading dot syntax. For eg:
```
//before
.channelOption(ChannelOptions.explicitCongestionNotification, value: true)
.channelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
//after
.channelOption(.explicitCongestionNotification, value: true)
.channelOption(.socketOption(.so_reuseaddr), value: 1)
```
### Motivation
Some tests were making use of `Task.sleep(for:)` which isn't present on older platforms.
```
/workspace/Tests/NIOEmbeddedTests/AsyncTestingEventLoopTests.swift:505:32: error: 'sleep(for:tolerance:clock:)' is only available in tvOS 16.0 or newer
try await Task.sleep(for: .milliseconds(1))
```
### Modifications
Update tests to use `Task.sleep(nanoseconds:)` via `TimeAmount.nanoseconds` instead, which is available on those platforms.
### Result
Can build tests on older platforms.
### Motivation:
The two testing event loops—`EmbeddedEventLoop` and `NIOAsyncTestingEventLoop`—have different semantics for outstanding work during shutdown, which are both different from the production `SelectableEventLoop`, specifically with newly scheduled tasks that result from running existing scheduled work at the time of shutdown.
There are three axes to consider:
1. Scheduled tasks that are at or past their deadline.
2. Scheduled tasks with a deadline in the future.
3. Newly scheduled work that result from either running or cancelling (1) and (2).
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Run | Run resulting (1) and resulting (2) |
Note that `NIOAsyncTestingEventLoop` may never terminate because of this.
### Modifications:
This PR aligns `EmbeddedEventLoop` and `NIOTestingEventLoop` and makes them more similar to `SelectableEventLoop` and less surprising semantics.
### Result:
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
### Future work:
Given both the `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` are used in tests of NIO applications that run with `SelectableEventLoop` in production, it might be worth extending both to also support quiescing to give a more representative shutdown behaviour in tests.
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
# Motivation
We are currently missing a bunch of annotations in our tests which leads to compilation failures if you build against generic iOS/macOS/watchOS/tvOS
# Modification
This PR adds the missing availability annotations in the tests.
# Motivation
We introduced `XCTAsyncTest` to bridge the gap where some Swift versions did not have `async` test support.
# Modification
This PR removes `XCTAsyncTest` and migrates all the places where we used it to native `async` tests.
# Result
Less custom code.
* Embedded: getOption(.allowRemoteHalfClosure) -> OK
Motivation:
In `swift-nio-ssl`, I am currently working on allowing half-closures
which relies on querying the underlying channel if
`ChannelOptions.Types.AllowRemoteHalfClosureOption` is enabled. As a lot of
`swift-nio-ssl`'s tests rely on `EmbeddedChannel` and it did not support
this option, a lot of the tests failed.
Modifications:
* add a `public var allowRemoteHalfClosure` to `EmbeddedChannel`
* enable setting/getting
`ChannelOptions.Types.AllowRemoteHalfClosureOption` in
`EmbeddedChannel` (only modifies the `allowRemoteHalfClosure` variable
* add test for new behaviour
* AsyncTestingChannel: getOption(.allowRemoteHalfClosure) -> OK
Motivation:
`AsyncTestingChannel` interface should be in step with `EmbeddedChannel`
interface. Therefore also add support for the
`AllowRemoteHalfClosureOption`
Modifications:
* add a `public var allowRemoteHalfClosure` to `AsyncTestingChannel`
* enable setting/getting
`ChannelOptions.Types.AllowRemoteHalfClosureOption` in `AsyncTestingChannel`
(only modifies the `allowRemoteHalfClosure` variable
* add tests for new behaviour
* Synchronize access to allowRemoteHalfClosure
Modifications:
* add `ManagedAtomic` property `_allowRemoteHalfClosure` to
`EmbeddedChannelCore`
* make sure that access to `allowRemoteHalfClosure` from
`AsyncTestingChannel` and `EmbeddedChannel` is synchronized by
accessing underlying atomic value in `channelcore`
* Update allocation limits
Motivation:
swift- nio was failing builds that should pass
Modifications:
Adding available to the necessary sections
* Updating test OnToRunClosure
Motivation:
testCancelledScheduledTasksDoNotHoldOnToRunClosure() was not allowed enough time and timing off at moments, causing it to fail occasionally
Modifications:
Added a ConditionLock throughout the code to make sure it only unlocks when the code has waited enough time for it to not hit the precondition failure
# Motivation
`EmbeddedChannel` is often used in testing and currently any code under testing that uses `context.localAddress` cannot be mocked, since `EmbeddedChannelCore` is always throwing.
# Modification
Use the same values for `localAddress` in `localAddress0()`. Same for `remoteAddress`
# Result
We can now properly test code that needs local/remote addresses with `EmbeddedChannel`
Motivation:
#fileID introduced in Swift 5.3, so no longer need to use #file anywhere
Modifications:
Changed #file to #filePath or #fileID depending on the situation
Motivation
Testing versions of NIO code that involve interfacing with Swift
Concurrency is currently a difficult business. In particular,
EmbeddedChannel is not available in Swift concurrency, making it
difficult to write tests where you fully control the I/O.
To that end, we should provide a variation of EmbeddedChannel that makes
testing these things possible.
Modifications
Provide an implementation of NIOAsyncTestingChannel.
Results
Users can write tests confidently with async/await.
* Enhance and rename AsyncEmbeddedEventLoop
Motivation
AsyncEmbeddedEventLoop is an important part of our ongoing testing story
for NIO. However, it suffers from two problems.
The first is a usability one. As we discovered during the original
implementation, following EmbeddedEventLoop's pattern of not having any
EventLoop "Tasks" execute until run() meant that simple constructs like
`EventLoopFuture.wait` and `EventLoopFuture.get` didn't work at all,
forcing us to add an annoying `awaitFuture` method.
When playing with implementing EmbeddedChannel, this got worse, as I/O
methods also don't run immediately if called from the testing thread. We
couldn't easily work around this issue, and it meant that common
patterns (like calling `channel.writeAndFlush`) would deadlock!
This is unacceptable, so a change had to be made.
While we're here, we received feedback that the name is unclear to
users. Given that this particular event loop is in no sense "embedded",
we no longer need the name, so we can take this opportunity to use a
better one.
Modifications
Changed `func execute` to immediately execute its task body, and to
dequeue all pending tasks at this time. Essentially, it's the equivalent
to run(). This is a major change in its behaviour.
Renamed the loop to `NIOAsyncTestingEventLoop`.
Result
Better names, easier to use.
* Make soundness happy
* Remove awaitFuture
Motivation
The rise of Swift concurrency has meant that a number of our APIs need
to be recontextualised as async/await capable. While generally this is a
straightforward task, any time those APIs were tested using
EmbeddedChannel we have a testing issue. Swift Concurrency requires the
use of its own cooperative thread pool, which is completely incapable of
safely interoperating with EmbeddedChannel and EmbeddedEventLoop. This
is becuase those two types "embed" into the current thread and are not
thread-safe, but our concurrency-focused APIs want to enable users to
use them from any Task.
To that end we need to develop new types that serve the needs of
EmbeddedChannel and EmbeddedEventLoop (control over I/O and task
scheduling) while remaining fully thread-safe. This is the first of a
series of patches that adds this functionality, starting with the
AsyncEmbeddedEventLoop.
Modifications
- Define AsyncEmbeddedEventLoop
Result
A required building block for AsyncEmbeddedChannel exists.
Co-authored-by: Franz Busch <privat@franz-busch.de>
### Motivation:
In my previous PR https://github.com/apple/swift-nio/pull/2010, I was able to decrease the allocations for both `scheduleTask` and `execute` by 1 already. Gladly, there are no more allocations left to remove from `execute` now; however, `scheduleTask` still provides a couple of allocations that we can try to get rid of.
### Modifications:
This PR removes two allocations inside `Scheduled` where we were using the passed in `EventLoopPromise` to call the `cancellationTask` once the `EventLoopFuture` of the promise fails. This requires two allocations inside `whenFailure` and inside `_whenComplete`. However, since we are passing the `cancellationTask` to `Scheduled` anyhow and `Scheduled` is also the one that is failing the promise from the `cancel()` method. We can just go ahead and store the `cancellationTask` inside `Scheduled` and call it from the `cancel()` method directly instead of going through the future.
Importantly, here is that the `cancellationTask` is not allowed to retain the `ScheduledTask.task` otherwise we would change the semantics and retain the `ScheduledTask.task` longer than necessary. My previous PR https://github.com/apple/swift-nio/pull/2010, already implemented the work to get rid of the retain from the `cancellationTask` closure. So we are good to go ahead and store the `cancellationTask` inside `Scheduled` now
### Result:
`scheduleTask` requires two fewer allocations
Motivation:
EmbeddedChannel is an important testing tool, and we want to use it
without needing to bring along the POSIX layer. They are not tightly
coupled. However, it also doesn't belong naturally in NIOCore, so we
should probably put it in its own place.
Modifications:
- Moved EmbeddedChannel and EmbeddedEventLoop to NIOEmbedded.
- Moved the tests to NIOEmbeddedTests
- Duplicated some test helpers
Result:
Easy to use EmbeddedChannel without the POSIX layer.
Si Beaumont
aryan-25
Rick Newton-Rogers
finagolfin
Klaas Pieter AnnemaandCory Benfield
hamzahrmalik
Franz Busch
Johannes Weiss
John ZhouandCory Benfield
George Barnett
Ayush Garg
Felix Schlegel
David Nadoba
carolinacass
YR ChenandCory Benfield
BenedictSt