Motivation:
Previously we were not running the (child/server)channelInitializers on the
event loop associated to the `Channel` we're initialising. Almost all
operations in there are pipeline modifications which are thread safe so
it presumably wasn't a massive correctness issue. However it's very
expensive to hop threads that often and it is also very unexpected. This
addresses this issue.
Modifications:
made all (child/server)channelInitializers run on the event loop that is
associated to their `Channel`
Result:
- more correctness
- less unexpected behaviour
Motivation:
With #413 and #415 merged, we can turn up the level of nastiness in the
ChannelTests and delay the close that happens if registration fails in a
Bootstrap.
Modifications:
Delay close in the three registration fails ChannelTests
Result:
Tests are tougher.
Motivation:
If registration of a Channel fails we need to close that Channel.
Previously we'd just drop it on the floor which triggers an assertion
that an open channel got leaked.
Modifications:
In cases where the Channel registration fails, we need to close that
channel.
Result:
- No crashes if registration fails.
- fixes#417
Motivation:
EmbeddedChannel just treated all `write`s as flushed. That's not a good
idea as all real channels obviously only actually write the bytes when
flushed. This changes that behaviour for `EmbeddedChannel`.
Modifications:
- make `write` only write to a buffer
- make `flush` then actually pretend to write the bytes
Result:
- EmbeddedChannel behaves more realisticly
Motivation:
In certain cases it's useful to quiesce a channel instead of just
closing them immediately for example when receiving a signal.
This lays the groundwork by introducing the
`ChannelShouldQuiesceUserEvent` user event that when received can be
interpreted by a ChannelHandler in a protocol & application specific
way. Some protocols support tear down and that would be a good place to
initiate the tear down.
Modifications:
- introduce `ChannelShouldQuiesceUserEvent`
- handle `ChannelShouldQuiesceUserEvent` in the `AcceptHandler` with
closing the server socket
- handle `ChannelShouldQuiesceUserEvent` in the
`HTTPServerPipelineHandler` by only handling a already in-flight
request and then no longer accepting input
- added `CircularBuffer.removeAll` (& tests)
- added tests for `nextPowerOf2()`
Result:
- handlers can now support quiescing
Motivation:
To detect if keepalive is used we need to search the headers (if HTTP/1.1 is used). This may be expensive depending how many headers are present. http_parser itself detects already if keep alive should be used and so we can re-use this as long as the user did not modify the headers.
Modifications:
Reuse keepalive parsed by http_parser as long as the headers were not modified.
Result:
Less overhead as long as the headers are not modified.
Motivation:
In a test we did `assumingMemoryBound(to: in6_addr.self)` for something
that was actually bound to `UInt8`. So we should use
`bindMemory(to: in6_addr.self, count: 1)` instead
Modifications:
replace one `assumingMemoryBound` by `bindMemory`
Result:
more memory correctness is good even for tests
Motivation:
When the ByteToMessageDecoder re-entranced in its decode(...) / decodeLast(...) methods it could be possible that the ordering of processing and so the seen bytes are mixed.
Modifications:
- Refresh the buffer (and take a slice) on each loop iteration and update the cumulationBuffers indicies.
Result:
More robust code.
Motivation:
Very unfortunately [SR-7602](https://bugs.swift.org/browse/SR-7602)
Swift's String doesn't work very well on UTF-8 encoded data. Fortunately
(but unfairly see point (3) in
[SR-7602](https://bugs.swift.org/browse/SR-7602)) however there is now a
contiguous String storage for ASCII-only Strings that _only the stdlib_
can access. To benefit from the stdlib's capabilities of enumerating the
bytes faster than we can, we should use UnsafeMutableBufferPointer#initialize(from: Bytes).
Thanks @airspeedswift for pointing this out.
The performance gains on a recent Swift version are noticeable:
after this patch:
```
no-net_http1_10k_reqs_1_conn: 0.225782990455627, 0.228317022323608, 0.225206971168518, 0.22549307346344, 0.223728060722351, 0.225827932357788, 0.222437977790833, 0.221745014190674, 0.220004916191101, 0.222965002059937
http1_10k_reqs_1_conn: 0.688782095909119, 0.682757973670959, 0.689703941345215, 0.708670020103455, 0.691922068595886, 0.687381982803345, 0.681567072868347, 0.684866905212402, 0.683283090591431, 0.699701905250549
http1_10k_reqs_100_conns: 0.782313942909241, 0.779283046722412, 0.773630023002625, 0.776782989501953, 0.774596929550171, 0.775121927261353, 0.773631930351257, 0.784304022789001, 0.795854926109314, 0.786051988601685
```
before this patch:
```
no-net_http1_10k_reqs_1_conn: 0.232797980308533, 0.236240983009338, 0.237086057662964, 0.236070990562439, 0.236560940742493, 0.236254930496216, 0.234392046928406, 0.235393047332764, 0.234088897705078, 0.245710015296936
http1_10k_reqs_1_conn: 0.76228404045105, 0.788464903831482, 0.747326016426086, 0.753327012062073, 0.735787987709045, 0.727090001106262, 0.720335006713867, 0.721843004226685, 0.719871044158936, 0.728644967079163
http1_10k_reqs_100_conns: 0.841314911842346, 0.884468078613281, 0.854345917701721, 0.847344040870667, 0.847607016563416, 0.841101050376892, 0.814589977264404, 0.818693995475769, 0.832610964775085, 0.828818082809448
```
Modifications:
use UnsafeMutableBufferPointer#initialize(from: Bytes) to enumerate
String's bytes.
Result:
faster but still not fast enough, we need SR-7602 to be addressed
Motivation:
When discardReadBytes() was called and we still did not pass the Head to the user it was quite possible that the headers / URI could be corrupted as the stored readerIndex did not matchup anymore.
Modifications:
- Override most of the functionality of ByteToMessageDecoder in HTTPDecoder to better work with the provided state machine of http_parser
- Remove allocations by removing the pendingInOut Array as its not needed anymore.
- Add guards against re-entrance calls
- Add unit test that shows that everything works as expected now.
Result:
Fixed bug and reduced allocations (8% - 10% perf win).
Motivation:
Previously we would eagerly register any fd with the selector
(epoll/kqueue) which works well with kqueue. With epoll however, you'll
get `EPOLLHUP` immediately if you supplied a socket that hasn't had
connect/bind called on it.
We got away with this because we took quite a bit of care to always make
sure we call connect/bind pretty much immediately after we registered
it. And crucially before we ever asked the selector to tell us about
new events.
Modifications:
made selector registration lazy (when we try activating the channel by
calling connect/bind)
Result:
the user can now register and connect whenever they feel like it.
Motivation:
Once again, we had an extra event loop hop between a channel
registration and its activation. Usually this shows up as `EPOLLHUP` but
not so for accepted channels. What happened instead is that we had a
small race window after we accepted a channel. It was in a state where
it was not marked active _yet_ and therefore we'd not read out its data
in case we received a `.readEOF`. That usually leads to a stale
connection. Fortunately it doesn't happen very often that the client
connects, immediately sends some data and then shuts the write end of
the socket.
Modifications:
prevent the event loop hop between registration and activation
Result:
will always read out the read buffer on .readEOF
Motivation:
@vlm hit an interesting situation which is very likely the sign of
another bug that we have yet to find:
During a close, @vlm hit an error in close which lead to a user callout
which then closed again.
The immediate fix is simple (this PR) is as always not to call out to
the user before reconciling our own state. But hitting this bug also
means that either a `socket.close` or a `selectableEventLoop.deregister`
failed as we only called out in those situations. That definitely hides
a condition that is hard to imagine without another bug that we still
need to find.
Modifications:
in `BaseSocketChannel.close0` follow rule 0 and don't call out before
reconciling state.
Result:
fewer crashes, less potential to hide other bugs.
Motivation:
On Linux registration without bind/connect leads to `EPOLLHUP` and in
tests we often hit that race. This fixes one more instance of that.
Modifications:
Make sure the bind happens before we enter the selector.
Result:
more stable tests, fixes#378.
Motivation:
Warnings are bad.
Modifications:
Changed things that still work in 4.0/4.1 but are warnings in 4.2 to
improved versions.
Result:
less warnings on the upcoming 4.2
Motivation:
We can't nil out the connectPromise before we fail it. This totally
slipped through because we never fully waited for it because of the
reasons below.
It's quite easy to misuse `eventLoop.submit` as if you do some
asynchronous operation (will return `EventLoopFuture<T>`) the result of
the submit will be `EventLoopFuture<EventLoopFuture<T>>`. So if you just
do
try eventLoop.submit {
return somethingAsynchronous
}.wait()
then you will have only waited for the asynchronous operation to have
started (it might still be running). Fortunately, the compiler usually
warns you about that unless it returns `Void`. So it naturally tells you
to write
try eventLoop.submit {
return somethingAsynchronous
}.wait().wait()
which will wait for both. Unfortunately, `XCTAssertNoThrow` will just
eat away any type so there it's advisable to write
XCTAssertNoThrow(try eventLoop.submit { ... }.wait.wait() as Void)
to be sure it's actually a void. The Swift compiler however has an
annoying bug where it won't always realise that that's a problem
(https://bugs.swift.org/browse/SR-7478). Fortunately the 4.1 release
behaves okay so CI will catch it.
Modifications:
- replace the bad pattern
_ = try el.submit { ... }
- added `as Void`s inside `XCTAssertNoThrow`s that do submit
- added missing `.wait()`s
Result:
tests should be more correct.
Motivation:
When debugging, I often use ByteBuffer.debugDescription which dumps
the bytes. Unfortunately, the format wasn't compatible with what
`xxd -r -p` expects. Now you can just copy some output, for example
echo '04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12' | xxd -r -p
and xxd -r -p will correctly parse the byte values.
Modifications:
made BB.debugDescription prefix byte values under 0x10 with a 0 instead
of a space.
Result:
easier debugging with the standard tools
Motivation:
Unfortunately as a result of #108 the WebSocketFrameDecoder can emit a
WebSocketFrame more than once if the user closes the connection in
channelRead, or any other callback while decode() is on the call stack.
This is obviously less than ideal, as it can allow multiple delivery of
frames.
Modifications:
Given WebSocketFrameDecoder a no-op implementation of decodeLast.
Result:
No multi-delivery of frames.
Motivation:
We should not need to trigger any copy when the ByteBuffer was fully consumed and discardReadBytes() is called.
Modifications:
- If fully consumed just reset reader and writer Index.
- Add unit test
Result:
Less memory copies
Motivation:
When compiling the tests the following warning is printed:
swift-nio/Tests/NIOTests/SelectorTest.swift:361:43: warning: result of call to 'wait()' is unused
}.wait().forEach { return try! $0.wait() } as Void)
Modifications:
Use XCTAssertNoThrow to silence the warnings.
Result:
No more warnings.
Motivation:
In the new CI, ChannelTests.testGeneralConnectTimeout often fails with
```
error: ChannelTests.testGeneralConnectTimeout : threw error "connection reset (error set): Network is unreachable (errno: 101) "
```
It's not a great test but this just works around it by saying that if
the network's down that's fine too.
Modifications:
also accept ENETDOWN and ENETUNREACH
Result:
tests should pass in new CI.
Motivation:
HTTP{Request,Response}Head were too large even with a less than 3 word
ByteBuffer, this shrinks them box CoW boxing them. Should also reduce
ARC traffic when passing around a bunch.
Modifications:
CoW box HTTP{Request,Response}Head
Result:
fewer existential containers created, more performance
Motivation:
NIO's dynamic pipeline comes with one performance caveat: If your type
isn't specialised in NIOAny (only types in the NIO module can be) and
it's over 3 words (24 bytes on 64-bit platforms), you'll suffer an
allocation for every time you box it in NIOAny. Very unfortunately, both
ByteBuffer and FileRegion were exactly 3 words wide which means that any
enum containing those would be wider than 3 words. Worse, both
HTTP{Request,Response}Head contained types that were wider than 3 words.
The best solution to this problem is to shrink ByteBuffer and FileRegion
to just under 3 words and that's exactly what this PR is doing. That is
slightly tricky as ByteBuffer was already bit packed fairly well
(reader/writer indices & slice begin/end were stored as a UInt32). The
trick we employ now is to store the slice beginning in a UInt24 and the
file region reader index in a UInt56. That saves one byte for both
ByteBuffer and FileRegion with very moderate tradeoffs: The reader index
in a file now needs to be within 64 PiB (peta bytes) and a byte buffer
slice beginning must start within 16 MiB (mega bytes). Note: The
reader/writer indices as well as slice ends are _not_ affected and can
still be within 4 GiB. Clearly no one would care about the restrictions
for FileRegions in the real world but we might hit the ByteBuffer slice
beginning limit in which case the slice would be copied out. But
given that slices are mostly used to slice off headers in network
protocols, 16 MiB should be _plenty_.
Norman was kind enough to measure the perf differences:
master (before this PR):
```
$ wrk -c 256 -d 10s -t 4 -s ~/Downloads/pipeline-many.lua -H "X-Host: SomeValue" -H "Host: swiftnio.io" -H "ThereAreEvenMoreHeaders: AndMoreValues" http://localhost:8888
Running 10s test @ http://localhost:8888
4 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 93.02ms 158.13ms 1.97s 93.07%
Req/Sec 112.79k 26.05k 258.75k 84.50%
4501098 requests in 10.04s, 214.63MB read
Socket errors: connect 0, read 111, write 0, timeout 89
Requests/sec: 448485.61
Transfer/sec: 21.39MB
```
after this PR:
```
$ wrk -c 256 -d 10s -t 4 -s ~/Downloads/pipeline-many.lua -H "X-Host: SomeValue" -H "Host: swiftnio.io" -H "ThereAreEvenMoreHeaders: AndMoreValues" http://localhost:8888
Running 10s test @ http://localhost:8888
4 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 107.53ms 206.56ms 1.99s 90.55%
Req/Sec 124.15k 26.56k 290.41k 89.25%
4952904 requests in 10.03s, 236.17MB read
Socket errors: connect 0, read 161, write 0, timeout 22
Requests/sec: 493852.65
Transfer/sec: 23.55MB
```
so we see a nice 10% improvement
Modifications:
- shrank ByteBuffer by 1 byte, making it 23 bytes in total
- shrank FileRegion by 1 byte, making it 23 bytes in total
- added @_inlineable to NIOAny where appropriate to not suffer boxed existentials in different places
- added tests for the new edge cases
Result:
- more speed
- fewer allocations
Motivation:
Its often useful to be able to add / retrieve elements on both sides of a CircularBuffer.
Modifications:
- Add last / prepend to be able to act on both ends.
- Add tests
Result:
Fixes https://github.com/apple/swift-nio/issues/279
Motivation:
The pipelining handler made the assumption that `channelRead` is never
called recursively. That's mostly true but there is at least one
situation where that's not true:
- pipelining handler seen a response .end and delivers a .head (which is
done in `channelRead`)
- a handler further down stream writes and flushes some response data
- the flushes fail which leads to us draining the receive buffer
- if the receive buffer contained more requests, the pipelining
handler's `channelRead` is called again (recursively)
The net result of that was that the new request parts from the receive
buffer would now jump the queue and go through the channel pipeline
next, before other already buffered messages.
Modifications:
made the pipelining handler buffer if a `channelRead` comes in from the
pipeline and there is already at least one message buffered.
Result:
the ordering of the incoming messages should now be respected which is
very important...
Motivation:
Often its useful to be still be able to access the local / remote address during channelInactive / handlerRemoved callbacks to for example log it. We should ensure its still accessible during it.
Modifications:
- Fallback to slow-path in ChannelHandlerContext.localAddress0 / remoteAddress0 if fast-path fails to try accessing the address via the cache.
- Clear cached addresses after all callbacks are run.
- Add unit test.
Result:
Be able to access addresses while handlers are notified.
Motivation:
There were two errors in testShuttingDownFailsRegistration that
could cause spurious test failures.
The first was that the test did not wait for connection establishment
before it began shutting the selector down, due to a change made in
The second was that the server side of the connection was not wedged open,
and so it would be closed by the call to loop.closeGently. That closure
could, if the main thread took long enough, lead to the client side
reading EOF and being forcibly closed.
Modifications:
Added a promise to wait for connection establishment.
Added support for wedging both the server and client side open.
Result:
Fewer test failures.
Motivation:
We should use power of two for the underlying storage to allow us to make use of bitmasking which is faster then modulo operations.
Modifications:
Use power of two and replace module with bitmasking.
Result:
Faster implementation of CircularBuffer.
Motivation:
Since forever we had a major bug in the Selector: In this condition:
- kqueue/epoll had many events
- in one of the earlier events we unregister a Channel whose fd is on of
the later events
- we subsequently (still in the same event loop tick) register a new
channel which gets the same fd as the previously closed one
then we would deliver an event that was meant for a previous channel to
a newly opened one.
Thanks to @mcdappdev for hitting this bug, helping us debug it and also
providing a repeatedly working repro.
Modifications:
if during event delivery any fd gets unregistered, we stop delivering
the remaining events and rely on the selector to redeliver them
again next time.
Result:
we don't deliver events for previously closed channels to new ones.
Motivation:
When a pending connect is still in process and the Channel is closed we need to ensure we use the correct ordering when notify the promises and ChannelHandlers in the pipeline.
The ordering should be:
- Notify pending connect promise
- Notify close promise
- Call channelInactive(...)
Modifications:
- Correct ordering of notification
- Add unit test
Result:
Ensure correct ordering when connect is still pending
Motivation:
In some contexts it is important to be able to pass file descriptors
around instead of specifying the particular host and port. This allows removing
certain windows for race conditions, and has positive security implications.
Modifications:
Added `withConnectedSocket(_:)` call to the `ClientBootstrap` that can
be used to create a new `Channel` out of existing file descriptor
representing the connected socket.
Result:
Allows initializing `Channel`s off the existing file descriptors.
Motivation:
If a server binds "localhost", and then we try to connect with an
AF_INET socket, then on macOS at least some of the time we will have
a protocol mismatch, as at least some of the time localhost will have
resolved to ::1 before 127.0.0.1.
That's bad.
Modifications:
Create a socket whose address family matches the server's.
Result:
This test will pass more often
Motivation:
Since #286, we do eagerly detect connection resets which is good. To
resolve the situation we first try to read everything off the socket. If
something fails there, everything is good and the client gets the
correct error. If that however doesn't lead to any errors, we would
close the connection with `error: ChannelError.eof` which the client
would then see for example on a connect that happened to get
`ECONNRESET` or `ECONNREFUSED`. That's not right so these situations are
fixed here.
Modifications:
- on reset, try to get the socket error if any
- if no socket error was detected, we send a `ECONNRESET` because we
know we have been reset but not why and didn't encounter any errors
- write tests for these situations
Result:
connection resets that don't hit any other errors will now see the
correct error
Motivation:
Opening a file, seeking it or querying its size (or other information)
is blocking on UNIX so `NonBlockingFileIO` should support that too.
Modifications:
Added a method to `NonBlockingFileIO` which lets the user open a file
without blocking the calling thread.
Result:
Less blocking is good :)
Motivation:
When a connect() call returns an error other than EINPROGRESS, we
currently leave the FD registered and don't close the channel. This is
wrong, we should take this as a signal to close the socket immediately,
rathern than letting the selector tell us this FD is dead: after all,
we know it's dead.
Modifications:
Call `close0()` in synchronous connect failures.
Result:
Faster failures!
Motivation:
PR #286 started processing EPOLLHUPs. Sadly epoll also immediately
throws EPOLLHUP at you if you register a socket that isn't either
connected or listening. This fixes two more instances of this problem.
Modifications:
made sure sockets that are registered are either listening or
connected (and before we let the Selector wait for events)
Result:
tests should be more reliable
Motivation:
When implementing a custom ChannelCore, you will probably need access
to the data inside a NIOAny. It should be possible to unwrap that.
Modifications:
Added an extension to ChannelCore to allow unwrapping a NIOAny.
Added @_versioned to almost all of NIOAny.
Result:
ChannelCore is implementable outside NIO
* Allow to specify the max websockets frame size when using the upgrader.
Motivation:
At the moment its not possible to adjust the max websockets frame size when using the upgrader while its possible when directly use the WebSocketFrameDecoder.
Modifications:
Allow to specify max frame size via an init argument (with default value).
Result:
More flexible way to use the upgrader.
Motivation:
We had a number of problems:
1. We wanted to lazily process input EOFs and connection resets only
when the user actually calls `read()`. On Linux however you cannot
unsubscribe from `EPOLLHUP` so that's not possible.
2. Lazily processing input EOFs/connection resets wastes kernel
resources and that could potentially lead to a DOS
3. The very low-level `Selector` interpreted the eventing mechanism's
events quite a lot so the `EventLoop`/`Channel` only ever saw
`readable` or `writable` without further information what exactly
happened.
4. We completely ignored `EPOLLHUP` until now which on Unix Domain
Socket close leads to a 100% CPU spin (issue #277)
Modifications:
- made the `Selector` interface richer, it now sends the following
events: `readable`, `writable`, `readEOF` (input EOF), `reset`
(connection reset or some error)
- process input EOFs and connection resets/errors eagerly
- change all tests which relied on using unconnected and unbound sockets
to user connected/bound ones as `epoll_wait` otherwise would keep
sending us a stream of `EPOLLHUP`s which would now lead to an eager
close
Result:
- most importantly: fix issue #277
- waste less kernel resources (by dealing with input EOFs/connection
resets eagerly)
- bring kqueue/epoll more in line
Motivation:
We had a bug which is happens in the combination of these states:
- we held a request in the pipelining handler (because we're procesing a
previous one)
- a http handler error happened whilst a response's `.head` had already
been sent (but not yet the `.end`)
- the HTTPServerProtocolErrors handler is in use
That would lead to this situation:
- the error isn't held by the pipelining handler
- the error handler then just sends a full response (`.head` and `.end`)
but the actual http server already send a `.head`. So all in all, we
sent `.head`, `.head`, `.end` which is illegal
- the pipelining handler didn't notice this and beause it saw an `.end`
it would send through the next requst
- now the http server handler is in the situation that it gets `.head`,
`.head` too (which is illegal)
Modifications:
- hold HTTP errors in the pipelining handler too
Result:
- more correctness
Motivation:
Its very likely that the response will be HTTP/1.1 or HTTP/1.0 so we can optimize for it by minimizing the buffer.write(...) calls.
Beside this we should also change the pattern of *.write(into: inout ByteBuffer) to extension on ByteBuffer itself.
Modificiations:
- Optimize for HTTP/1.0 and 1.1
- Use extensions on ByteBuffer
Result:
Faster and more clean code.
Motivation:
HTTP message framing has a number of edge cases that NIO currently does
not tolerate. We should decide what our position is on each of these edge
cases and handle it appropriately.
Modifications:
Provide an extensive test suite that codifies our expected handling of
these edge cases. Fix divergences from this behaviour.
Result:
Better tolerance for the weird corners of HTTP.
Motivation:
Callouts to user code allows the user to make calls that re-enter
channel code. In the case of channel closure, we could call out to the
user before the channel knew it was completely closed, which would
trigger a safety assertion (in debug mode) or hit a fatalError
(in release mode).
Modifications:
Reconciled channel state before we call out to user code.
Added a test for this case.
Result:
Fewer crashes, better channel state management.
Motivation:
When an asynchronous connect failed we would crash because we didn't
correctly close the connection to start tearing everything down.
That correctly made the `SocketChannelLifecycleManager` trip.
This was filed as #302.
Modifications:
Correctly deregister and close the connection if an async connect fails.
Result:
Fewer crashes, fixes#302.
Motivation:
If we not filled the whole buffer with read(...) we should stop reading and wait until we get notified again. Otherwise chances are good that the next read(...) call will either read nothing or only a very small amount of data.
Also this will allow us to call fireChannelReadComplete() which may give the user the chance to flush out all pending writes.
Modifications:
Stop reading and wait for next wakup when we not fill the whole buffer.
Result:
Less wasted read calls and early chance of flushing out pending data.
Motivation:
We are currently parsing each header eagly which means we need to convert from bytes to String frequently. The reality is that most of the times the user is not really interested in all the headers and so it is kind of wasteful to do so.
Modification:
Rewrite our internal storage of HTTPHeaders to use a ByteBuffer as internal storage and so only parse headers on demand.
Result:
Less overhead for parsing headers.
* Add EventLoopFuture reduce(initialResult:) function (#240)
Motivation:
Add a new function to allow us to work with multiple EventLoopFutures, that return values, by combining their results and wrapping it in a new
EventLoopFuture, using a specified reducer function.
Modifications:
- Add a new function
EventLoopFuture<T>.reduce<U>(initialResult:futures:eventLoop:nextPartialResult)
- Changed the implementation of andAll<Void> to use
EventLoopFuture<Void>.reduce(initialResult)
- Add a new function EventLoopFuture<T>.reduce<U>(into:)
Result:
Can reduce multiple EventLoopFutures without having to deal with
creating EventLoopFutures.
Simplified EventLoopFuture andAll<Void>
- An API which can be used to construct collections from future values in a linear time.
Motivation:
HTTPHeaders had an unusual API for Swift: `getCanonicalForm(String)` which is
better suited as a subscript `[canonicalForm: String]`.
Also the implementation was needlessly inefficient.
Modifications:
- renamed HTTPHeaders.getCanonicalForm to HTTPHeaders[canonicalForm]
- improved efficiency by replacing anti-pattern
Array.map { ... }.reduce(+, []) by flatMap
Result:
more Swift in both meanings
Motivation:
Unfortunately we cannot guarantee that a weak ref is going to be nil in
a timely fashion if a strong reference to the object exists in another
thread. This makes us fail tests on slow machines, which is bad.
Modifications:
Wait for up to a second for the other thread to get its house in order
before we give up.
Result:
Hopefully I'll never have to "fix" this test again.
Motivation:
Currently the HTTP decoders can throw errors, but they will be ignored
and lead to a simple EOF. That's not ideal: in most cases we should make
a best-effort attempt to send a 4XX error code before we shut the client
down.
Modifications:
Provided a new ChannelHandler that generates 400 errors when the HTTP
decoder fails.
Added a flag to automatically add that handler to the channel pipeline.
Added the handler to the HTTP sample server.
Enabled integration test 12.
Result:
Easier error handling for HTTP servers.
Motivation:
Our HTTP code handles only HTTP/1.X. There is no reason to support
HTTP/0.9, and we cannot safely handle a major protocol higher than 1 in
this code, so we should simply treat requests/responses claiming to be
of those protocols as errors.
Modifications:
HTTPDecoder now checks the major version is equal to 1 before it
continues with parsing. If it hits an error, that error will be propagated
out to the user.
Result:
Better resilience against bad HTTP messages.
Motivation:
Add primitive for folding futuristic values.
Modifications:
Add a new EventLoopFuture extension containing the implementation of
`fold<U>(futures:combiningFunction)`.
Result:
An API for foding futures.
Motivation:
The current test function does not wait for the futures from close to
return, and so they may in principle be leaked. This obscures errors that
the test might hit with precondition failures.
Modifications:
Changed the defer statements to wait for the futures.
Result:
Fewer spurious test failures.
Motivation:
Adding @convention(c) is beneficial for performance of variables bound
to C functions so we should do that in ByteBuffer.
Modifications:
Adds @convention(c) to the malloc/realloc/free constants in ByteBuffer
Result:
Slightly better performance.
Motivation:
The channel leak detection test currently gates its progress based on
the firing of the promise passed to close(). That promise fires way, way
before the channel stops scheduling work, which means that it's highly
likely the event loop will still be holding a ref to the channel shortly
after that promise fires.
That causes the test to fail. Failing tests are bad.
Modifications:
Wait for the closeFuture instead. This fires at the end of the last
callback a socket channel ever schedules, so will fire just before the
channel drops out of scope.
Result:
Less flaky tests.
Motivation:
When calling Selector.whenReady(...) and using a SelectorStrategy.blockUntil(...) we may end up re-arm the timerfd each time even if the wakeup time did not change.
This is expensive and can be avoided in most situations.
Modifications:
Keep track of the earliest scheduled timer and only schedule a new one if it would fire sooner.
Result:
Less overhead when using epoll and timers.
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:
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:
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 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.
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:
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:
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:
`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 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:
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:
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:
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:
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:
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.
Motivation:
We not always ensure correct ordering which made it quite hard to reason about things. We should always first notify the promise before we call the fire* method that belongs to the event. Beside this we sometimes fired events or notified promised before correctly update the active state / addresses of a Channel which could result in unexpected results when query the Channel during execution of callbacks and handlers.
Modifications:
- Ensure we always notify promise first
- Always correctly update channel state before notify promise
- Add test to verify notification order.
Result:
Correct ordering of events which makes things easier to reason about and which follows what netty is doing.
Motivation:
Testing specific ordering behaviours (e.g. cancelling tasks) requires
that EmbeddedEventLoop behave as similarly as possible to the full
event loop.
Modifications:
Adopt the batched scheduled task dispatch logic from the regular
event loop.
Result:
Easier to test subtle behaviours around task dispatch.
Motivation:
320561f437 added a few tests which missed to correctly shutdown the EventLoopGroup
Modifications:
Add code to shutdown group
Result:
Correctly release resources during tests.
Motivation:
When the SelectableEventLoop is shutdown it will process a defer block in which we tried to fail all scheduled tasks that were not processed yet. The problem here was that we incorrectly not dropped the processed task and so ended in an endless loop which prevent the defer block to complete.
Beside this how we processed the scheduled tasks that were ready for execution was quite in-efficient as we always called removeFirst() which will most likely case a memory copy to move the elements around.
Modifications:
- Correctly fail all scheduled tasks without end up in an endless loop
- Use a for each loop to reduce overhead of processing ready tasks.
Result:
It's always possible to shutdown a SelectableEventLoop and less overhead when processing ready tasks.
Motivation:
A somewhat common requirement when working with chains of futures
is needing to hop from one event loop to another. This is particularly
common when relying on the fact that EventLoopFuture will synchronize
with the event loop that created it: you often want to rely on that
implicit locking, rather than use Dispatch or Lock to achieve the
same effect.
While doing this hop requires relatively little code, it's not
necessarily totally apparent to new users how they would do it.
Additionally, the most naive implementation incurs the overhead of
allocations and reference counting in cases where it's not necessary
(e.g. when you have only one event loop, or when both work items are
being scheduled on the same event loop).
For this reason, we should have a nice concise way for a user to
request this behaviour and get a relatively performant implementation
of the behaviour.
Modifications:
Added EventLoopFuture<T>.on(eventLoop:).
Changed AcceptHandler to use the new method rather than its (slower)
alternative.
Result:
Users will have an easier time working with EventLoopFutures.
Motivation:
EventLoopFuture.and had serious threading issues if the EventLoops
weren't the same.
Modifications:
Fixed the threading issues and tested them properly.
Result:
Hopefully `and` and `andAll` now don't crash if you use them across
EventLoops.
Motivation:
We did not verify that fireErrorCaught(...) was called for recoverable errors while accept new sockets.
Modifications:
Verify errorCaught is called.
Result:
Better test.
Motivation:
We should not close the DatagramChannel when recvfrom fails with ECONNREFUSED / ENOMEM. The former may be caused by a previous sendto and the other may be recoverable by just stop reading for some time.
Modifications:
- Special handle ECONNREFUSED and ENOMEN to not close the channel.
- Add unit tests
Result:
More robust datagram support.
Motivation:
For servers NIO had a race where when you start tearing down the
`ServerSocketChannel` and at roughly the same time a client connects, we
would not fully bring that channel up, nor tear it down. That lead to
open channels being deallocated which leads to a crash as that's an
illegal state.
Modifications:
Made sure newly accepted channels get closed if the
`ServerSocketChannel` is already clsing.
Result:
No crashes anymore when a client connects to a closing server.
Motivation:
We returned port numbers directly from the sockaddr structures but
they're in network byte order. Our code only read them correctly when
presenting, oops.
Modifications:
Corrected the port number reading and tested that.
Result:
Port numbers should now be correct.
Motivation:
Often accept errors are recoverable over time as for example you may just run out of file descriptors. Because of this it may be useful to just "delay" accepts a bit to recover.
Modifications:
Add ChannelHandler which will backoff accepts.
Result:
Be able to recover from accept errors gracefully.
Motivation:
At the moment its possible that channelRead is triggered even if autoRead is never changed to true and no read() is triggered.
Modifications:
- Always register with .none on the Selector and then just call readIfNeeded0()
- Only register for EPOLLRDHUP if .read, .write or .all is used.
- Add test case.
Result:
Fixes https://github.com/apple/swift-nio/issues/160.
Motivation:
The current HTTP server helpers are not modular, meaning that each
time we add a new first-party HTTP handler we need to add a brand new
method. Additionally, they do not currently compose, so if you want
assistance with HTTP pipelining *and* to support HTTP upgrade you are
out of luck.
Modifications:
Deprecate the two existing server helpers.
Add a new server helper that can be extended to support all of the
possible features that users may want in their HTTP pipelines.
Result:
Users will have a single place to go that can be used to configure
their HTTP server pipeline, and that understands how the different
handlers fit together in a complete pipeline.
Motivation:
In certain cases it would be useful to be able to access the current EventLoop (which is tied to a thread).
Modifications:
- Add MultiThreadEventLoopGroup.currentEventLoop
- Add tests
Result:
Fixes [#59]
Motivation:
The HTTP pipeline may contain other HTTP protocol specific handlers,
beyond just the ones that the upgrade handler knows explicitly. It should
be possible to get the upgrade handler to remove those at the right time.
Modifications:
Add extra support for removing handlers.
Result:
It'll be possible for the HTTP upgrade logic to remove things like the
pipelining handler at upgrade time.
Motivation:
It is often useful to find a handler of a given type even when you
aren't holding a reference to that handler. This patch allows us to
do that.
Modifications:
Add context(handlerType:) function to the channel pipeline.
Result:
Users can find handlers of a given type in a pipeline.
Motivation:
In a few instances, we have code that ignores a `()` argument in a
closure with this syntax:
{ (_: Void) in
whilst that's a bit odd, it shouldn't be wrong and Swift 4.0.x is also
totally happy with it. The latest Swift 4.1 candidates however are not
and also there's a better way of spelling this:
{ () in
and in some cases even just
{
Modifications:
Removed instances of `{ (_: Void) in`
Result:
Compiles with `swift-4.1-DEVELOPMENT-SNAPSHOT-2018-03-12-a`
Motivation:
Websockets is a major protocol in use on the web today, and is
particularly valuable in applications that use asynchronous I/O
as it allows servers to keep connections open for long periods of
time for fully-duplex communication.
Users of NIO should be able to build websocket clients and servers
without too much difficulty.
Modifications:
Provided a WebsocketFrameEncoder and Decoder that can serialize and
deserialize Websocket frames.
Result:
Easier use of websockets.
Motivation:
HTTP pipelining can be tricky to handle properly on the server side.
In particular, it's very easy to write out of order or inconsistently
mutate state. Users often need help to handle this appropriately.
Modifications:
Added a HTTPServerPipelineHandler that only lets one request through
at a time.
Result:
Better servers that are more able to handle HTTP pipelining
Motivation:
We should not close the server socket when accept fails with ECONNABORTED / EMFILE / ENFILE, ENOBUFS. ENOMEM as generally speaking these may be "recoverable" once some existing connections were closed for example. It's possible to implement a backoff strategy for these with a ChannelHandler.
Modifications:
- Special handle of ECONNABORTED / EMFILE / ENFILE, ENOBUFS. ENOMEM for ServerSocketChannel.
Result:
Not close servet socket for errors from which it is possible to recover.
Motivation:
Currently we only supporting adding channel handlers at the front
or the end of the pipeline. This is potentially a bit frustrating:
it becomes impossible to insert handlers relative to a specific
point in the pipeline, which limits users ability to code generic
pipeline modification operations.
Modifications:
Added two new methods to ChannelPipeline, specifically
add(handler:before:) and add(handler:after:). Also performed a
substantial internal refactoring of the ChannelPipeline handler
adding code to ensure that all operations go through the same
logical path, redefining add(handler:first:) in terms of adding
either before tail or after head.
Result:
Users will be able to insert channel handlers at any point in a
ChannelPipeline, so long as they are able to point at a handler
already in that pipeline.
Motivation:
Reduce memory footprint of BaseSocket and FileHandle
Modifications:
- made isOpen a computed var from 'descriptor >= 0'
- add withSocketpair to TestUtils
- use real pipe and socketpair file descriptors for tests
- BaseSocket and FileHandle init check precondition 'descriptor >= 0'
Result:
- isOpen no longer requires storage space
- close() sets descriptor to -1
- descriptor property changed from let to var
- tests will need to use valid file descriptors
Motivation:
We can cleanup the code by removing return keywords in closures sometimes.
Modifications:
Remove return when possible.
Result:
Cleaner code.
Motivation:
Right now addHandlers(first:) does not actually add handlers at the
front of the pipeline: it just adds them *backwards* at the end of
the pipeline. That's definitely not right.
Modifications:
Pass the first: flag into the call to add each specific handler to
ensure they're actually put at the front of the pipeline.
Result:
Handlers added to the front of the pipeline actually are.
Motivation:
Fix for issue #110, .description on IPv6 addresses
return a String with 0 codepoints in it, like so:
[IPv6]::1<NUL><NUL><NUL>....lots..
Modifications:
Use String(cString:) instead of decode, which doesn't
care about \0-cstr terminators.
Result:
Issue fixed, everything is awesome, everyone is happy.
Motivation:
It should be possible to have channels that are not registered
for any form of I/O without them getting stuck in that model
forever.
Modifications:
Remove the code that prevents channels registered for `.none` from
registering for anything else.
Result:
Channels can actually be registered for nothing without becoming
wedged open forever.
Motivation:
Sometimes getting read/getInteger to guess the type correctly can be
annoying and it's handy to just say `as: UInt8.self` or similar. In
other cases it's not as the type is clear from the environment. This now
enables both, just like we already have for `write/set(integer:)`
Modifications:
added an `as: T.Type = T.self` argument which is optional as it has a
default
Result:
easier to use `ByteBuffer` interfaces
Motivation:
We missed to correctly update the cached remote and local addresses for accepted Channels. Because of this localAddress and remoteAddr always returned nil.
Modifications:
- Update cached addresses when constructing SocketChannel from existing Socket.
- Add testcase
Result:
Fixes [#97].
Motivation:
The previous delayed upgrade test didn't really validate what it
claimed to be doing: it validated properties of the I/O, which made
it enormously flaky.
Modifications:
Change the tests to check for the presence/absence of the server
upgrade handler, which is a mark of the actual upgrade process.
Result:
Less flaky tests
Motivation:
When writing programs that use networks, particularly if you want to
perform multicast joins, it's extremely important to be able to query
network interfaces and get results.
Modifications:
Add support for querying interfaces using getifaddrs and wrap those query
results in a Swift class.
Result:
Users will be able to enumerate the interfaces on their machine.
Motivation:
Previously if we'd futureOnEL1.then { ... in futureInEL2 } we'd forget
to hop from EL1 to EL2 which is bad. Shout out to @tanner0101 for
reporting!
Modifications:
Hop from one EventLoop to another if needed.
Result:
thens between Futures of different EventLoops now correctly switch
EventLoops.
Motivation:
Sometimes a user needs to implement ChannelInboundHandler and ChannelOutboundHandler. We should remove some of the "boilerplate" code for doing so.
Modifications:
- Add ChannelDuplexHandler and use it.
Result:
Less boilerplate code.
Motivation:
The upgrade test would race with itself and occasionally fail.
Modifications:
Give enough time for all the work to settle before closing the
client channel.
Result:
No more flaky tests.
Motivation:
We'd go into an infinite loop when trying to increase the capacity of a
0 capacity ByteBuffer *doh*. Thanks @vlm for reporting and writing the
test.
Modifications:
made sure ensureAvailableCapacity returns at least 1 if we don't have
enough capacity available.
Result:
0 capacity ByteBuffers resize now.
Motivation:
FileHandle and BaseSocket should implement a common protocol as these are both "based" on file descriptors
Modifications:
- Add FileDescriptor protocol
- Implement it for FileHandler and BaseSocket
Result:
Better code structure
Motivation:
73a805c7f6 did some changes but missed to rename one variable. This broke the build on linux.
Modifications:
Rename fn to body.
Result:
Builds again on linux.
Motivation:
Sometimes we deviated from the style the Swift stdlib sets out for no
reason.
Modifications:
Fixed some stylistic deviations.
Result:
Looks more Swift-like.
Motivation:
At the moment EmbeddedChannel calls fireChannelRegistered() when connect(...) is called. This is not correct, as it should be done when register(...) is called.
Modifications:
- Correctly call `fireChannelRegistered(...)` when register(...) is called
- Simplify init of EmbeddedChannel
- Fix EmbeddedChannelTest
Result:
Correct invocation of methods.
Motivation:
ChannelLifecycleHandler should record .registered as lifecycle in channelRegistered and not .inactive
Modification:
- Correct record .registered
- Fix tests
Result:
Correctly record lifecycle state.
Motivation:
We no longer have flush promises, so the PWM no longer needs to
handle them.
Modifications:
Removed all flush promise handling from the PWM and associated
tests.
Result:
Less code, more clarity.
Motivation:
A substantial chunk of code complexity exists in the PDWM to manage
flush promises. This complexity is no longer justified now that we
don't bother with them.
Modifications:
Removed much of the complexity of the PDWM handling of flush promises.
Result:
Simpler, easier to understand code.
Motivation:
We used Range for WriteBufferWaterMark which did not do any validation of the low and high marks.
Modifications:
Make WriteBufferWaterMark a struct and validate low and high values
Result:
More correct impl for watermarks.
Motivation:
When the connect() call does not complete synchronously we were not
updating the peer IP address when it finally did complete. This meant
that any channel that didn't connect synchronously (in the real world,
almost all of them) would not correctly see the remote peer IP.
Modifications:
Update the cached addresses on delayed connect.
Result:
Connected channels will see the correct remote IP.
Motivation:
Currently the HTTPUpgradeHandler does not allow for the protocol
upgraders to return EventLoopFuture objects. This leads to an
awkward scenario: it's essentially required that an upgrader be able
to synchronously change the pipeline to its satisfaction and prepare for
more bytes.
This is not really possible: ChannelPipeline.add(handler:) returns a
Future, which means that it is possible for this operation, at least
in principle, to not execute synchronously. This reality puts the
HTTPUpgradeHandler at odds with the reality of channel pipelines, and
cannot stand.
Modifications:
Give the HTTPProtocolUpgrader protocol to have upgrade() return an
EventLoopFuture<Void>, and update the HTTPServerUpgradeHandler to
appropriately handle the returned future by buffering until the future
completes.
Result:
It is no longer necessary to synchronously change the channel
pipeline during protocol upgrade.
Motivation:
Right now the test gen script will rewrite every file. Not ideal!
Modifications:
Added the license header, and made the follow-on comment match that
style.
Result:
Test files will look the same when running this script as they do
now.
Motivation:
The HTTP upgrade handler should not require that we case-match with
the mandatory headers.
Modifications:
Treat all headers as lowercase.
Result:
Upgrade is easier.
Motivation:
guard !self.open else { ... } is a double negation and can be confusing,
guard self.isOpen else { ... } is much better
Modifications:
replaced all `var closed: Bool` with `var isOpen: Bool`
Result:
we're more positive
Motivation:
Out half-closure tests did not wait for the actual event to be received before trying to tear-down the Channel which could lead to test-failures if the event was received not fast enough. We need to ensure we wait until we receive the event before closing the Channel and assert the received events.
Modifications:
Ensure we wait for the events before we try to close the Channel and assert the received events.
Result:
Less flaky tests.
Motivation:
Previously we required the `ByteBuffer.set/write` methods to be a
`Collection` of `UInt8` but a `Sequence` is really enough.
Modifications:
Implemented `ByteBuffer.write/set` for `Sequence`s of `UInt8`
Result:
More stuff can be used with `ByteBuffer`
Motivation:
We have multiple reasons why flush promises weren't great, for example:
- writeAndFlush sounds like it's a write + flush optimisation but in
reality it was more expensive (2 extra promises)
- the semantics were never quite clear
- lots of implementation complexity, especially for the datagram channel
Modifications:
- removed the flush promises in the API
- this deliberately doesn't do the PendingWritesManager simplifications
that this unlocks
Result:
flush doesn't have a promise anymore.
Motivation:
Except for blocking methods (which shouldn't be used on the event loop),
there aren't that many error cases that we should just send through the
pipeline automatically. Best proof for that is that nothing in the code
base did that. Therefore these methods shouldn't be throwing.
If the user does indeed want to catch and fire an error through the
pipeline that's still super simple `ctx.fireErrorCaught(error)`.
Modifications:
removed `throws` from the `ChannelInboundHandler` methods.
Result:
better
Motivation:
ByteToMessageDecoder implementations can potentially have usage
patterns that never reclaim memory. This is problematic when used
for a long time, as they can be holding on to large chunks of memory
they don't need.
Modifications:
Gave ByteToMessageDecoder a new protocol method,
shouldReclaimBytes(buffer:), and a default implementation.
Result:
ByteToMessageDecoders will not be able to hold on to more than 2kB
of unreachable memory under any circumstances, and will often hold
less memory than that.
Motivation:
There were a couple of places in the Channel implementations that just
weren't thread-safe at all, namely:
- localAddress
- remoetAddress
- parent
those are now fixed. I also re-ordered the code so it should be easier
to maintain in the future (the `// MARK` markers were totally
incorrect).
Modifications:
- made Channel.{local,remote}Address return a future
- made Channel.parent a `let`
- unified more of `SocketChannel` and `DatagramSocketChannel`
- fixed the `// MARK`s by reordering code
- annotated methods/properties that are in the Channel API and need to
be thread-safe
Result:
slightly more thread-safety :)
Motivation:
We used a bool to signal if we should continue decoding or not. Using an enum is more self-documenting.
Modifications:
Add DecodingState and use.
Result:
Code is more self-documenting.
Motivation:
Don't make the system fall over under slowloris-style attacks.
Modifications:
Remove the quadratic copy loop from ByteToMessageDecoder.
Result:
NIO goes from 🚂 to 🚄
Motivation:
Foundation is problematic for a few reasons:
- its implementation is different on Linux and on macOS which means our
macOS tests might be inaccurate
- on macOS it uses ObjC Foundation which means the autorelease pool
might get populated
- it links the world on Linux which means we can't do static
binaries at all
Modifications:
removed the last bits of Foundation dependency
Result:
no Foundation dependency
Motivation:
For HTTP parsing, we used Foundation to trim whitespace which is not
needed.
Modifications:
Implemented whitespace trimming in Swift.
Result:
less Foundation
to not return a value
Motivation:
We recently had a bug where we had `EventLoopFuture<EventLoopFuture<()>>` which didn't make any sense. The compiler couldn't catch that problem because we just ignored a closure's argument like this:
future.then { _ in
...
}
which is dangerous. For closures that take an empty tuple, the `_ in`
isn't actually required and the others should state the type they want
to ignore.
And most whenComplete calls can be better (and often shorter) expressed
by other combinators.
Modifications:
remove pretty much all closures which just blanket ignore their
parameter.
Result:
- no closures which just ignore their parameter without at least stating
its type.
- rewrote all whenCompletes that actually used the value
Motivation:
We had a few return statements that could be removed and some places where trailing closure syntax could be used.
Modifications:
- Remove returns
- Use trailing closures
Result:
Cleaner code.
* Expose BlockingIOThreadPool
Motivation:
Sometimes we need to execute some blocking IO. For this we should expose the BlockingIOThreadPool that can be used.
Modifications:
- Factor out BlockingIOThreadPool
- Added tests
- Correctly start threadpool before execute NonBlockingIO tests.
Result:
Possible to do blocking IO.
* Corrys comment
* Correctly start pool before using it
Johannes Weiß