Motivation:
Previously we tried to use `host` to determine localhost's IPv4 address.
That didn't work reliably in Docker and sometimes on macOS. Let's just
fix this to 127.0.0.1.
Modifications:
Hardcode localhost's IPv4 address as 127.0.0.1.
Result:
More stable test_16_tcp_client_ip
Motivation:
In shell, subshells don't inherit the shell flags (such as `set -eu`)
and therefore need to set them again. The allocation counter test uses a
subshell but forgot to `set -eu` in the subshell.
Modifications:
`set -eu` in the subshell for the allocation counter tests.
Result:
if there's an error building the test program the test now fails.
Motivation:
We have code running on multiple threads and certain allocations/frees
happen for example after a promise has been fulfilled. We can't
deterministically wait for that so the only thing we can do is sleep a
little and wait...
Modifications:
Added a 100ms delay after an allocation counted test run.
Result:
Hopefully we don't see this again:
["total_allocations": 322051, "remaining_allocations": 24], ["total_allocations": 322051, "remaining_allocations": -24]
where the first test run had 24 allocations that weren't freed and the
second one on the other hand had 24 frees for allocations that "didn't
happen"...
Motivation:
We had a lot of problems with the Channel lifecycle statemachine as it
wasn't explicit, this fixes this. Additionally, it asserts a lot more.
Modifications:
- made Channel lifecycle statemachine explicit
- lots of asserts
Result:
- hopefully the state machine works better
- the asserts should guide our way to work on in corner cases as well
Motivation:
Memory leaks are rare in Swift but they can happen. If we run a
realistic workload and the number of `malloc`s and `free`s is very close
to 0, we can be fairly sure we didn't have any (too bad) leaks.
Therefore this adds a simple leaks detector.
Modifications:
Add a leak detector.
Result:
We should now detect leaks even if the total number of allocations
doesn't increase.
Motivation:
In Swift, it's really easy to accidentally add allocations yet the
number of allocations is really important for performance.
Therefore it's useful to have an integration test that counts the number
of allocations and compares them to a reference (and fails if the number
of allocations increases).
Modifications:
- add an allocation counter
- add a HTTP1 client/server example that is run with the allocation
counter intrumentation
Result:
We should now be able to track the number of allocations.
Motivation:
We moved the priority queue implementation into the main `NIO` module
for performance reasons. I initially thought we didn't expose it to
external packages but we did (thanks @lukasa).
Modifications:
Bring back the `NIOPriorityQueue` module but deprecate it.
Result:
Speed & backwards compatibility :)
Motivation:
We can simplify the init method of ChannelHandlerContext and replace one let by a computed property.
Modifications:
Remove some if let statements that we not need and use computed property.
Result:
Simpler code and need one less variable per ChannelHandlerContext.
Motivation:
We only use our GetaddrinfoResolver for SocketChannels (TCP stream channels), so we should set some hints for getaddrinfo.
Modifications:
Set proto and socktype hints.
Result:
More correct usage of getaddrinfo. Fixes https://github.com/apple/swift-nio/issues/201.
Motivation:
generate_linux_tests.rb is written in a non idiomatic way.
Modifications:
I used Rubocop to refactor the code.
More info: https://github.com/bbatsov/rubocop
Result:
A more idiomatic Ruby script.
Motivation:
We triggered extra allocations for the generics on PriorityQueue/Heap.
They unfortunately can't be solved using `@_specialize` as the element
type is `ScheduledTask`.
Fortunately we never exposed those types externally so we can just
inline without breaking the public API.
Modifications:
Moved everything from the `NIOPriorityQueue` module into the `NIO`
module.
Result:
Less allocations, more happiness.
Motivation:
We should make use of lock.withLockVoid to simplify our code and make it more robust.
Modifications:
- Replace manual lock() / unlock() calls by withLockVoid(...)
- Mark lock.withLock* as @_inlineable to ensure we not get an allocation for the closure.
Result:
Code is cleaner and easier to understand.
Motivation:
We need to update isActive before notify the promises of connect and close to correctly refect Channel.isActive in callbacks.
Modifications:
- Correctly update isActive before notify promises.
- Add unit test.
Result:
Correct Channel state in callbacks.
Motivation:
When using KQueue we directly can tell what IOEvent we want to use in SelectorEvent without and branching.
Modifications:
Add new init method to SelectorEvent that takes the IOEvent directly and use it when using KQueue
Result:
Less branching per event when using KQueue
Motivation:
We can remove a promise allocation in Selector.closeGently if there are still Channels registered and also simplify the code a bit.
Modifications:
Remove creation of promise and just use eventLoop.new*Future(...) when needed.
Result:
Less allocations and cleaner code.
Motivation:
The Socket channels and PendingDatagramWritesManager had a bunch of
overhead left over from when they were handling flush promises. This was
only adding to code complexity and perf cost.
Modifications:
Removed the promise label in markPendingWritePoint.
Reworked the code to stop expecting write promises in
PendingDatagramWritesManager.
Result:
Smaller, faster code.
Motivation:
We can remove one lock() / unlock() pair by saving the next ready task during loop execution.
Modifications:
Remvoe one lock() / unlock() pair.
Result:
Less locking during EventLoop run executions.
Motivation:
At some point we needed to check for #available(OSX 10.12, *) which is not true anymore as we use our own Thread impl.
Modifications:
Remove #available(OSX 10.12, *) check
Result:
Remove incorrect and not needed check.
Motiviation:
When the Selector is closed we also close the registered Channels and notify the future once all the close operations complete. The problem here is that while the close operation may be complete already there may still be events flowing through the ChannelPipeline. The only way to ensure this not happens anymore is to take the closeFuture into account of each Channel (as the closeFuture will only be notified once all is done for a Channel).
Modification:
- Also take the closeFuture into account per Channel
- Add test
Result:
Safer shutdown of EventLoopGroup.
Motivation:
Resolves#228
Modifications:
Remove type information in empty closure fixed build warning with compiler suggestion
Result:
No API changes, no issues building on 4.1!
Motivation:
ServerSocketChannel should just fail the EventLoopFuture for unsupported operations and not crash.
Modifications:
- Correctly fail the EventLoopFuture for unsupported operations
- Add unit test.
Result:
No more crashes when users invoke unsupported operations.
Motivation:
The Head and Tail ChannelHandlers should not be accessible as these are implementation details of the ChannelPipeline and removal of these will even make the whole ChannelPipeline not work anymore.
Modifications:
- Make it impossible to access / removal the Head and Tail ChannelHandlers
- Exclude the Head and Tail ChannelHandler from the debug String
- Add unit tests.
Result:
More robust ChannelPipeline.
Motivation:
Usually we retry the syscall when EINTR is reported. This is not safe in the case of close(...) as the file descriptor may already be closed. The problem arise if this was the case and we retry it and the file descriptor is already reused again. In this case we would close a wrong file descriptor.
See:
- https://bugs.chromium.org/p/chromium/issues/detail?id=269623
- https://lwn.net/Articles/576478/
Modifications:
Ignore EINTR on close(...) and just assume the FD was closed.
Result:
Not possible to close the wrong file descriptor when close(...) reports EINTR.
Motivation:
Fix inconsistency of <Void> and <()> occurances in the codebase.
Modifications:
Changed EventLoop{Future,Promise}<()> occurances to EventLoop{Future,Promise}<Void>
Result:
Consistent code style for EventLoopPromise and EventLoopFuture.
Motivation:
b8055f7467 added some code to guard against the case when a fd is deregistered while processing ready events. On Linux it also added an assert which tried to verify that the fd in question is not registered anymore via epoll. The problem here is that it may also be closed which will have epoll_ctl fail with EBADF
Modifications:
Remove assert and add test-case which would have triggered the assert.
Result:
No more assertion error on linux.
Motivation:
{,Server,Datagram,Base}SocketChannel hold quite a bit of complexity and
also a many of lines of code. Yet they all shared the very same file
which isn't great.
Modifications:
- moved BaseSocketChannel into its own file
- made the methods that need overriding internal (from `fileprivate`)
Result:
SocketChannel.swift has a lot fewer lines of code.
Motivation:
`readIfNeeded` was triggered straight after the channel was registered
but _before_ `channelActive` was called which is wrong.
Modifications:
Moved the calls to `readIfNeeded` into `becomeActive0` and removed them
everywhere else (except some error path).
Result:
handle ChannelHandler lifecycle more correctly.
Motivation:
It's not clear to a user which inbound / outbound messages is expected for the different Channel implementations. This can result into wrong code and confused users.
Modifications:
Add some docs to explain which messages are supported / expected by the different Channels that can be bootstrapped.
Result:
More clear docs.
Motivation:
It's possible someone deregistered a Selectable while Selector.whenReady(...) is processed and an even for the now deregistered Selectable is ready. In this case we crashed (due a force-unwrap) on linux.
Modifications:
- Skip the processing of the Selectable if deregistered (by replacing force-unwrap with an if let).
- Add unit test.
Result:
No more crashes caused by this on Linux
Motivation:
Introduced some new parameters to fix#202
Modifications:
Created new parameters for the file and line newPromise method inside ChannelOutboundInvoker.
Result:
File and line numbers are now passed to the eventLoop object, all unit tests have been passed.
Motivation:
If connectSocket() completes directly we failed to mark the Channel active by calling becomeActive0(...).
Modifications:
Correctly call becomeActive0(...)
Result:
Mark Channel active if connect succeed directly.
Motivation:
If a non-blocking connect failed, the error indicated that `getsockaddr`
failed which isn't true.
Modifications:
Adjusted the message.
Result:
Fewer lies.
Motivation:
We should return a failed EventLoopFuture when getOption(...) / setOption(...) is called on a closed Channel as otherwise it may not be safe to modify the Channel ioptions after its closed.
Modifications:
- Add guards that check if the Channel is still open and if not fail the operation.
- Add testcase
Result:
Calling getOption(...) / setOption(...) on a closed Channel fails.
Motivation:
The HappyEyeballs state machine incorrectly assumed that it would always
be able to cancel any scheduled task that would execute on the event loop
that the state machine itself was running on. This is not accurate: if
the scheduled task is scheduled to execute at roughly the same time as
another callback into the state machine then it may be uncancellable.
Modifications:
Allowed the three scheduled tasks (connect timeout, connect delay, and
resolver delay) to fire in completed.
Result:
No fatalErrors in window conditions under high load
Motivation:
If we don't wait for the promise to succeed it may not be succeded
before we tear the loop down. This causes precondition failures.
Modifications:
We now always wait for the promise to succeed.
Result:
My tests will run.
Motivation:
fireChannelActive(...) must always be called before fireChannelRead(...) which was not the cause all of the times as once ChannelOptions were applied to the accepted SocketChannel it was possible that read0() was called during this. This would then lead to an assert error as it tried to reregister on the Selector before we ever registered.
Modifications:
- Ensure we only try to reregister once we have registered to the Selector
- Only trigger read0() or pauseRead0() if the autoRead value actually changed
- Add unit test
Result:
Correct order of events and no crashes.
Motivation:
When running swift test two warnings were printed.
Modifications:
- Signal that we want to ignore the return value
- Use boolean test in the while loop
Result:
Cleaner build
Motivation:
We allocated a new ContiguousArray per each EventLoop run when tasks needed to be executed. We can just re-use the same instance all the time and so safe malloc operations (we will need to do memset tho).
Beside this we also did not reserve the needed capacity up fron which could lead to realloc operations.
Modifications:
- Reuse same ContiguousArray instance
- Correctly set the initial capacity
Result:
Less overhead when processing tasks via the EventLoop
Motivation:
header iterator that could be used to keep access to the iterator with
type erasure. However, it was accidentally made internal, and therefore
useless.
Modifications:
Made the shim public.
Result:
Users can actually upgrade their systems.
Motivation:
Changes the Boolean `fulfilled` to `isFulfilled` to make it more clear it is in fact a Boolean.
Also changes a `func isRecoverable()` to `var isRecoverable`.
Both of these were non-public, so this is no breaking change.
Modifications:
Renamed one Boolean and changed one function to a variable.
Result:
No use of unncessary (and potentially misinforming) functions, as well as adhering to the Swift boolean naming guidelines.
Motivation:
Passing in a function directly makes the code a bit more concise.
Modifications:
Changing `flatMap { Int($0) }` to `flatMap(Int.init)`.
Result:
More concise code.
Motivation:
There are quite a few `switch`es that cover just two cases. Explicitly state this via an `if case` + `else` structure.
Modifications:
Changes something like:
switch a {
case .b:
return c
default:
return d
}
to
if case .b = a {
return c
} else {
return d
}
Result:
This should prevent misinterpretation and thus potential bugs in the future.
Motivation:
Removes unncessary explicit types that are inferred.
Modifications:
Infer types that are being returned from a function or computed variable.
Result:
Cleans up the code.
Motivation:
This changes two things:
- it removes the negation of non-used associated types (the removal of `(_)`).
- it prefers exhausting `switch`es over `default`ing.
This change does not impact bahavior; it is pure refactoring.
Modifications:
Omitting needless `(_)`'s and explicitly exhausting switches.
Result:
This change is pure refactoring.
Johannes Weiß