### 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.
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
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:
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:
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:
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.
* 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
`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:
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
finagolfin
Klaas Pieter AnnemaandCory Benfield
hamzahrmalik
Johannes Weiss
John ZhouandCory Benfield
Ayush Garg
Franz Busch
Felix Schlegel
carolinacass
BenedictSt