* 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
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:
Our allocation counter tests still build for 5.0 by default. This isn't
great, but we've been getting away with it because building for 5.7
requires changing the way we express our dependencies. Doing that is a
big breaking pain in the neck that requires changes in multiple repos.
As a shorter-term goal, however, to enable testing in at least one more
repo (swift-nio-ssh), this PR lifts the version to 5.1, which is the
last release compatible with the Package.swift dependency structure we
use but that supports the Platforms we need to express.
Modifications:
- Move allocation counter to 5.1
- Add platforms from swift-nio-ssh
Result:
We can write allocation counter tests for swift-nio-ssh.
* RawSocket prototype
* Conform `ProtocolSubtype` to `Hashable`
* Add public `NIOIPProtocol` type
Make `ProtocolSubtype` internal
* Subset of IANA protocols with an RFC
* Add `CustomStringConvertible` to `NIOIPProtocol`
* Add `init(_ rawValue: Int)`
* Rename `NIOBSDSocket.ProtocolSubtype.ip` to `.default`
* Add `NIOBSDSocket.ProtocolSubtype.mptcp`
and remove `NIOBSDSocket.mptcpProtocolSubtype`
# Motivation
We landed the async bridge types a while back but never added allocation and performance tests. Since we expect these types to be used performance critical paths we really should cover those with tests.
# Modification
Extends the allocation counter scaffolding to support async tests. Furthermore, add allocations tests for both the writer and producer. Lastly, I a also added a performance test for the producer.
# Result
We now have baseline tests for the `NIOAsyncWriter` and `NIOAsyncSequenceProducer`
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
SwiftNIO periodically drops support for older Swift versions. Now that
5.7 has been released, 5.4 will be dropped.
Modifications:
- Remove 5.4 specific Package.swift and docker-compose
- Update the 5.7 docker-compose to use the released 5.7 and move from
focal (2004) to jammy (2204)
- Remove unused swiftformat from Dockerfile
- Update tools version in syscall wrapper tests to 5.5
- Update docs
Results:
Minimum Swift version is 5.5
* Adopt `Sendable` for types in `BSDSocket` namespace
* Adopt `Sendable` for `CircularBuffer` and `MarkedCircularBuffer`
and also `Endianness`
* move `NIOSendable` into its own file and copy it over to syscall integration tests
* Drop support for 5.2 and 5.3
As outlined in a [Swift forums post in November ’21](https://forums.swift.org/t/swiftnio-swift-version-support/53232), SwiftNIO will only support the latest non-patch Swift release and the 2 immediately prior non-patch versions.
In this commit we drop support for Swift 5.2 and 5.3. We update CI for Swift 5.4 to run on bionic instead of focal to ensure that we still test bionic.
* Added Versions paragraph to Security document
* Apply suggestions from code review
Co-authored-by: Cory Benfield <lukasa@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
The nightly builders have started automatically adding a struct with
@main to packages generated using swift package init. This is
interfering with our syscallwrapper tests, so we should delete that
file.
Motivation:
Protocols like Sequence have "private" hooks that can be implemented to
provide fast-paths for some operations. We missed a few on BBV, and I'd
like to add them. This is one use-case where they can help.
Modifications:
- Add an allocation-counter benchmark and a runtime benchmark for
copying BBV to Array.
Result:
We have some benchmarks.
### Motivation:
`read` should never return `EINVAL`. If it does return it though it is a bug inside NIO and we should precondition for it similar to what we do for `EBADF` & `EFAULT`. Closes https://github.com/apple/swift-nio/issues/1339
### Modifications:
This PR adds a new precondition for `EINVAL` and adds test that exercise this precondition.
### Result:
We are now preconditioning against `read` returning `EINVAL`.
### Motivation:
In issue https://github.com/apple/swift-nio/issues/1316, we see a large number of allocations to happen when scheduling tasks. This can definitely be optimized. This PR adds a number of baseline allocation and performance tests for both `scheduleTask` and `execute`. In the next PRs, I am going to try a few optimizations to reduce the number of allocations.
### Modifications:
Added baseline performance and allocation tests for `scheduleTask` and `execute`
Motivation:
To justify performance changes we need to measure the code being
changed. We believe that `HTTPHeaders.subscript(canonicalForm:)` is a
little slow.
Modifications:
- Add allocation and performance tests for fetching header values in
their canonical form
Results:
More benchmarks!
Motivation:
The remaining NIO code really conceptually belongs in a module called
NIOPosix, and NIOCore should really be called NIO. We can't really do
that last step, but we can prepare by pushing the bulk of the remaining
code into a module called NIOPosix.
Modifications:
- Move NIO to NIOPosix
- Make NIO an umbrella module.
Result:
NIOPosix exists.
Motivation:
This is the next stage of our move of common interchange objects to
NIOCore and out of the NIO module. This time around we need
SocketAddress, which is part of the Channel API. Sadly, SocketAddress is
not as clean as some of the other ports, because it leaks a number of
POSIX-y concepts. This also forces us to bring along NIOBSDSocket, which
ideally we would not move, but we foolishly exposed as API on
SocketAddress.
Modifications:
- Move NIOBSDSocket API components into NIOCore.
- Split out the internal abstractions for NIOBSDSocket and leave those
in NIO.
- Move SocketAddress into NIOCore.
Result:
SocketAddress will be sitting in NIOCore.
Motivation:
Peter thinks that result-erasing maps should not allocate, and we have
special code paths in the code to try to make Void -> Void maps not
allocate. Sadly, both code paths currently do allocate.
Per our rules for not trying to make optimizations without data, we
should start measuing these closures so we can make optimizations.
Modifications:
- Added an alloc couter test for result-erasing maps.
Result:
Alloc counter test suitable for any fix of #1697.
Motivation:
Allocation counter tests are good, and we aren't measuring this today.
Modifications:
- Wrote some add/remove tests that use different remove functions.
Result:
Better insight into performance.
Motivation:
The allocation counter test framework only supports a single shared
file which is quite limiting, especially if you rely on generated code.
Modifications:
- The '-s' option can now be passed multiple times
- The symlink name for any of the passed files will be the basename of
the provided file rather than 'shared.swift'
Result:
Allocation counter test framework can use more than one shared file.
* Add synchronous channel options
Motivation:
The functions for getting and setting channel options are currently
asynchronous. This ensures that options are set and retrieved safely.
However, in some cases the caller knows they are on the correct event
loop but still has to pay the cost of allocating a future to either get
or set an option.
Modifications:
- Add a 'NIOSynchronousChannelOptions' protocol for getting and setting
options
- Add a customisation point to 'Channel' to return 'NIOSynchronousChannelOptions'.
- Default implementation returns nil so as to not break API.
- Add implementations for 'EmbeddedChannel' and 'BaseSocketChannel'
- Allocation tests for getting and setting autoRead
Results:
Options can be get and set synchronously.
Motivation:
`ChannelPipeline` is explicitly thread-safe, any of the operations may
be called from outside of the channel's event loop. However, there are
often cases where it is known that the caller be on the right event
loop, and an asynchronous API is unnecessary.
In some cases -- such as when a pipeline is configured dynamically and
handlers are added from the 'channelRead' implementation of one handler
-- it forces the caller to write code that they might not actually need:
such as buffering events which may happen before the future completes.
This is unnecessary complexity when the caller knows that they must
already be on an event loop.
Modifications:
- Add a 'SynchronousOperations' view to the 'ChannelPipeline' which is
available to callers via 'syncOperations'.
- Supported operations include: adding a handler, adding multiple
handlers, retrieving a context via various predicates and retrieving a
handler of a given type.
- Some of the operations in 'ChannelPipeline' were refactored to have an
explicitly synchronous version, asynchronous versions complete their
promise based on the result of these calls.
- Various minor documentation fixes and addition of 'self' where it was
not used explicitly.
Result:
Users can perform synchronous operations on the 'ChannelPipeline' if
they know they are on the right event loop.
Motivation:
The alloc counters need to store the pointer to the original libc
implmentations of the hooked functions somewhere. Previously, we would
store them in thread locals. That works fine but creates quite some
overhead (and allocations) per thread (to do dlsym on every thread).
Modifications:
Instead, we now store the libc function pointers in (atomic) globals so
we need to only resolve each function once, no matter how many threads
we use.
Result:
Faster, and more accurate.
Motivation:
I'm sick of typing `.init(major: 1, minor: 1)`.
Modifications:
- Added static vars for common HTTP versions.
Result:
Maybe I'll never type `.init(major: 1, minor: 1)` ever again.
* Add allocation test for adding multiple handlers
Motivation:
I believe there is at least 1 avoidable allocation in this area.
Even if there isn't, making sure we don't increase allocations is good.
Modifications:
Add a test of allocations when adding multiple handlers.
Set limits for docker images.
Result:
Allocations when adding multiple handlers are now checked.
* Remove an allocation from addHandlers
Motivation:
Fewer allocations should improve performance.
Modifications:
Split out a sub function from addHandlers.
I originally thought I'd have to change the part of this
function which reads `var handlers = handlers` as there was
a surprising allocation at the beginning of this function.
It seems that breaking out some of the logic is sufficient
to remove an allocation.
Result:
1 fewer allocation.
* Fix up alloc tests.
Motivation:
It's possible relaying ECN information will cause allocations.
Having a test which uses them will ensure this doesn't happen.
Modifications:
Added metadata data to always send ECN state and set a channel
option requesting to recieve it.
Result:
Allocation test will attempt to send and receive ECN data.
Motivation:
Split out the internal implied BSD socket API code to allow alternative implementations of these interfaces on other platforms.
Modifications:
- Split the code apart.
- Provide POSIX and Windows implementations of the BSD socket API.
Result:
Should be easier to run NIO unmodified on Windows.
Motivation:
There is at least a theoretical race to flush before close in prior version.
Having terrible code in the NIO repo is asking for someone to copy it.
Modifications:
flatMap the various parts of the client together which also ensures the
flush is complete before close is called.
Result:
Slightly nicer code, slightly fewer allocations.
Motivation:
so_reuseaddr on linux allows multiple binding to the same port simultaneously - this has not been seen to happen in practice but it is best to be cautious.
Modifications:
Remove address reuse from the UDP allocation tests.
Result:
Reduced danger of binding the same port twice.
Motivation:
Hanging tests make doing work and asserting correctness very difficult.
Modifications:
Remove address reuse from the server side of the udp channel.
Result:
The tests no longer hang.
(The reason things we hanging was that the client side was occasionally getting assigned the same port number as the server side (extremely agressive port reuse)
Motivation:
The Swift runtime is now using malloc_zone_*, we need to implement
replacements for these too. This is just a first pass, eventually, we
should implement _all_ replacements as `malloc_zone_memalign` which is
powerful enough to implement all others.
Modifications:
Provide new replacements.
Result:
Alloc tests work again on macOS.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
We now have sufficiently many integration tests that the number of TCP
sockets in TIME_WAIT begins to cause resource pressure on the system.
This can lead to the integration tests failing due to being unable to
assign a socket.
We can force this to not happen by disabling linger. As we're running on
a lossless fabric (localhost), this is entirely safe.
Modifications:
- Disable TIME_WAIT for integration tests.
Result:
Integration tests will pass.
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Co-authored-by: Peter Adams <pp_adams@apple.com>
Motivation:
The 1000 TCP connections allocation counter test did way too many thread
crosses and other things that allocated for unrelated reasons.
Modifications:
Remove those.
Result:
Much more stable
Motivation:
Allocation tests are very hard to use productively if they don't produce stable results.
Modifications:
Change TCP and UDP connection tests to monitor on the server side as well as the client side to make sure all data is completely send and received before stopping counting allocations.
Result:
Allocation tests are more stable than before.
Motivation:
It's 2020, we shouldn't use unnecessary offensive, insulting, or
outdated language.
Modifications:
Fix a few examples.
Result:
More inclusive.
Motivation:
There are multiple sub-optimal ByteBuffer creation patterns that occur
in the wild. Most often they happen when people don't actually have
access to a `Channel` just want to "convert" a `String` into a
`ByteBuffer`. To do this, they are forced to type
var buffer = ByteBufferAllocator().buffer(capacity: string.utf8.count)
buffer.writeString(string)
Sometimes, they don't get the capacity calculation right or just put a
`0`.
Similar problems happen if NIO users want to cache a ByteBuffer in their
`ChannelHandler`. You will then find this code:
```swift
if self.buffer == nil {
self.buffer = receivedBuffer
} else {
var receivedBuffer = receivedBuffer
self.buffer!.writeBuffer(&receivedBuffer)
}
```
And lastly, sometimes people want to append one `ByteBuffer` to another
without mutating the appendee. That's also cumbersome because we only
support a mutable version of `writeBuffer`.
Modifications:
- add `ByteBuffer` convenience initialisers
- add convenience `writeBuffer` methods to `Optional<ByteBuffer>`
- add `writeBufferImmutable` which doesn't mutate the appendee.
Result:
More convenience.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
It's often very important to test that in certain cases we actually
crash with a certain error message whilst having access to all of NIO.
Modifications:
Add a crash tester test suite.
Result:
We can test crashers.
Fine grained UDP allocation tests
Motivation:
The existing UDP allocation tests count allocations everywhere.
Split out 3 tests to cover:
Making connections.
Making bootstraps.
Transferring data.
Modifications:
Three new tests.
Result:
Easier to debug changes to allocation profiles in UDP.
Motivation:
The existing tcp allocation tests cover everything from bootstrap creation through to sending data in one massive test. The parameters of how many iterations control the focus of the test. It would be better to be more explicit about what we're trying to test.
Modifications:
2 new tests:
* client bootstrap creation.
* connection establishment.
(Data transfer is already covered by the ping_pong test)
Result:
Easier to assess the impact of changes to memory allocation patterns.
Motivation:
The linker test script that checks what libraries we link was totally
broken.
Modifications:
Fix the linker test script.
Result:
We can correctly determine what we link and what not.
Motivation:
The names of most of the BSD socket option values were brought over into
their NIO helper properties, with their namespace removed. This vastly
increases the risk of collision, particularly for things like TCP_INFO,
which just became .info.
Modifications:
- Added the prefixes back, e.g. `.info` is now `.tcp_info`.
- Renamed socket type `.dgram` to `.datagram`, as this is the nice clean
API and we should use nice clean names.
Result:
Better APIs
UDP based allocation tests (#1496)
Motivation:
With future work being considered in the UDP space it will be useful to have a baseline measure of allocations.
Modifications:
New allocation tests for UDP -
Ideas inspired by the TCP allocation tests.
Code a modified version of UDPEchoServer and UDPEchoClient examples.
Result:
There are UDP based allocation tests.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Cory Benfield