* 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
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
`EmbeddedChannel` and `EmbeddedEventLoop` should not be `Sendable`. However, they inherit from `Channel` and `EventLoop` respectively. Therfore, they need to be `Sendable` (and are already implcitily `Sendable`. Given these constraints, we can’t mark `EmbeddedChannel` and `EmbeddedEventLoop` as non-Sendable.
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:
In my previous PR https://github.com/apple/swift-nio/pull/2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially.
### Modifications:
This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute`
### Result:
`scheduleTask` and `execute` require one less allocation
Motivation:
For NIO's 'promise leak detector' we added file:line: labels to
makePromise and there it makes sense. A user might create a promise and
then never fulfil it, bad. With the file:line: arguments we can give
good diagnostics.
However, we (probably that @weissi again) also added it to flatMap and
friends where it doesn't make sense at all.
Sure, EventLoopFuture's implementation may create a promise in the
implementation of flatMap but this promise is never leaked unless the
previous future is never fulfilled (or NIO has a terrible bug). Suffice
to say that in a future chain, it's never a flatMap etc which is
responsible for leaking the first promise...
Explain here the context, and why you're making that change.
What is the problem you're trying to solve.
Modifications:
Remove all unnecessary `file:line:` parameters whilst keeping the public
API intact.
Result:
More sensible code.
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.
Felix Schlegel