Fixes all warnings when `-require-explicit-sendable` flag is enabled and
enables the flag on macOS CI.
### Motivation:
We want to ensure our public API is either explicitly marked as
`Sendable` or not.
### Modifications:
Marked appropriate public types as `Sendable`, or explicitly defined
their conformance to the `Sendable` protocol as unavailable.
### Result:
We can now enable `-require-explicit-sendable` compiler flag in our
codebase.
Motivation:
There's a known compiler issue on the 5.10 toolchain for x86 on Ubuntu
Noble. This issue causes the compile to fail, even though the code is
fine. We shouldn't allow that.
Modifications:
Shuffle the whitespace around to fix the issue.
Suppress the linter issues.
Result:
Compiler is back. Resolves#3223.
Improve usability of NIOAsyncChannel.executeThenClose function from an
Actor
### Motivation:
When calling executeThenClose from inside an actor, the closure which we
pass is not isolated to the same actor
This makes it hard to use
e.g. the following code won’t compile on Swift 6 because the closure is
`self-isolated` so can’t be passed
```
await withDiscardingTaskGroup { group in
do {
try await serverChannel.executeThenClose { inboundStream, _ in
try await self.handleInboundStream(inboundStream: inboundStream, group: &group)
}
} catch {}
}
```
Making the closure as @Sendable allows it to be passed, but then we
can’t access `group` anymore
### Modifications:
Added actor isolation parameter to the `executeThenClose` function.
### Result:
`executeThenClose` will be able to be asynchronously called from inside
an Actor, as it will now be part of the Actor's isolation domain.
---------
Co-authored-by: Franz Busch <privat@franz-busch.de>
After the changes introduced in
https://github.com/apple/swift-nio-http2/pull/487, we need to make a
small change in the implementation of `NIOAsyncChannel` to wait on the
`closeFuture` instead of on `close`'s promise in the `executeThenClose`
implementation.
### Motivation:
`executeThenClose` shouldn't fail from errors arising from closing the
channel - at this point, the user of the channel cannot really do
anything, and since the channel has been closed, we should not fail
since resources have been cleaned up anyways.
### Modifications:
This PR changes the implementation of `NIOAsyncChannel` to wait on the
`closeFuture` instead of on `close`'s promise in the `executeThenClose`
implementation.
It also updates the docs for `closeFuture` to better explain when it
will be succeeded and why it won't ever be failed.
### Result:
`executeThenClose` won't throw errors upon closing.
Motivation:
Users writing NIO code in a strict concurrency world often need to
interact with futures, promises, and event loops. The main interface to
these has strict sendability requirements, as it is possible the user is
doing so from outside the EventLoop that provides the isolation domain
for these types.
However, in many cases the user knows that they are on the isolation
domain in question. In that case, they need more capabilities. While
they can achieve their goals with NIOLoopBound today, it'd be nice if
they had a better option.
Modifications:
- Make EventLoop.Isolated public.
- Make EventLoopFuture.Isolated public.
- Make EventLoopPromise.Isolated public.
- Make all their relevant methods public.
- Move the runtime isolation check from the point of use to the point of
construction.
- Make the types non-Sendable to ensure that isolation check is
sufficient.
- Add unsafeUnchecked options to create these types when performance
matters and correctness is clear.
- Add tests for their behaviour.
- Update the documentation.
Result:
Writing safe code with promises, futures, and event loops is easier.
* 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:
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
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
# 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
# Motivation
Currently, when the channel closes and a user tries to write something the writer throws an `AlreadyFinished` error. This error can also be thrown when calling `finish` on the writer and then trying to call `write` again. This makes it hard to distinguish if the thrown error was due to the channel being closed or due to a business logic error in handling the writer.
# Modification
This PR finishes the writer with a `ChannelError.ioOnClosedChannel` if the writer gets finished to due a channel inactive or handler removed.
# Result
Users can now distinguish if they did something wrong with the writer or if the channel closed.
Motivation:
NIOAsyncChannel requires users to explicitly close it, this is typically
done by calling `executeThenClose`. If a `NIOAsyncChannel` isn't closed
then its outbound writer will hit a precondition failure on `deinit`.
Not calling `executeThenClose` is a programmer error.
However there are some sharp edges: if NIO never returns the
`NIOAsyncChannel` to the caller (e.g. if a connect attempt fails) then
nothing will finish the writer and precondition will fail in the deinit.
Working around this from a user perspective is non-obvious and requires
keep tracking of all `NIOAsyncChannel`s created from a connect attempt
and closing the unused ones.
We still want to maintain the precondition when users don't close the
channel, one way of achieving this is by defining a point in time at
which NIO hands responsibility of the channel to the user.
Modifications:
- Retain the writer in the outbound writer handler until channel active
- On successful connect attempts, the channel becomes active and the
connected channel is returned to the caller.
- On failed attempts channel active isn't called so the writer is
retained until the handler is removed from the pipeline at which point it is
finished.
Result:
Failed connect attempts don't result in precondition failures when using
NIOAsyncChannel.
* Introduce `assumeIsolated()` methods on `EventLoop`, `EventLoopPromise` and `EventLoopFuture`
> All methods/types are currently `internal` so we don't have to bikeshed just yet but we can move forward to get `NIOCore` warning free under strict concurrency
# Motivation
Methods on the above types are often called from the same event loop; however, we cannot prove to the compiler that this is true so we had to mark many methods on those types with `@Sendable` or require the generic type to be `Sendable`. This leads to unnecessary usage of `NIOLoopBound` when instead we should just dynamically assert that we are on the event loop. @dnadoba opened a very similar PR https://github.com/apple/swift-nio/pull/2228.
# Modification
This PR provides a method called `assumeIsolated()` on the three types that returns a type which re-declaration of all methods of the wrapped typed that have `Sendable` annotations. This new type is asserting at runtime that we are on the right event loop; hence, we don't need a `Sendable` value.
# Result
This PR makes it easier for our adopters to avoid newly introduced `Sendable` warnings
* Review
* Add `closeOnDeinit` to the `NIOAsyncChannel` init
# Motivation
In my previous PR, I already did the work to add `finishOnDeinit` configuration to the `NIOAsyncWriter` and `NIOAsyncSequenceProducer`. This PR also automatically migrated the `NIOAsyncChanell` to set the `finishOnDeinit = false`. This was intentional since we really want users to not use the deinit based cleanup; however, it also broke all current adopters of this API semantically and they might now run into the preconditions.
# Modification
This PR reverts the change in `NIOAsyncChannel` and does the usual deprecate + new init dance to provide users to configure this behaviour while still nudging them to check that this is really what they want.
# Result
Easier migration without semantically breaking current adopters of `NIOAsyncChannel`.
* Rename to `wrappingChannelSynchronously`
* Add `withInboundOutboud` to `NIOAsyncChannel` and deprecate deinit based cleanup
# Motivation
We just released our new async NIO APIs and have already gotten quite a bunch of feedback from adopters. One of the feedback was that the deinit based closing that we have added to the `NIOAsyncChannel` has caused problems since it leads to unexpected closure of their `Channel`. Furthermore, it makes it impossible to determine how many open sockets a program has at any given time since deinit based clean up relies on the optimizer and can happen at random times.
# Modifications
This PR adds new inits to `NIOAsyncSequenceProducer` and `NIOAsyncWriter` which disable the `deinit` based clean up and instead replace them with an assertion. This allows developers to still catch these issues at debug time. Furthermore, I added a new `withInboundOutbound` scoped access to `NIOAsyncChannel` which will close the channel at the end of the scope. This still gives users a nice API while not having to care much about closing themselves.
# Result
We are no longer using deinit based clean up and bring back one of the core principles of NIO which is deterministic resource usage.
* Review comments
* Internal labels for closure arguments
* Rename to `executeThenCloseChannel`
* Actually call `sinkDeinitialized`
* Change preconditions
* Move logic to deinits
* Rename to `executeThenClose` and review nits
# Motivation
We were setting `self.sink = nil` in the `NIOAsyncChannelOutboundWriterHandler` twice in the same call stack which is an exclusivity violation. This happens because the first `self.sink = nil` triggers the `didTerminate` delegate call which again triggered `self.sink = nil`.
# Modification
This PR changes the code to only call `self.sink?.finish()` and only sets the `sink` to `nil` in the `didTerminate` implementation. This follows what we do for the inbound handler implementation. I also added a test that triggers this exclusivity violation.
# Result
No more exclusivity violations in our code.
* Add support for unidirectional `NIOPipeBootstrap`
# Motivation
In some scenarios, it is useful to only have either an input or output side for a `PipeChannel`. This fixes https://github.com/apple/swift-nio/issues/2444.
# Modification
This PR adds new methods to `NIOPipeBootstrap` that make either the input or the output optional. Furthermore, I am intentionally breaking the API for the new async methods since those haven't shipped yet to reflect the same API there.
# Result
It is now possible to bootstrap a `PipeChannel` with either the input or output side closed.
* Docs and naming
# Motivation
Over the past months, we have been working on new async bridges to make using NIO's `Channel` from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.
# Modification
This PR removes the SPI from the `NIOAsyncChannel`, the bootstrap methods, protocol negotiation and HTTP upgrade.
# Result
Everyone can use the our new APIs🚀
* Call `NIOAsyncWriterSinkDelegate` outside of the lock
# Motivation
The current `NIOAsyncWriter` implementation expects that the delegate is called while holding the lock to avoid reentrancy issues. However, this prevents us from executing the delegate calls directly on the `EventLoop` if we are on it already.
# Modification
This moves all of the delegate calls outside of the locks and adds protection against reentrancy into the state machine.
# Result
Less allocations.
Clarify the reentrancy problems in docs and protect against them in the writer
* Code review
* Implement fast paths for `AsyncChannelInboundStreamChannelHandler`
* Add `_TinyArray` from `swift-certificates`
* Implement single element customization point in `NIOAsyncChannelOutboundWriterHandler`
* Call the single element optimization more often and store suspended producers in `_TinyArray`.
* Update thresholds
* Fix compiler warning
# Motivation
To reduce allocations when wrapping a channel into a `NIOAsyncChannel` we should make the `NIOAsyncChannel` a struct instead of a class.
# Modification
This PR changes the `NIOAsyncChannel` to a struct.
# Motivation
We spelled back pressure a few different ways throughout our async interfaces.
# Modification
This PR aligns to `backPressure` everywhere and uses `back pressure` in comments.
* Bump minimum Swift version to 5.7
Motivation:
Now that Swift 5.9 is GM we should update the supported versions and
remove 5.6
Modifications:
* Update `Package.swift`
* Remove `#if swift(>=5.7)` guards
* Delete the 5.6 docker compose file and make a 5.10 one
* Update integration test script
* Update docs
Result:
Remove support for Swift 5.6, add 5.10
* fix indentation issues
* 5.9 docker image use release image
# Motivation
We recently provided testing utilities for the `NIOAsyncChannelInboundStream`. On thing that was missing is a way to finish the stream with an error.
# Modification
This PR provides a `finish()` method that takes an error which is thrown from the inbound stream.
# Result
Better way to test code relying on the AsyncChannel work.
# Motivation
When using the `NIOAsyncChannel` to build higher level abstractions it is hard to test right now since one cannot drive the `NIOAsyncChannelInputStream` nor the `NIOAsyncChannelOutputWriter`.
# Modification
This PR adds new factory methods to both of those types which allow to either drive the input stream or record the outputs written to the output writer.
# Result
One can now successfully test their code written on top of the input stream and output writer.
# Motivation
When adding new async methods for all the bootstrap we had to create 3 sets of method for every bootstrap. This resulted in lots of code and only provided little benefit for users.
# Modification
This PR removes all bind/connect methods except the most generic ones. This leaves it up to the user to wrap their channels in a `NIOAsyncChannel` or to await the protocol negotiation result.
# Result
Less code to maintain on our side.
# Motivation
While adopting the new `NIOAsyncChannel` type we saw an exploding number of parameters passed to the various connect/bind/configure methods. All these methods had in common that we had to pass configuration for the `NIOAsyncChannel`.
# Modification
This PR introduces a new type `NIOAsyncChannel.Configuration` which groups all four configuration parameters into a struct.
# Result
We can now write more concise methods that use a single configuration object.
* Align ServerBootstrap bind methods with the initializer style
# Motivation
After experimenting with the bind/connect APIs of the other bootstraps, we realized that passing the type of the handler is not working very well and choose to add an initializer closure to all the bind/connect methods which returns a concrete result.
# Modification
This PR aligns the `ServerBootstrap` APIs with the new initializer based approach. To make this work I had to refactor the `NIOAsyncChannel` SPIs a bit. Overall, this is a net reduction in code.
# Result
We now have aligned bootstrap APIs.
* Code review
* Fix CI
* Add `AsyncChannel` based `ServerBootstrap.bind()` methods
# Motivation
In my previous PR, we added a new async bridge from a NIO `Channel` to Swift Concurrency primitives in the from of the `NIOAsyncChannel`. This type alone is already helpful in bridging `Channel`s to Concurrency; however, it is hard to use since it requires to wrap the `Channel` at the right time otherwise we will drop reads. Furthermore, in the case of protocol negotiation this becomes even trickier since we need to wait until it finishes and then wrap the `Channel`.
# Modification
This PR introduces a few things:
1. New methods on the `ServerBootstrap` which allow the creation of `NIOAsyncChannel` based channels. This can be used in all cases where no protocol negotiation is involved.
2. A new protocol and type called `NIOProtocolNegotiationHandler` and `NIOProtocolNegotiationResult` which is used to identify channel handlers that are doing protocol negotiation.
3. New methods on the `ServerBootstrap` that are aware of protocol negotiation.
# Result
We can now easily and safely create new `AsyncChannel`s from the `ServerBootstrap`
* Code review
* Fix typo
* Fix up tests
* Stop finishing the writer when an error is caught
* Code review
* Fix up writer tests
* Introduce shared protocol negotiation handler state machine
* Correctly handle multi threaded event loops
* Adapt test to assert the channel was closed correctly.
* Code review
* Land `NIOAsyncChannel` as SPI
# Motivation
We want to provide bridges from NIO `Channel`s to Swift Concurrency. In previous PRs, we already landed the building blocks namely `NIOAsyncSequenceProducer` and `NIOAsyncWriter`. These two types are highly performant bridges between synchronous and asynchronous code that respect back-pressure.
The next step is to build convenience methods that wrap a `Channel` with these two types.
# Modification
This PR adds a new type called `NIOAsyncChannel` that is capable of wrapping a `Channel`. This is done by adding two handlers to the channel pipeline that are bridging to the `NIOAsyncSequenceProducer` and `NIOAsyncWriter`.
The new `NIOAsyncChannel` type exposes three properties. The underlying `Channel`, a `NIOAsyncChannelInboundStream` and a `NIOAsyncChannelOutboundWriter`. Using these three types the user a able to read/write into the channel using `async` methods.
Importantly, we are landing all of this behind the `@_spi(AsyncChannel`. This allows us to merge PRs while we are still working on the remaining parts such as protocol negotiation.
# Result
We have the first part necessary for our async bridges. Follow up PRs will include the following things:
1. Bootstrap support
2. Protocol negotiation support
3. Example with documentation
* Add AsyncSequence bridge to NIOAsyncChannelOutboundWriter
* Code review
* Prefix temporary spi public method
* Rename writeAndFlush to write
aryan-25
Fabian FettandCory Benfield
Rafael Cepeda
Gus Cairo
Franz Busch
Johannes Weiss
George Barnett
Rick Newton-Rogers
David Nadoba