Use fatalError in all .wait() calls for WASI builds only, and point
developers towards .get() instead.
### Motivation:
While working to adopt NIO in some wasm code, I've commonly ran into
issues any time code calls `.wait()` during wasm runtime. Typically the
executable either traps or crashes any time `.wait()` is called with an
ambiguous error: `Uncaught (in promise) RuntimeError: Atomics.wait
cannot be called in this context`. The error occurs because it is
forbidden to block the main thread for a wasm executable, and the
current implementation of `.wait()` blocks the calling thread.
The fix is straight forward, all calls to `.wait()` need refactor to
`.get()`, which sometimes involves some Swift Concurrency adoption (eg.
`async`) in the process. That change avoids blocking the main thread.
### Modifications:
Added `fatalError` with a descriptive error message to help developers
identify the issue easier.
### Result:
For WASI builds only, changes the trap error message `Uncaught (in
promise) RuntimeError: Atomics.wait cannot be called in this context`
into the following error message instead:
```
NIO's wait() function should not be called on WASI platforms. It will freeze or crash. Use get() instead.
```
### Alternatives considered
- We could instead conditionalize `.wait()` completely out of nio for
WASI builds, to turn runtime errors into compiler errors. That makes
detection of this issue easier, but forces a refactor and breaking
change, and somewhat precludes the possibility that WASI might support
this down the road with a future change.
- We could add a deprecation for WASI only. But long term WASI may be
able to support this blocking call, so deprecation and removal is
communicating the wrong message if the future ends up being reality.
### Context
This PR is [part of a larger effort by
PassiveLogic](https://github.com/PassiveLogic/swift-web-examples/issues/1)
to move Swift for WebAssembly support forward in a large number of
dependencies.
Fixes all warnings when `-require-explicit-sendable` flag is enabled and
enables the flag on macOS CI.
### Motivation:
We want to ensure our public API is either explicitly marked as
`Sendable` or not.
### Modifications:
Marked appropriate public types as `Sendable`, or explicitly defined
their conformance to the `Sendable` protocol as unavailable.
### Result:
We can now enable `-require-explicit-sendable` compiler flag in our
codebase.
Motivation:
The compiler is better at optimising closures wrapped in nominal types
than raw closures when used as generic parameters. CallbackList is used
a lot and stores an optional closure as well as on optional array of
closures. Wrapping these internally should save some allocations.
Modifications:
- Wrap the element stored in the `CallbackList`.
Result:
Fewer allocs
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
### Motivation:
Whilst working on the NIOHTTP1 strict concurrency work I encountered a
few gaps in the API where I needed isolated analogues of methods on
futures.
### Modifications:
* Add `makeSucceededIsolatedFuture`
* Add a new internal initializer for `EventLoopFuture` which does not
require the value to be Sendable for use in isolated contexts.
* Add a variant of `flatMapError` which can handle closures which return
isolated futures
* Add a `futureResult` computed property which allows us to avoid the
otherwise clumsy `future.nonIsolated().futureResult.assumeIsolated()`
### Result:
More ability to work with isolated futures.
Motivation:
Unfortunately, closure composition is really expensive: closures that
capture closures always heap allocate. To make ELF.Isolated perform
well, then, we need to inline the method bodies directly.
Modifications:
- Add some isolated functions into ELF for enqueueing callbacks.
- Inline the implementation of the ELF methods into the isolated view.
Result:
Allocation counts match between isolated/nonisolated.
### Motivation:
SwiftNIO contained a bogus error message:
```
BUG in SwiftNIO (please report), unleakable promise leaked.:486: Fatal error: leaking promise created at (file: "BUG in SwiftNIO (please report), unleakable promise leaked.", line: 486)
```
The actual meaning is that the ELG was shut down prematurely.
### Modifications:
- Replace the message with an accurate one
### Result:
- Users less confused
- fixes#2855
- fixes#2201
### Motivation:
In many situations, for example continuous profiling with sampling
profilers, it's important to distinguish between on- and off-CPU work.
That's most easily done if there are clear function names/prefixes to
grep for that are common places to just wait (off-CPU) and do no real
work. In SwiftNIO those are chiefly three places:
1. The `EventLoops` waiting for work
2. `The NIOThreadPool` threads waiting for work
3. `EventLoopFuture.wait()`
This patch makes sure that each of those will have a function with
prefix `_blockingWaitFor` in the function name which is easily
greppable.
### Modifications:
- Create some non-inlinable functions with `_blockingWaitFor...` that
just wait for `...`.
### Result:
SwiftNIO makes operational excellence easier and your SRE team more
happy.
### Motivation:
Documentation checking catches more issues in Swift 6.0.
### Modifications:
Adopt the Swift 6.0 image and fix the errors.
### Result:
More accurate docs.
# Motivation
We need to tackle the remaining strict concurrency checking related
`Sendable` warnings in NIO. The first place to start is making sure that
`EventLoopFuture` and `EventLoopPromise` are properly annotated.
# Modification
In a previous https://github.com/apple/swift-nio/pull/2496, @weissi
changed the `@unchecked Sendable` conformances of
`EventLoopFuture/Promise` to be conditional on the sendability of the
generic `Value` type. After having looked at all the APIs on the future
and promise types as well as reading the latest Concurrency evolution
proposals, specifically the [Region based
Isolation](https://github.com/apple/swift-evolution/blob/main/proposals/0414-region-based-isolation.md),
I came to the conclusion that the previous `@unchecked Sendable`
annotations were correct. The reasoning for this is:
1. An `EventLoopPromise` and `EventLoopFuture` pair are tied to a
specific `EventLoop`
2. An `EventLoop` represents an isolation region and values tied to its
isolation are not allowed to be shared outside of it unless they are
disconnected from the region
3. The `value` used to succeed a promise often come from outside the
isolation domain of the `EventLoop` hence they must be transferred into
the promise.
4. The isolation region of the event loop is enforced through
`@Sendable` annotations on all closures that receive the value in some
kind of transformation e.g. `map()` or `whenComplete()`
5. Any method on `EventLoopFuture` that combines itself with another
future must require `Sendable` of the other futures `Value` since we
cannot statically enforce that futures are bound to the same event loop
i.e. to the same isolation domain
Due to the above rules, this PR adds back the `@unchecked Sendable`
conformances to both types. Furthermore, this PR revisits every single
method on `EventLoopPromise/Future` and adds missing `Sendable` and
`@Sendable` annotation where necessary to uphold the above rules. A few
important things to call out:
- Since `transferring` is currently not available this PR requires a
`Sendable` conformance for some methods on `EventLoopPromise/Future`
that should rather take a `transffering` argument
- To enable the common case where a value from the same event loop is
used to succeed a promise I added two additional methods that take a
`eventLoopBoundResult` and enforce dynamic isolation checking. We might
have to do this for more methods once we adopt those changes in other
targets/packages.
# Result
After this PR has landed our lowest level building block should be
inline with what the rest of the language enforces in Concurrency. The
`EventLoopFuture.swift` produces no more warnings under strict
concurrency checking on the latest 5.10 snapshots.
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
Dispatch is not supported on WASI, and only Unix domain sockets are
supported, which means we have to exclude those APIs on this platform.
There's work in progress to enable tests for this on CI, but nothing I
can provide for this PR at the current moment.
---------
Co-authored-by: Franz Busch <f.busch@apple.com>
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
Motivation:
Many operations accept an optional promise. It's not uncommon to batch
operations (which may each have their own promise) and complete them
as a single operation. Combining these optional promises is slightly
tedious.
Modifications:
- Add an extension to `Optional` to set or cascade a promise
- If a promise exists, its result is cascaded to the provided promise.
Otherwise the optional is set to the provided promise.
Result:
It's easier to combine optional promises.
* Bump minimum Swift version to 5.7
Motivation:
Now that Swift 5.9 is GM we should update the supported versions and
remove 5.6
Modifications:
* Update `Package.swift`
* Remove `#if swift(>=5.7)` guards
* Delete the 5.6 docker compose file and make a 5.10 one
* Update integration test script
* Update docs
Result:
Remove support for Swift 5.6, add 5.10
* fix indentation issues
* 5.9 docker image use release image
* added assertSuccess() and assertFailure()
* test functions for assertSuccess() and assertFailure()
* removed callbacks approach and add precondition variants
* added test for precondition variants
* self.always to handle future w/o creating a promise, added fileID and line for debug
* updated test for asserts on EventLoopFuture
Motivation:
Up until recently, it has not been possible to regression check our
documentation. However, in recent releases of the DocC plugin it has
become possible to make warnings into errors, making it possible for us
to CI our docs.
This patch adds support for doing that, and also cleans up our
documentation so that it successfully passes the check.
Along the way I accidentally wrote an `index.md` for `NIOCore` so I
figure we may as well keep it.
Modifications:
- Structure the documentation for NIOCore
- Fix up DocC issues
- Add `check-docs.sh` script to check the docs cleanly build
- Wire things up to our docker-compose scripts.
Result:
We can CI our docs.
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
ELF.wait() waits for a condition variable to become true, which can
frequently lead to extremely long waits. This is a bad thing to call on
a Swift Concurrency thread, especially as we have ELF.get() which is
preferable.
Modifications:
Make ELF.wait() unavailable from async.
Result:
Users are encouraged to use the correct primitive.
Motivation:
#fileID introduced in Swift 5.3, so no longer need to use #file anywhere
Modifications:
Changed #file to #filePath or #fileID depending on the situation
* Adopt `Sendable` for `EventLoopFuture` and `EventLoopPromise`
* use internal typealias to deduplicate method bodies
* fix `Sendable` warnings for nightly toolchains
* fix compliation for Swift 5.5 & 5.6
* Adopt `Sendable` only in Swift 5.7+
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
For NIO's 'promise leak detector' we added file:line: labels to
makePromise and there it makes sense. A user might create a promise and
then never fulfil it, bad. With the file:line: arguments we can give
good diagnostics.
However, we (probably that @weissi again) also added it to flatMap and
friends where it doesn't make sense at all.
Sure, EventLoopFuture's implementation may create a promise in the
implementation of flatMap but this promise is never leaked unless the
previous future is never fulfilled (or NIO has a terrible bug). Suffice
to say that in a future chain, it's never a flatMap etc which is
responsible for leaking the first promise...
Explain here the context, and why you're making that change.
What is the problem you're trying to solve.
Modifications:
Remove all unnecessary `file:line:` parameters whilst keeping the public
API intact.
Result:
More sensible code.
Motivation:
The next step on extracting the base abstractions is to pull out
EventLoop, EventLoopGroup and related types, and also EventLoopPromise
and EventLoopFuture. These types are fundamental to the API.
Modifications:
- Extract EventLoop, EventLoopGroup, and related types.
- Extract EventLoopPromise and EventLoopFuture.
- Add new @testable imports.
- Split MultiThreadedEventLoopGroup into new file, appropriately named.
Result:
Better separation.
scottmarchant
Cory Benfield
Rafael Cepeda
George BarnettandCory Benfield
Roman A
Rick Newton-Rogers
Johannes Weiss
Max DesiatovandFranz Busch
Gustavo Cairo
Rick Newton-Rogers
dkz2
Oleksandr ZhurbaandOleksandrZhurba
carolinacass
David Nadoba
BenedictSt
Adam Fowler