### Motivation:
In reviewing a copy of this file in swift-log, @czechboy0
[noticed](https://github.com/apple/swift-log/pull/398#discussion_r2685605800)
that the `mutex` variable is essentially unused and unnecessarily
allocated for certain conditions. It is possible the compiler elided
usage. But given the unconditional variable reference in the `deinit`,
the compiler may not be able to.
This change ensures the `mutex` property is completely elided except in
the conditions where it is used.
### Modifications:
- Adjusted compiler directive pattern to completely elide the `mutex`
property for conditions where it would be unused.
- Adjusted `deinit` to avoid referencing `mutex` unless it is compiled
into the `Lock` class
### Result:
The `mutex` property is no longer allocated or even compiled into the
`Lock` class for unsupported configurations. All unit tests and checks
pass.
### Research:
The `deinit` used to have a blanket call to `mutex.deallocate()`. This
has been moved into the platform-specific checks within the `deinit`.
This allows complete elision of the `mutex` property altogether. The
`_runtime(_multithreaded)` condition [appears to be rooted to this
implementation](https://github.com/swiftlang/swift/blob/ffc51b914602765c5d680241796dbd3c3711fa6b/lib/Basic/LangOptions.cpp#L490).
Diving deeper, all [operating systems in this
list](https://github.com/swiftlang/swift/blob/ffc51b914602765c5d680241796dbd3c3711fa6b/lib/Basic/LangOptions.cpp#L554)
except for UnknownOS and WASI result in `_runtime(_multithreaded)`
returning true. That means that `OpenBSD` (and many other operating
systems besides windows) were almost certainly using the `#elseif
(compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) &&
_runtime(_multithreaded))` condition in the `deinit`. In conclusion,
moving `mutex.deallocate()` up into the conditional clauses inside
`deinit` should be identical to the previous implementation for all
operating systems except `llvm::Triple::UnknownOS`. And
`llvm::Triple::UnknownOS` is likely not to have every compiled anyways
due to the [check on line
32](https://github.com/apple/swift-nio/blob/main/Sources/NIOConcurrencyHelpers/lock.swift#L32).
So in summary, it is expected that this clean up will compile identical
to the previous implementation for all platforms except WASI platforms
that don't have pthread support. For non-pthread WASI platforms, the
unused `mutex` is properly elided from compilation altogether.
### Testing Done:
- Confirmed that unit tests pass locally using Xcode
- Verified [PR checks
pass](https://github.com/PassiveLogic/swift-nio/actions/runs/21149729751)
As observed by @kkebo, this was missing from the implementation.
### Motivation:
Correct pthread API usage.
### Modifications:
Initialize the pthread_mutexattr_t with pthread_mutexattr_init before
passed to pthread_mutex_init.
### Result:
Even though this lock API appears to be deprecated, we'll correct the
usage anyway. The non-deprecated NIOLock class doesn't have this
problem.
### Motivation:
In reviewing a copy of this file in swift-log, @czechboy0 noticed that
the call to `pthread_mutexattr_destroy` is missing. Since
pthread_mutexattr_t is like allocated on the stack rather than heap on
most platforms, this is likely not a memory leak. But for correctness
the missing destroy call should be added.
This change is the start of a resolution
[requested](https://github.com/apple/swift-log/pull/398#discussion_r2689459360)
in a corresponding Lock rollout in swift-log
### Modifications:
- Added missing call to `pthread_mutexattr_destroy`
- Duplicated test for `NIOLock` to also test `Lock`, as a sanity test.
### Result:
The new sanity test passes. If there are other ways this change can be
realistically verified as safe and proper, please feel free to provide
feedback in the PR.
### Motivation:
Additional partial platform support.
### Modifications:
* Create a new CNIOBSD module for OpenBSD, for general cleanliness, and
make use of it throughout. Some of the changes are borrowed directly
from CNIOLinux; some may technically be unnecessary; I am erring
somewhat on expediency to functionality.
* Since `malloc_size` is unavailable on OpenBSD, and some of the helpers
make use of ManagedBufferPointer which makes use of it, mark some of
this as unavailable as well.
* Usual pthread optional typing changes, since pthread types are
pointers on this platform.
* Add conditionals to exclude other functionality not available here,
like IP_RECVPKTINFO or IP_PKTINFO.
* d_ino is a backwards-compatibility macro valued token for dirent, so
instead, just expand with a conditional.
* Use kqueue on OpenBSD. This necessitates adding some conditionals for
type and feature compatibility.
* The vsock API is unavailable on OpenBSD.
### Result:
Tested the NIOTCPEchoClient and NIOTCPEchoServer appears to work and
that swift-nio-ssh (hopefully wlog) builds with a local repository with
these changes.
Because NIOFS/_NIOFileSystem depend on some non-portable components,
such as extended attributes, sendfile, non-portable linkat flags, and
renameat2, this is only just partial OpenBSD support, which means that
`swift build` on the full swift-nio project won't build cleanly, but at
least the portable parts can be used to build servers and clients. This
commit will mark the linked bug as fixed; I will open a new bug for file
system support.
Fixes#3383.
---------
Co-authored-by: Rick Newton-Rogers <rnro@apple.com>
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.
Modern 32-bit platforms (such as embedded Linux builds) support 64-bit
timestamps in `time_t` and `timespec`. Accommodate this.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
ManagedBuffer is marked as explicitly not sendable on Swift nightly
builds. NIOLock and NIOLockedValueBox used a type derived from
ManagedBuffer and must be Sendable. Currently the derived type is marked
as `@unchecked Sendable`. However on nightly toolchains this now
conflicts with being explicitly not Sendable.
Modifications:
- Remove Sendable checking on NIOLock and NIOLockedValueBox
Result:
Fewer warnings
### 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.
Use `withLockPrimitive` to access the underlying lock.
### Motivation:
The current method of accessing the lock doesn't build.
This fixes bug #2934.
### Modifications:
Access the lock via `withLockPrimitive`.
### Result:
Now it builds! :)
### Motivation:
Recently we changed the init and deinit behavior of `ConditionLock`, at
that time the `UnsafeMutablePointer` allocation of the held
`pthread_cond_t` was put behind a conditional compiler and runtime
check. The deallocation of the same object was also put behind a
conditional, however the conditions were mismatched leading to a leak on
some platforms. Notably this surfaced when `wait`ing on an event loop
future.
### Modifications:
`ConditionLock.deinit` now deallocs the held `pthread_cond_t` under the
same conditions in which it allocs it.
### Result:
Creating a `ConditionLock` no longer leaks memory.
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>
Motivation:
This patch adds strict concurrency and Sendable support to
NIOConcurrencyHelpers. This is a useful first step in adding full
support across the ecosystem.
Modifications:
- Added some explicit Sendable conformances that were missing.
- Fixed some declarations to make them Strict Sendable safe.
Result:
One step towards strict concurrency
# Motivation
In Swift 5.10 the usage of `unsafeDownCast` can lead to a miss-compile which will result in bad runtime behaviour.
# Modification
This PR changes the `unsafeDownCast` to use a `as!` instead. This is safe and should result in the same performance when done with `ManagedBuffer` which is inlinable.
# Result
No more miss compiles in 5.10
Motivation:
Get this repo building again for Android with the new overlay
Modifications:
- Import the new module or overlay wherever `Glibc` is used
- Keep this repo building with Swift 5 by duplicating some declarations
Result:
All the same tests keep passing on my Android CI, finagolfin/swift-android-sdk#158
* 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:
NIOLockedValueBox has a 'safer' API than NIOLock as it only provides
scoped access to its boxed value. NIOLock requires users to only access
protected state while the lock is acquired. As such NIOLockedValueBox
should be preferred where possible. However, there are cases where
manual control must be used (such as storing a continuation) and users
must use a NIOLock for this.
There are two downsides to this:
1. All other access to the protected state must use the NIOLock API
putting the onus on the developer to only access the protected state
while the lock is held.
2. NIOLock can't store its protected state inline which typically
results in users storing it on a class.
Modifications:
- Add an 'unsafe' view to NIOLockedValueBox which allows users to
manually control the lock and access its protected state
- Update NIOAsyncWriter and NIOThrowingAsyncSequenceProducer to use NIOLockedValueBox
Result:
- Safer locking API is used in more places
- Fewer allocations
# Motivation
The latest nightly toolchains have merged the patch that removes `Sendable` conformance from the various `Unsafe*Pointer` APIs. Our `Lock` type was using such a type internally and had a `Sendable` conformance. This conformance was now failing since the pointer was no longer `Sendable`.
# Modification
This PR changes the `Sendable` conformance of `Lock` to `@unchecked Sendable`.
# Result
No more `Sendable` warnings in non-strict mode on nightly toolchains
Motivation
Fix build errors on Android
Modifications
- Fix previous Musl modifications that assumed Glibc wasn't imported on Android
- Add errors for all libc imports, so new platform ports error out early
Result
NIO builds natively on Android again, with all the same tests passing
* Add support for Musl libc
Since Musl is sufficiently different from Glibc (see https://wiki.musl-libc.org/functional-differences-from-glibc.html), it requires a different import, which now should be applied to files that have `import Glibc` in them.
* Fix `msghdr` initialization
* Fix msghdr mutability
* Fix `UnsafeMutableRawPointer` type conversions
Motivation:
NIOLock uses a storage object constructed from a ManagedBuffer. In
general this is fine, but it's a tricky API to use safely and we want to
avoid violating any of its guarantees.
Modifications:
- Store the value in the header and the lock in the elements
- Add debug assertions on alignment
Result:
We'll be a bit more confident of the use of NIOLock
Motivation:
Currently, NIOLock and NIOLockedValueBox incur unnecessary allocation and indirection costs.
Modifications:
Create namespace LockOperations for organization
Create LockStorage<Value>, a subclass of ManagedBuffer<LockPrimitive, Value>
Store the mutex as the header and the generic value as the first and only element
Update NIOLock and NIOLockedValueBox to use LockStorage
Result:
Optimal lock and value memory layout
Fewer allocations and less indirection
Code generation is excellent
Motivation:
When using a `Lock` instance handed by another library and calling `withLock` or `withLockVoid` on it,
the compiler will issue an incorrect fix-it warning that `withLock` (or `withLockVoid`) was renamed to `NIOLock`.
Modifications:
The warning results from annotating the extension that adds `withLock` and `withLockVoid` with `@available(*, deprecated, renamed: "NIOLock")`.
This removes the annotation and moves these two methods into the main declaration of `Lock`.
Result:
The fix-it warning no longer appears when using `withLock` or `withLockVoid`.
# Motivation
We went through a lot of changes for the API of the `NIOAsyncWriter` and some doc comments suffered from this.
# Modification
Update doc comments for the `NIOAsyncWriter`.
Small fixup for the docs of the `NIOLockedValueBox`
# Result
More accurate docs
Motivation:
`LockedValueBox` can help programmers to not forget to actually take a
`Lock` when they should.
Modifications:
Add `LockedValue`.
Result:
Hopefully fewer concurrency bugs.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Stop Sendable related warnings when using types defined inside NIOConcurrencyHelpers. All the code should actually already be thread safe, so no changes needed other than labelling some classes as either Sendable or @unchecked Sendable.
Motivation:
`pthread_mutex_t` locks can do advanced error checking with
`PTHREAD_MUTEX_ERRORCHECK`, the cost of which was unknown. With #1994 I
could now measure it.
The differences from my local machine (M1 MBP) for uncontended locks
are:
- on Linux: 10% faster without `PTHREAD_MUTEX_ERRORCHECK`
- on macOS: 25% faster without `PTHREAD_MUTEX_ERRORCHECK`!
Modification:
Do the extended error checking only in debug mode. Note that we still
`precondition` on all pthread return values (which is basically free).
Result:
Faster code.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Add support for Windows to the `NIOConcurrencyHelpers`, replacing the
`pthreads` usage for Windows threading primitives.
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
On LLP64 platforms, `Int` is mapped to `intptr_t` and `UInt` is mapped
to `uintptr_t`. Use the newly minted `CNIOAtomics` operations to
support these semantics.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Motivation:
Atomic references cannot easily be maintained correctly because the
`load` operation gets a pointer value that might (before the loader gets
a change to retain the returned reference) be destroyed by another
thread.
Modifications:
- Deprecate `AtomicBox`
- Implement `NIOCASLoopBox` with functionality similar to AtomicBox (but
implemented using a CAS loop :( ).
Result:
- fixes#1286
Motivation:
The existing Atomic class holds an UnsafeEmbeddedAtomic which holds an OpaquePointer to raw memory for the C atomic. This results in multiple allocations on init. The new NIOAtomic class uses ManagedBufferPointer which tail allocates memory for the C atomic as part of the NIOAtomic class allocation.
Modifications:
Created NIOAtomic class that uses ManagedBufferPointer to allocate memory for the C atomic. Created version of catmc_atomic_* C functions that expect memory to be allocated/deallocated by caller. Created NIOAtomicPrimitive protocol for the new version of catmc_atomic_* functions.
Result:
Purely additive. NIOAtomic is available as a replacement for Atomic which results in fewer memory allocations.
Motivation:
NIO's Lock currently just deadlocks if a thread which already holds a
lock tries to reacquire it. This however isn't even really defined
behaviour.
Modifications:
Switch us to a guaranteed crash from a unguarnateed hang.
Result:
Easier to debug deadlocks with NIO's lock.
Motivation:
We should have better iOS compatibility.
Modifications:
- make all OS-conditional imports the same
- don't check TSI_S_ESTABLISHED (which is unavailable on iOS)
- update availability to reflect the OS versions we actually support
- make sure temporary UDS paths aren't too long
Result:
better iOS compatibility
Motivation:
Spotted a couple of issues with the documentation and fixed them.
Modifications:
- explicitly marked all `public class`es as `public final class` to get
consistency in the Jazzy documentation.
- removed `public` extension methods of the `internal struct
PriorityQueue` which Jazzy showed in the docs.
- improved the largely missing `EmbeddedChannel` and `EmbeddedEventLoop`
documentation.
- clear up lies in the B2MD docs
- add some other missing docs
Result:
better docs makes happier users
Motivation:
We now depend on Swift 5.0, so we can remove all the backporting of
functionality.
Modifications:
deleted all backported Swift functionality
Result:
less code
Kushal Pisavadia
scottmarchant
3405691582
Fabian FettandCory Benfield
Rafael Cepeda
Luke HowardandCory Benfield
Johannes Weiss
George Barnett
Jeff
Max DesiatovandFranz Busch
finagolfin
finagolfin
Philip Brown
David Nadoba
Florian Friedrich
toluRV
Mahdi Bahrami
YR ChenandCory Benfield
David Evans
BenedictSt
Gwynne Raskind
Saleem AbdulrasoolandJohannes Weiss
Kyle Browning
Jake
Sergo Beruashvili