Skip to content

Update our locks to use Synchronization.Mutex or os_unfair_lock #3428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update our locks to use Synchronization.Mutex or os_unfair_lock #3428

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5294f7
Update NIOLock to use Synchronization.Mutex
asdf-bro Oct 27, 2025
ffc91b5
Merge pull request #1 from asdf-bro/asdf-bro-mutex-refresh
asdf-bro Oct 28, 2025
ec88898
Change technique on Darwin platforms
asdf-bro Oct 28, 2025
37a85a9
Avoid _fixLifetime usage
asdf-bro Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 44 additions & 179 deletions Sources/NIOConcurrencyHelpers/NIOLock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,200 +14,77 @@

#if canImport(Darwin)
import Darwin
#elseif os(Windows)
import ucrt
import WinSDK
#elseif canImport(Glibc)
@preconcurrency import Glibc
#elseif canImport(Musl)
@preconcurrency import Musl
#elseif canImport(Bionic)
@preconcurrency import Bionic
#elseif canImport(WASILibc)
@preconcurrency import WASILibc
#if canImport(wasi_pthread)
import wasi_pthread
#endif
#else
#error("The concurrency NIOLock module was unable to identify your C library.")
import Synchronization
#endif

#if os(Windows)
@usableFromInline
typealias LockPrimitive = SRWLOCK
#else
@usableFromInline
typealias LockPrimitive = pthread_mutex_t
#endif

@usableFromInline
enum LockOperations: Sendable {}

