Motivation:
We're centralizing on the Swift code of conduct, so we'll x-reference
that instead of holding our own.
Modifications:
Hyperlink out to Swift.
Result:
Shared CoC across the projects.
* Add NIOAsyncChannel based connect methods to ClientBootstrap
# Motivation
In my previous PR, I added new `bind` methods to `ServerBootstrap` that vend `NIOAsyncChannel` or support an async protocol negotiation. This PR focuses on adding new `connect` methods to `ClientBootstrap` which offer the same functionality.
# Modification
This PR adds new `connect` methods that either vend a `NIOAsyncChannel` or an asynchronous protocol negotiation result. To make this work I had to change the `HappyEyeballs` resolver so that it can return a generic value on resolving. Lastly, I adapted the bootstrap tests to use the new `ClientBootstrap` capabilities which now demonstrate a client/server protocol negotiation dance.
# Result
We can now bootstrap TCP clients with `NIOAsyncChannel`s
* Reduce code duplication
* Create a new set of APIs to tunnel an arbitrary Sendable payload through the inits
* Pass EL to closure
* Fix documentation
Motivation:
The HappyEyeballs connector synchronises state on an event loop but
calls out to a 'Resolver' to do DNS lookups. The resolver returns
results as a future which may be on a different loop than the connector.
The connector does not hop back to its own event loop before processing
the results.
For client bootstraps, if no resolver is specified then the default
resolver uses the same event loop as the connector so in many cases this
is not an issue. However, if a custom resolver is used this guarantee is
lost and data races are much more likely.
Modifications:
- Hop back to the connector's event loop after calling the resolver.
- Add a test.
Result:
Fewer data races.
* Fix flaky test in NIOAsyncWriter
# Motivation
We had flaky test in the writer where we were relying on some timing.
# Modification
Change the test so that it is deterministic.
# Result
Fixes https://github.com/apple/swift-nio/issues/2427
* Update allocation counters
* Add narrative documentation for NIO's concurrency bridges
# Motivation
We started to add bridges from NIO to Swift Concurrency and we should provide some narrative documentation for our users how to adopt them.
# Modification
Adds a new article that explains the NIO concurrency bridges.
* Address smaller review comments
* Rearrange sections and introduce general guidance section
* remove some commas
* Add missing catch
* 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:
Running the script failed as it looked for a CI build for Swift 5.5,
which is not run by our CI anymore.
Modifications:
* don't look for CI builds for Swift 5.5
* additionally, also check for CI build for Swift 5.9
Motivation
We shouldn't crash on somewhat likely user error.
Modifications
Pass on writes after close(mode: .output) instead of crashing.
Result
User code is more robust to weird edge cases.
* added assertSuccess() and assertFailure()
* test functions for assertSuccess() and assertFailure()
* removed callbacks approach and add precondition variants
* added test for precondition variants
* self.always to handle future w/o creating a promise, added fileID and line for debug
* updated test for asserts on EventLoopFuture
Motivation:
Currently the server pipeline handler ignores close. This generally works,
except in the rare case that the user calls close(mode: .output). In this
instance they have signalled that they'll never write again, and they're
likely expecting a final close shortly after.
However, it is possible that the pipeline handler has suspended reads
at the same time. On Linux this isn't an issue, because we'll still be told
about the eventual socket close. However, on Apple platforms we won't: we've
masked off the reads, and we can't listen to EVFILT_EXCEPT due to some
other issues. This means that on Apple platforms the server pipeline handler
can accidentally wedge the Channel open and prevent it from closing.
We should take this opportunity to have the server pipeline handler be smart
about close(mode: .output). What _should_ happen here is that the pipeline
handler should immediately refuse to deliver further requests on the Channel.
If one is in-flight, it can continue, but everything else should be dropped.
This is because the server cannot possibly respond to further requests.
Modifications:
- Add new states to the server pipeline handler
- Drop buffered requests and new data after close(mode: .output)
- Add tests
Result:
Server pipeline handler behaves way better.
Motivation:
Follow up to #2412 which changed the async channel bootstrap tests to
bind to port 0 instead of a fixed port. A few instances were missed,
this fixes up the remainder.
Modifications:
- Bind to port 0
Result:
Tests should run in parallel without failures.
# Motivation
The `AsyncChannelBootstrapTests` were failing when running parallel since they were all using the same port.
# Modification
This PR uses port `0` to let the system assign a port instead. It also runs the formatter on this file.
# Result
No more tests failures.
* Extend the integration test harness to track FDs
Motivation
This patch extends the NIO integration test harness to track
file descriptors, in particular to search for leaks. This
change has been validated on Linux and Darwin, and in both cases
correctly diagnoses FD leaks.
The goal is to enable us to regression test for things like
Modifications
- Add support for hooking socket and close calls.
- Wire up this support into the test harness.
- Extend the test harness to handle the logging.
- Add new regression test for #2047.
Results
We can write regression tests for FD leaks.
* Disable FD checking in most builds.
I'm doing this for speed reasons
* Always print the leaked fds number
* 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
Motivation:
The fix provided in #2407 was subtly wrong. ignoreSIGPIPE, which throws
the error in question, closes the FD on error _except_ on EINVAL from
fcntl, where it instead does not. This inconsistent behaviour is the
source of the bug. Because this behaviour is inconsistent, the fix from
PR #2407 is also inconsistent and can in some cases double-close the
socket.
The actual issue is not as old as I expected: the code can be observed
by reviewing the change in #1598, which incorrectly inserted the error
transformation before the call to close.
Modifications:
- Revert the change from #2407.
- Move the close in ignoreSIGPIPE to before the error check, rather than
after, so we unconditionally execute it.
Result:
More resilient fix.
Motivation:
When an error is hit during a read loop, a channel is able to tolerate
that error without closing. This is done for a number of reasons, but
the most important one is accepting sockets for already-closed
connections, which can trigger all kinds of errors on the read path.
Unfortunately, there was an edge-case in the code for handling this
case. If one or more reads in the loop had succeeded before the error
was caught, the inner code would be expecting a call to readIfNeeded,
but the outer code wouldn't make it. This would lead to autoRead
channels being wedged open.
Modifications:
This patch extends the Syscall Abstraction Layer to add support for
server sockets. It adds two tests: one for the basic accept flow, and
then one for the case discussed above.
This patch also refactors the code in BaseSocketChannel.readable0 to
more clearly show the path through the error case. There were a number
of early returns and partial conditionals that led to us checking the
same condition in a number of places. This refactor makes it clearer
that it is possible to exit this code in the happy path, with a
tolerated error, which should be considered the same as reading
_something_.
Result:
Harder to wedge a channel open.
Motivation:
In some circumstances we can accept a socket that is already closed. In
those cases, creating the underlying Socket type will fail, as
attempting to ignore SIGPIPE will fail. On Apple platforms, this causes
us to leak the accepted socket, and can lead to file descriptor
exhaustion.
Modifications:
- Close the accepted socket if we fail to create a Socket class
Result:
No FD leaks
# Motivation
I spotted a bug in the ALPNHandler where it doesn't properly unbuffer reentrant reads. This can lead to dropped reads.
# Modification
Instead of buffering into an array we are now buffering into a Deque and unbuffer as long as there are reads in the Deque.
# Result
No more dropped reads.
### Motivation:
Follow up PR for https://github.com/apple/swift-nio/pull/2399
We currently still return `nil` if the current `Task` is canceled before the first call to `NIOThrowingAsyncSequenceProducer.AsyncIterator.next()` but it should throw `CancellationError` too.
In addition, the generic `Failure` type turns out to be a problem. Just throwing a `CancellationError` without checking that `Failure` type is `any Swift.Error` or `CancellationError` introduced a type safety violation as we throw an unrelated type.
### Modifications:
- throw `CancellationError` on eager cancellation
- deprecates the generic `Failure` type of `NIOThrowingAsyncSequenceProducer`. It now must always be `any Swift.Error`. For backward compatibility we will still return nil if `Failure` is not `any Swift.Error` or `CancellationError`.
### Result:
`CancellationError` is now correctly thrown instead of returning `nil` on eager cancelation. Generic `Failure` type is deprecated.
Motivation:
Up until recently, it has not been possible to regression check our
documentation. However, in recent releases of the DocC plugin it has
become possible to make warnings into errors, making it possible for us
to CI our docs.
This patch adds support for doing that, and also cleans up our
documentation so that it successfully passes the check.
Along the way I accidentally wrote an `index.md` for `NIOCore` so I
figure we may as well keep it.
Modifications:
- Structure the documentation for NIOCore
- Fix up DocC issues
- Add `check-docs.sh` script to check the docs cleanly build
- Wire things up to our docker-compose scripts.
Result:
We can CI our docs.
Co-authored-by: George Barnett <gbarnett@apple.com>
* 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
Motivation:
NIOLock uses a storage object constructed from a ManagedBuffer. In
general this is fine, but it's a tricky API to use safely and we want to
avoid violating any of its guarantees.
Modifications:
- Store the value in the header and the lock in the elements
- Add debug assertions on alignment
Result:
We'll be a bit more confident of the use of NIOLock
Motivation:
SwiftPM has changed its default layout for packages in
apple/swift-package-manager#6144. This breaks our CI, which assumes the
prior layout. We should work around this.
Modifications:
Enhance the code to tolerate both layouts.
Result:
Integration tests run on all platforms
Motivation:
The recently added UDP GRO tests fail on some older Linux Kernel
versions. We believe that UDP GRO support on loopback was limited in
early versions so we should tolerate those failures.
However, we've verified that on 5.15 and newer that GRO is supported so
we should not tolerate failure in those cases.
Modifications:
- Add shims to CNIOLinux to get system info via uname
- Verify GRO works before running GRO tests and if it doesn't then
validate the kernel version isn't greater than 5.15.
Results:
Less flaky tests.
* Pool buffers for messages and addresses.
* Revert changes related to controlMessageStorage
* Cosmetic fix.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Support was added for UDP_SEGMENT in #2372 which allows for large UDP
datagrams to be written to a socket by letting the kernel or NIC segment
the data across multiple datagrams. This reduces traversals across the
network stack which can lead to performance improvements. UDP_GRO is the
receive-side counterpart allowing the kernel/NIC to aggregate datagrams
and reduce network stack traversals.
Modifications:
- Add a function in CNIOLinux to check whether UDP_GRO is supported
- Add the relevant socket and channel options
- Add tests
Result:
- UDP_GRO can be enabled where supported and applications may receive
large buffers.
mark syncShutdownGracefully noasync
Motivation:
The code as-is blocks the calling thread.
Modifications:
* mark `EventLoopGroup.syncShutdownGracefully()` and `NIOThreadPool.syncShutdownGracefully()` noasync on Swift > 5.7
* offer NIOThreadPool.shutdownGracefully()
* add renamed to syncShutdownGracefully()
Motivation:
EventLoopTest has an availability annotation for the various Darwin
platforms. This availability check is repeated in a few tests which
yields warnings.
Modifications:
Remove the class level availability guard.
Result:
Warning free.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
I (foolishly) didn't validate the test fix in #2382, instead I validated
that the original test passed with 61 segments (rather than 64). The
channel needs to be recreated first.
Modifications:
- Rebuild the channel before trying again if 64 segments is too many.
Result:
Test passes.
Motivation:
On older kernel versions testWriteBufferAtGSOSegmentCountLimit fails
because the write fails with EINVAL. This appears to be a kernel bug as
it passes on more recent versions.
Modifications:
- Try again with a lower segment limit if the write fails with EINVAL.
Result:
Less flaky test.
setOption forms a raw pointer to a generic argument. The compiler will
warn on this as of:
[proposal] Constrain implicit raw pointer conversion... #1963https://github.com/apple/swift-evolution/pull/1963
/Sources/NIOPosix/BaseSocket.swift:286:31: warning: forming
'UnsafeRawPointer' to a variable of type 'T'; this is likely incorrect
because 'T' may contain an object reference.
option_value: &val,
^
Ideally, this would be fixed by adding a BitwiseCopyable constraint to
the 'value' parameter of 'BaseSocker.setOption'. That would not only
eliminate the warning, but would make the API safer. But
BitwiseCopyable isn't quite ready for public use. In the meantime,
this is a reasonable workaround.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
On Linux, the UDP_SEGMENT socket option allows for large buffers to be
written to the kernel and segmented by the kernel (or in some cases the
NIC) into smaller datagrams. This can substantially decrease the number
of syscalls.
This can be set on a per message basis on a per socket basis. This
change adds per socket configuration.
Modifications:
- Add a CNIOLinux function to check whether UDP_SEGMENT is supported on
that particular Linux.
- Add a helper to `System` to check whether UDP_SEGMENT is supported on
the current platform.
- On Linux only:
- add the udp socket option level
- add the udp_segment socket option
- Add the `DatagramSegmentSize` channel option.
- Get/Set the option in `DatagramChannel`
Results:
UDP GSO is supported on Linux.
Co-authored-by: Cory Benfield <lukasa@apple.com>
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:
Our time types are trivial, and they should be fully transparent. This
produces minor performance improvements in code handling time types, but
is mostly useful in terms of allowing the compiler to observe that these
functions have no side effects, thereby eliding some ARC traffic.
Modifications:
Make our time types inlinable.
Result:
Better performance.
dnrops
Cory Benfield