Motivation:
When scheduling a task too far into the future on a Darwin-kernel,
a fatal error is produced. Whilst the error indicates that a parameter
to the kevent syscall was invalid (EINVAL) it gives no further
information as to the root cause.
As the user's intent is to schedule an task far into the future, a fatal
error should not result. Instead the time should be clamped to the
maximum value that the Darwin-kernel supports. This ensures that
, provided the system remains running, the task will eventually run
(though this may be several yeasr into the future).
Modifications:
Duplicated the bounds checks from those in Darwin-kernel interval timer,
clamping timespec values to the maximum value. Added an assert to check
that the timespec tv_nsec field is valid (according to definition of
the timespec datatype in ISO C11). And a precondition on the tv_sec
field being > 0 (tasks schedule with times <= 0 should not result in
a kevent being registered as they are scheduled immediately).
Added note clarifying behaviour under Darwin-based OSes to the EventLoop
scheduleTask() method.
Added new unit test scheduling a task at the maximum value (which
under any regression would crash).
Result:
Scheduled tasks where the timeout exceed the maximum value supported by
Darwin-based OSes will be clamped to the maximum.
Co-authored-by: Graeme Jenkinson <graemee_jenkinson@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
This repairs the build on the CNIOWindows module on Windows. The `NIO`
macro is unavailable in the definition (intentionally) and was being
used accidentally.
These need to be filled out, but lets stub them out for the time being
to get the symbol resolution sorted out. These are possible to
implement using the WinSDK.
* Cleanup unix socket pathname on server socket close or bind
* cleaner use of swift syntax
* using stat and unlink via syscal wrappers
* separate error type when UDS path is not a socket file
* track if socket needs cleanup to avoid extra syscall
* struct instead of enum for a single new error type
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Resolves https://github.com/apple/swift-nio/issues/1558
A common MTU on the internet is 1500 bytes, and it should fit in the initial size buffer
Modifications:
Only the default initial size is increased
Result:
Little is changed, but should result in fewer reallocations e.g. during TLS conneciton handshake
`Posix.*` cannot be used on Windows. Sockets and (file) descriptors are
in entirely different namespaces, and will result in an invalid
operation.
Co-authored-by: Cory Benfield <lukasa@apple.com>
This removes the testing-only API of `poll` on Windows which does not
have a semantic compatible poll.
Co-authored-by: David Evans <d.evans@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
`ssize_t` is not a standard type and does not have a portable Swift
spelling. Use `size_t` which is compatible in size, but is signed
instead. This is needed in order to be compatible to more standard
conforming environments like Windows.
Motivation:
`MarkedCircularBuffer.popFirst()` asserts that the backing buffer should
contain more than zero elements yet `popFirst()` allows `nil` to be
returned when there is no value to return.
Modifications:
- Move the assertion from `popFirst() -> Element?` to `removeFirst() -> Element`
- Test for `popFirst()`
Result:
- We can safely call `popFirst()` on an empty `MarkedCircularBuffer` in
debug mode.
Motivation:
The doc block for `_ChannelOutboundHandler.read(context:)` mentions suppressing
a *flush*, it appears this is a copy-paste error from the docs for `flush(context:)`
above and should in fact refer to suppressing the *read*.
Modifications:
Substituted `read` for `flush` in the doc block for `_ChannelOutboundHandler.read(context:)`
Motivation:
While looking at a different part of the code in swift-nio-http2 I
noticed that we can't inline many straightforward ByteBufferView
operations, including bytewise-access. That's pretty sad, so I thought
we should fix it.
Modifications:
- Slap @inlinable on all the BBV entry points.
Result:
Better code generation, particularly around repeated subscript accesses.
Motivation:
If a write is buffered in the `PendingWritesManager` and the high
watermark is breached then a writability change will be fired as a
result. The corresponding event when the channel becomes writable again
comes when enough pending bytes have been written to the socket (i.e. as
a result of a `flush()`) and we fall below the low watermark.
However, if a promise associated with a write has a callback
registered on its future which writes enough data to breach the high
watermark (and also flushes) then we will re-entrantly enter
`flushNow()`. This is okay, `flushNow()` protects against re-entrancy
and any re-entrantly enqueud flushed writes will be picked up in the
write-spin-loop.
The issue here is that the `writeSpinLoop` does not monitor for changes
in writability. Instead if checks the writability before it begins and
after it completes, only signalling if there was a change. This is
problematic: the re-entrant write-and-flush could have caused the
channel to become unwritable yet no event will be fired to make it
writable again!
Modifications:
- Store a local writability state in the pending writes manager and
modify it when we emit writability changes
Result:
- Better protection against re-entrant writability changes.
Motivation:
NIONetworkInterface was a useful type, but its implementation was fairly
fatally flawed in a number of ways. While we could have lived with the
awkward type construction (it's a class, which isn't the semantic we
want), the ultimate flaw is that you cannot create a NIONetworkInterface
for an interface that does not have an IP address.
That's not helpful, given that it's not uncommon for an interface to not
have an IP address!
Modifications:
- Deprecate NIONetworkInterface
- Add NIONetworkDevice
- Provide new methods on MulticastChannel that use NIONetworkDevice
instead of NIONetworkInterface.
- Provide default implementations of new MulticastChannel protocol
requirements.
- Implement new MulticastChannel protocol requirements.
- Update the tests.
Result:
Devices will be able to reflect network devices without IP addresses.
Motivation:
All tests should get run.
Modifications:
The script to locate linux tests only scans files with names ending with Test/Tests.
Change the filename to match this.
Result:
All tests are run now on linux.
* Recover from EINVAL even if the error is untyped
* Handle fcntl EINVAL error
* Fix EINVAL test
* Rename and deprecate errors
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Per SR-10219, Data does not implement withContiguousStorageWithAvailable
and the maintainers do not believe it can. This forces code that writes
Data into ByteBuffers into slow-paths. Given that Data conforms to
ContiguousBytes and DataProtocol, we can arrange to serve a number of
use-cases by providing fast paths for all conforming types. This makes
it easier to use types vended by other Foundation-using libraries, such
as Swift Crypto.
Modifications:
- Implement `writeContiguousBytes` and `setContiguousBytes`.
- Implement `writeData` and `setData` for `DataProtocol` implementations
Result:
Better support for writing `Data` and friends into `ByteBuffer`s.
Motivation:
Right now Heap.heapify incurs quite a lot of ARC traffic when it is used
in SelectableEventLoop. Specifically, each of the invocations of
self.comparator in heapify incurs three refcount operations. These three
are all the result of the fact that the comparator is an arbitrary
function, but all three are guaranteed to be unnecessary.
The first refcount is for the closure context for self.comparator. Of
course, self.comparator has only two possible values, neither of which
needs a closure context, but the compiler doesn't know that and the type
system doesn't encode it.
The other two refcounts are for the ScheduledTask objects themselves.
Again, the compiler doesn't know what the comparator function will do,
so it must defensively ensure that a retain/release is inserted for the
arguments of that function.
If we happen to hit both branches of the code, we'll execute 6 retains
and 6 releases, and even in fairly average cases we'll execute 3 of
each.
All of this is unnecessary: the only runtime function ever used here is
a trivial comparison operation. We know what it is at compile time, we
just have to ask. While it was admirable to have the configurability of
the priority queue in place, after 3 years I think we have a pretty good
feeling that we're not going to need the ability to have a max-heap
instead of a min-heap.
We can always get this back if we need it.
Modifications:
- Removed the configurability of Heap.
- Removed HeapType.
- Removed arguments from PriorityQueue
- Removed all tests testing now-removed max-heap functionality.
Result:
Less ARC traffic in the core task scheduling loop.
Motivation:
This has come as a surprise to at least one person and is easy to forget.
Modifications:
Doc commented added at the top of ChannelPipeline.
Result:
Docs are clearer.
Motivation:
We included netinet/ip.h in CNIODarwin, which unfortunately is not
modularised. This causes issues when using SwiftPM to create xcodeproj
files. There's no real reason to do it this way, when we can just as
easily abstract the issue.
Modifications:
- Moved netinet/ip.h to the .c file instead of the .h.
- Defined some exported namespaced integers to correspond to the macros.
- Performed platform abstraction in Posix instead of everywhere.
Result:
- Creating xcodeproj files should be fine.
Motivation:
It turns out using this overflows Int on "Any iOS Device"
Modifications:
Switch the first d to a 7.
Result:
Code will compile on "Any iOS Device".
Motivation:
Ruby on Xenial is too old for the cocoapods downloader which jazzy uses.
Modifications:
Change Dockerfile to not install Jazzy on xenial.
Result:
Dockerfile will build again, but bionic or later is required for docs.
Motivation:
Network congestion can be controlled without packet loss if explicit congestion notification messages are understood.
Modifications:
Receive single and vector paths to get notification and surface.
Send paths for both single and vector sends.
Round trip tests.
Storage for control messages added in a reusable way.
I have tested allocations and they seem to be fine but the allocation tests for vector read and write were not stable enough to sensibly add to CI.
Result:
Explicit Congestion Notifications can be both sent and received.
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:
ECN information is held in metadata.
As part of the path to writing ECN to the network the
required state needs to pass through PendingDatagramWritesManager.
Modifications:
Add an extra metadata field to PendingDatagramWrite and ensure
this data is passed to the correct write method.
Result:
Nothing visible - just preparation for actual ECN send.
Motivation:
This needs to be possible so users can specify what
ECN status they wish to send. The auto-generated
initialiser is internal.
Modifications:
Add public initialiser to AddressedEnvelope.Metadata
Add public initialiser to AddressedEnvelope taking Metadata
Result:
Users will be able to construct AddressedEnvelopes with Metadata set.
Motivation:
cmsghdrs can be used to send an receive extra data on UDP packets.
For example ECN data.
Modifications:
Map in Linux and Darwin versions of cmsghdr macros as functions.
Create strctures for holding a received collection; parsing ecn
data from a received collection and building a collection suitable
for sending.
Result:
Functions to manipulate cmsghdr and data exist.
Motivation:
sendmsg is more powerful - this is paving the way for
Explicit Congestion Notifications to be sent.
Modifications:
Replace the sendto system call with sendmsg up to DatagramChannel.
Result:
Sendmsg will be used to send UDP data.
Motivation:
The socket address types are some of the worst types for handling in
Swift. They pervasively type-pun in a way that Swift strongly wants to
resist, and they expose a bunch of data through awkward fixed-length
arrays.
We've done a number of passes over this code to improve its pointer
hygeine. This is another one. Here, we remove some uses of the memory
binding APIs that were unnecessary, and that can instead take advantage
of the way Swift handles binding memory of homogeneous tuple types.
In Swift, whenever we have a homogeneous tuple (i.e. a tuple whose
elements are all the same type), that memory is actually bound to _two_
types: to the tuple type, and to the type of the elements. This allows
us to use `assumingMemoryBound(to:)` to convert between the pointer to
the tuple and the pointer to the first element of the tuple.
This knowledge lets us remove calls to `bindMemory` and
`withMemoryRebound`. Neither call is correct to use in this situation,
and while we were unlikely to encounter any damage, this is the best
possible use of the Swift pointer APIs for this work.
Modifications:
- Rewrite accesses to the UNIX domain socket path to use appropriately
typed pointers without binding or rebinding memory.
- Added some assertions about the static types of the stored elements.
Result:
Better management of pointer type bindings.
Motivation:
When attempting to obtain metadata about a read, we need to use recvmsg
in order to obtain that data. While we're using recvmmsg for vector
reads right now, our scalar reads use recvfrom, which does not provide
us with that metadata.
While we're not extracting metadata right now, we may well do so in
future, so we can apply it now.
Modifications:
- Move recvfrom usage to recvmsg.
Result:
We'll be able to extend recvmsg to extract metadata.
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:
Fixes: #1334
Modifications:
ELF hops on the correct EL before executing.
Result:
Fixes precondition failure, caused by being on the wrong ELG.
Cory Benfield