extension LockOperations {
@inlinable
static func create(_ mutex: UnsafeMutablePointer<LockPrimitive>) {
mutex.assertValidAlignment()

#if os(Windows)
InitializeSRWLock(mutex)
#elseif (compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) && _runtime(_multithreaded))
var attr = pthread_mutexattr_t()
pthread_mutexattr_init(&attr)
debugOnly {
pthread_mutexattr_settype(&attr, .init(PTHREAD_MUTEX_ERRORCHECK))
}

let err = pthread_mutex_init(mutex, &attr)
precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
#endif
}

@inlinable
static func destroy(_ mutex: UnsafeMutablePointer<LockPrimitive>) {
mutex.assertValidAlignment()

#if os(Windows)
// SRWLOCK does not need to be free'd
#elseif (compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) && _runtime(_multithreaded))
let err = pthread_mutex_destroy(mutex)
precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
#endif
}

@inlinable
static func lock(_ mutex: UnsafeMutablePointer<LockPrimitive>) {
mutex.assertValidAlignment()

#if os(Windows)
AcquireSRWLockExclusive(mutex)
#elseif (compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) && _runtime(_multithreaded))
let err = pthread_mutex_lock(mutex)
precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
#endif
}

@inlinable
static func unlock(_ mutex: UnsafeMutablePointer<LockPrimitive>) {
mutex.assertValidAlignment()

#if os(Windows)
ReleaseSRWLockExclusive(mutex)
#elseif (compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) && _runtime(_multithreaded))
let err = pthread_mutex_unlock(mutex)
precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
final class LockStorage<Value> {

#if canImport(Darwin)
@usableFromInline
@exclusivity(unchecked)
var mutex: os_unfair_lock_s
#else
@usableFromInline
let mutex: Mutex<Void>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the Mutex case, let's put the storage into the Mutex as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do that, we can't implement NIOLockedValueBox.Unsafe; the only public APIs on Mutex<non Void> are the two withLock methods.

We could use Synchronization._MutexHandle instead of Mutex<Void>, if you prefer. _MutexHandle is public and has public locking methods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's unfortunate. I'd like us to avoid using underscored parts of Swift to the greatest extent possible.

#endif

@usableFromInline
@exclusivity(unchecked)
var value: Value

@inlinable
init(value: Value) {
#if canImport(Darwin)
self.mutex = os_unfair_lock_s()
#else
self.mutex = Mutex(())
#endif
self.value = value
}
}

// Tail allocate both the mutex and a generic value using ManagedBuffer.
// Both the header pointer and the elements pointer are stable for
// the class's entire lifetime.
//
// However, for safety reasons, we elect to place the lock in the "elements"
// section of the buffer instead of the head. The reasoning here is subtle,
// so buckle in.
//
// _As a practical matter_, the implementation of ManagedBuffer ensures that
// the pointer to the header is stable across the lifetime of the class, and so
// each time you call `withUnsafeMutablePointers` or `withUnsafeMutablePointerToHeader`
// the value of the header pointer will be the same. This is because ManagedBuffer uses
// `Builtin.addressOf` to load the value of the header, and that does ~magic~ to ensure
// that it does not invoke any weird Swift accessors that might copy the value.
//
// _However_, the header is also available via the `.header` field on the ManagedBuffer.
// This presents a problem! The reason there's an issue is that `Builtin.addressOf` and friends
// do not interact with Swift's exclusivity model. That is, the various `with` functions do not
// conceptually trigger a mutating access to `.header`. For elements this isn't a concern because
// there's literally no other way to perform the access, but for `.header` it's entirely possible
// to accidentally recursively read it.
//
// Our implementation is free from these issues, so we don't _really_ need to worry about it.
// However, out of an abundance of caution, we store the Value in the header, and the LockPrimitive
// in the trailing elements. We still don't use `.header`, but it's better to be safe than sorry,
// and future maintainers will be happier that we were cautious.
//
// See also: https://github.com/apple/swift/pull/40000
@usableFromInline
final class LockStorage<Value>: ManagedBuffer<Value, LockPrimitive> {

@inlinable
static func create(value: Value) -> Self {
let buffer = Self.create(minimumCapacity: 1) { _ in
value
}
// Intentionally using a force cast here to avoid a miss compiliation in 5.10.
// This is as fast as an unsafeDownCast since ManagedBuffer is inlined and the optimizer
// can eliminate the upcast/downcast pair
let storage = buffer as! Self

storage.withUnsafeMutablePointers { _, lockPtr in
LockOperations.create(lockPtr)
}

return storage
}


@inlinable
func lock() {
self.withUnsafeMutablePointerToElements { lockPtr in
LockOperations.lock(lockPtr)
}
#if canImport(Darwin)
let mutex_ptr = _getUnsafePointerToStoredProperties(self).assumingMemoryBound(to: os_unfair_lock_s.self)
os_unfair_lock_lock(mutex_ptr)
_fixLifetime(self)
#else
self.mutex._unsafeLock()
#endif
}

@inlinable
func unlock() {
self.withUnsafeMutablePointerToElements { lockPtr in
LockOperations.unlock(lockPtr)
}
}

@inlinable
deinit {
self.withUnsafeMutablePointerToElements { lockPtr in
LockOperations.destroy(lockPtr)
}
}

@inlinable
func withLockPrimitive<T>(_ body: (UnsafeMutablePointer<LockPrimitive>) throws -> T) rethrows -> T {
try self.withUnsafeMutablePointerToElements { lockPtr in
try body(lockPtr)
}
}

@inlinable
func withLockedValue<T>(_ mutate: (inout Value) throws -> T) rethrows -> T {
try self.withUnsafeMutablePointers { valuePtr, lockPtr in
LockOperations.lock(lockPtr)
defer { LockOperations.unlock(lockPtr) }
return try mutate(&valuePtr.pointee)
}
#if canImport(Darwin)
let mutex_ptr = _getUnsafePointerToStoredProperties(self).assumingMemoryBound(to: os_unfair_lock_s.self)
os_unfair_lock_unlock(mutex_ptr)
_fixLifetime(self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid _fixLifetime. We can use withExtendedLifetime(self) { } which has an awkward spelling but avoids the underscored field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I changed it.

#else
self.mutex._unsafeUnlock()
#endif
}
}

// This compiler guard is here becaue `ManagedBuffer` is already declaring
// Sendable unavailability after 6.1, which `LockStorage` inherits.
#if compiler(<6.2)
@available(*, unavailable)
extension LockStorage: Sendable {}
#endif

/// A threading lock based on `libpthread` instead of `libdispatch`.
/// A threading lock based on `Synchronization.Mutex` instead of `libdispatch`.
///
/// - Note: ``NIOLock`` has reference semantics.
///
/// This object provides a lock on top of a single `pthread_mutex_t`. This kind
/// This object provides a lock on top of a single `Synchronization.Mutex`. This kind
/// of lock is safe to use with `libpthread`-based threading models, such as the
/// one used by NIO. On Windows, the lock is based on the substantially similar
/// `SRWLOCK` type.
/// one used by NIO.
public struct NIOLock {
@usableFromInline
internal let _storage: LockStorage<Void>
let _storage: LockStorage<Void>

/// Create a new lock.
@inlinable
public init() {
self._storage = .create(value: ())
self._storage = LockStorage(value: ())
}

/// Acquire the lock.
Expand All @@ -227,11 +104,6 @@ public struct NIOLock {
public func unlock() {
self._storage.unlock()
}

@inlinable
internal func withLockPrimitive<T>(_ body: (UnsafeMutablePointer<LockPrimitive>) throws -> T) rethrows -> T {
try self._storage.withLockPrimitive(body)
}
}

extension NIOLock {
Expand Down Expand Up @@ -259,10 +131,3 @@ extension NIOLock {
}

extension NIOLock: @unchecked Sendable {}

extension UnsafeMutablePointer {
@inlinable
func assertValidAlignment() {
assert(UInt(bitPattern: self) % UInt(MemoryLayout<Pointee>.alignment) == 0)
}
}
21 changes: 13 additions & 8 deletions Sources/NIOConcurrencyHelpers/NIOLockedValueBox.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,34 @@
/// - Note: ``NIOLockedValueBox`` has reference semantics and holds the `Value`
/// alongside a lock behind a reference.
///
/// This is no different than creating a ``Lock`` and protecting all
/// This is no different than creating a ``NIOLock`` and protecting all
/// accesses to a value using the lock. But it's easy to forget to actually
/// acquire/release the lock in the correct place. ``NIOLockedValueBox`` makes
/// that much easier.
public struct NIOLockedValueBox<Value> {

@usableFromInline
internal let _storage: LockStorage<Value>
let _storage: LockStorage<Value>

/// Initialize the `Value`.
@inlinable
public init(_ value: Value) {
self._storage = .create(value: value)
self._storage = LockStorage(value: value)
}

/// Access the `Value`, allowing mutation of it.
@inlinable
public func withLockedValue<T>(_ mutate: (inout Value) throws -> T) rethrows -> T {
try self._storage.withLockedValue(mutate)
self._storage.lock()
defer { self._storage.unlock() }
return try mutate(&self._storage.value)
}

/// Provides an unsafe view over the lock and its value.
///
/// This can be beneficial when you require fine grained control over the lock in some
/// situations but don't want lose the benefits of ``withLockedValue(_:)`` in others by
/// switching to ``NIOLock``.
@inlinable
public var unsafe: Unsafe {
Unsafe(_storage: self._storage)
}
Expand All @@ -51,6 +53,11 @@ public struct NIOLockedValueBox<Value> {
public struct Unsafe {
@usableFromInline
let _storage: LockStorage<Value>

@inlinable
init(_storage: LockStorage<Value>) {
self._storage = _storage
}

/// Manually acquire the lock.
@inlinable
Expand All @@ -72,9 +79,7 @@ public struct NIOLockedValueBox<Value> {
public func withValueAssumingLockIsAcquired<Result>(
_ mutate: (_ value: inout Value) throws -> Result
) rethrows -> Result {
try self._storage.withUnsafeMutablePointerToHeader { value in
try mutate(&value.pointee)
}
return try mutate(&self._storage.value)
}
}
}
Expand Down
Loading