Skip to content

Commit

Permalink
Fix sendable warnings (#533)
Browse files Browse the repository at this point in the history
  • Loading branch information
fabianfett authored Jan 27, 2025
1 parent 7c29718 commit d6b6487
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 35 deletions.
60 changes: 35 additions & 25 deletions Sources/ConnectionPoolModule/NIOLock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ import WinSDK
import Glibc
#elseif canImport(Musl)
import Musl
#elseif canImport(Bionic)
import Bionic
#elseif canImport(WASILibc)
import WASILibc
#if canImport(wasi_pthread)
import wasi_pthread
#endif
#else
#error("The concurrency NIOLock module was unable to identify your C library.")
#endif
Expand All @@ -37,16 +44,16 @@ typealias LockPrimitive = pthread_mutex_t
#endif

@usableFromInline
enum LockOperations { }
enum LockOperations {}

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

#if os(Windows)
#if os(Windows)
InitializeSRWLock(mutex)
#else
#elseif (compiler(<6.1) && !os(WASI)) || (compiler(>=6.1) && _runtime(_multithreaded))
var attr = pthread_mutexattr_t()
pthread_mutexattr_init(&attr)
debugOnly {
Expand All @@ -55,43 +62,43 @@ extension LockOperations {

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

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

#if os(Windows)
#if os(Windows)
// SRWLOCK does not need to be free'd
#else
#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
#endif
}

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

#if os(Windows)
#if os(Windows)
AcquireSRWLockExclusive(mutex)
#else
#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
#endif
}

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

#if os(Windows)
#if os(Windows)
ReleaseSRWLockExclusive(mutex)
#else
#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)")
#endif
#endif
}
}

Expand Down Expand Up @@ -129,9 +136,11 @@ final class LockStorage<Value>: ManagedBuffer<Value, LockPrimitive> {
@inlinable
static func create(value: Value) -> Self {
let buffer = Self.create(minimumCapacity: 1) { _ in
return value
value
}
// Avoid 'unsafeDowncast' as there is a miscompilation on 5.10.
// 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
Expand Down Expand Up @@ -165,7 +174,7 @@ final class LockStorage<Value>: ManagedBuffer<Value, LockPrimitive> {
@inlinable
func withLockPrimitive<T>(_ body: (UnsafeMutablePointer<LockPrimitive>) throws -> T) rethrows -> T {
try self.withUnsafeMutablePointerToElements { lockPtr in
return try body(lockPtr)
try body(lockPtr)
}
}

Expand All @@ -179,17 +188,14 @@ final class LockStorage<Value>: ManagedBuffer<Value, LockPrimitive> {
}
}

extension LockStorage: @unchecked Sendable { }

/// A threading lock based on `libpthread` instead of `libdispatch`.
///
/// - note: ``NIOLock`` has reference semantics.
/// - Note: ``NIOLock`` has reference semantics.
///
/// This object provides a lock on top of a single `pthread_mutex_t`. 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.
@usableFromInline
struct NIOLock {
@usableFromInline
internal let _storage: LockStorage<Void>
Expand Down Expand Up @@ -220,7 +226,7 @@ struct NIOLock {

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

Expand All @@ -243,12 +249,12 @@ extension NIOLock {
}

@inlinable
func withLockVoid(_ body: () throws -> Void) rethrows -> Void {
func withLockVoid(_ body: () throws -> Void) rethrows {
try self.withLock(body)
}
}

extension NIOLock: Sendable {}
extension NIOLock: @unchecked Sendable {}

extension UnsafeMutablePointer {
@inlinable
Expand All @@ -264,6 +270,10 @@ extension UnsafeMutablePointer {
/// https://forums.swift.org/t/support-debug-only-code/11037 for a discussion.
@inlinable
internal func debugOnly(_ body: () -> Void) {
// FIXME: duplicated with NIO.
assert({ body(); return true }())
assert(
{
body()
return true
}()
)
}
46 changes: 43 additions & 3 deletions Sources/ConnectionPoolModule/NIOLockedValueBox.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

/// Provides locked access to `Value`.
///
/// - note: ``NIOLockedValueBox`` has reference semantics and holds the `Value`
/// - 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
Expand All @@ -39,8 +39,48 @@ struct NIOLockedValueBox<Value> {
/// Access the `Value`, allowing mutation of it.
@inlinable
func withLockedValue<T>(_ mutate: (inout Value) throws -> T) rethrows -> T {
return try self._storage.withLockedValue(mutate)
try self._storage.withLockedValue(mutate)
}

/// 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``.
var unsafe: Unsafe {
Unsafe(_storage: self._storage)
}

/// Provides an unsafe view over the lock and its value.
struct Unsafe {
@usableFromInline
let _storage: LockStorage<Value>

/// Manually acquire the lock.
@inlinable
func lock() {
self._storage.lock()
}

/// Manually release the lock.
@inlinable
func unlock() {
self._storage.unlock()
}

/// Mutate the value, assuming the lock has been acquired manually.
///
/// - Parameter mutate: A closure with scoped access to the value.
/// - Returns: The result of the `mutate` closure.
@inlinable
func withValueAssumingLockIsAcquired<Result>(
_ mutate: (_ value: inout Value) throws -> Result
) rethrows -> Result {
try self._storage.withUnsafeMutablePointerToHeader { value in
try mutate(&value.pointee)
}
}
}
}

extension NIOLockedValueBox: Sendable where Value: Sendable {}
extension NIOLockedValueBox: @unchecked Sendable where Value: Sendable {}
13 changes: 7 additions & 6 deletions Sources/PostgresNIO/New/PSQLTask.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Logging
import NIOCore

enum HandlerTask {
enum HandlerTask: Sendable {
case extendedQuery(ExtendedQueryContext)
case closeCommand(CloseCommandContext)
case startListening(NotificationListener)
Expand Down Expand Up @@ -31,7 +31,7 @@ enum PSQLTask {
}
}

final class ExtendedQueryContext {
final class ExtendedQueryContext: Sendable {
enum Query {
case unnamed(PostgresQuery, EventLoopPromise<PSQLRowStream>)
case executeStatement(PSQLExecuteStatement, EventLoopPromise<PSQLRowStream>)
Expand Down Expand Up @@ -100,14 +100,15 @@ final class PreparedStatementContext: Sendable {
}
}

final class CloseCommandContext {
final class CloseCommandContext: Sendable {
let target: CloseTarget
let logger: Logger
let promise: EventLoopPromise<Void>

init(target: CloseTarget,
logger: Logger,
promise: EventLoopPromise<Void>
init(
target: CloseTarget,
logger: Logger,
promise: EventLoopPromise<Void>
) {
self.target = target
self.logger = logger
Expand Down
2 changes: 1 addition & 1 deletion Sources/PostgresNIO/PostgresDatabase+Query.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extension PostgresDatabase {
}
}

public struct PostgresQueryResult {
public struct PostgresQueryResult: Sendable {
public let metadata: PostgresQueryMetadata
public let rows: [PostgresRow]
}
Expand Down

0 comments on commit d6b6487

Please sign in to comment.