From ab0695f9fae3aad38328b2a9ae0f40ecde269487 Mon Sep 17 00:00:00 2001 From: Fabian Fett Date: Mon, 27 Jan 2025 13:37:42 +0100 Subject: [PATCH] Fix some more Sendable warnings --- Sources/ConnectionPoolModule/NIOLock.swift | 60 +++++++++++-------- .../NIOLockedValueBox.swift | 46 +++++++++++++- Sources/PostgresNIO/New/PSQLTask.swift | 13 ++-- .../PostgresNIO/PostgresDatabase+Query.swift | 2 +- 4 files changed, 86 insertions(+), 35 deletions(-) diff --git a/Sources/ConnectionPoolModule/NIOLock.swift b/Sources/ConnectionPoolModule/NIOLock.swift index 13a9df4a..b6cd7164 100644 --- a/Sources/ConnectionPoolModule/NIOLock.swift +++ b/Sources/ConnectionPoolModule/NIOLock.swift @@ -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 @@ -37,16 +44,16 @@ typealias LockPrimitive = pthread_mutex_t #endif @usableFromInline -enum LockOperations { } +enum LockOperations {} extension LockOperations { @inlinable static func create(_ mutex: UnsafeMutablePointer) { 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 { @@ -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) { 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) { 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) { 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 } } @@ -129,9 +136,11 @@ final class LockStorage: ManagedBuffer { @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 @@ -165,7 +174,7 @@ final class LockStorage: ManagedBuffer { @inlinable func withLockPrimitive(_ body: (UnsafeMutablePointer) throws -> T) rethrows -> T { try self.withUnsafeMutablePointerToElements { lockPtr in - return try body(lockPtr) + try body(lockPtr) } } @@ -179,17 +188,14 @@ final class LockStorage: ManagedBuffer { } } -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 @@ -220,7 +226,7 @@ struct NIOLock { @inlinable internal func withLockPrimitive(_ body: (UnsafeMutablePointer) throws -> T) rethrows -> T { - return try self._storage.withLockPrimitive(body) + try self._storage.withLockPrimitive(body) } } @@ -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 @@ -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 + }() + ) } diff --git a/Sources/ConnectionPoolModule/NIOLockedValueBox.swift b/Sources/ConnectionPoolModule/NIOLockedValueBox.swift index e5a3e6a2..c9cd89e0 100644 --- a/Sources/ConnectionPoolModule/NIOLockedValueBox.swift +++ b/Sources/ConnectionPoolModule/NIOLockedValueBox.swift @@ -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 @@ -39,8 +39,48 @@ struct NIOLockedValueBox { /// Access the `Value`, allowing mutation of it. @inlinable func withLockedValue(_ 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 + + /// 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( + _ 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 {} diff --git a/Sources/PostgresNIO/New/PSQLTask.swift b/Sources/PostgresNIO/New/PSQLTask.swift index 363f9394..6106fd21 100644 --- a/Sources/PostgresNIO/New/PSQLTask.swift +++ b/Sources/PostgresNIO/New/PSQLTask.swift @@ -1,7 +1,7 @@ import Logging import NIOCore -enum HandlerTask { +enum HandlerTask: Sendable { case extendedQuery(ExtendedQueryContext) case closeCommand(CloseCommandContext) case startListening(NotificationListener) @@ -31,7 +31,7 @@ enum PSQLTask { } } -final class ExtendedQueryContext { +final class ExtendedQueryContext: Sendable { enum Query { case unnamed(PostgresQuery, EventLoopPromise) case executeStatement(PSQLExecuteStatement, EventLoopPromise) @@ -100,14 +100,15 @@ final class PreparedStatementContext: Sendable { } } -final class CloseCommandContext { +final class CloseCommandContext: Sendable { let target: CloseTarget let logger: Logger let promise: EventLoopPromise - init(target: CloseTarget, - logger: Logger, - promise: EventLoopPromise + init( + target: CloseTarget, + logger: Logger, + promise: EventLoopPromise ) { self.target = target self.logger = logger diff --git a/Sources/PostgresNIO/PostgresDatabase+Query.swift b/Sources/PostgresNIO/PostgresDatabase+Query.swift index 483d5a7b..8de93814 100644 --- a/Sources/PostgresNIO/PostgresDatabase+Query.swift +++ b/Sources/PostgresNIO/PostgresDatabase+Query.swift @@ -40,7 +40,7 @@ extension PostgresDatabase { } } -public struct PostgresQueryResult { +public struct PostgresQueryResult: Sendable { public let metadata: PostgresQueryMetadata public let rows: [PostgresRow] }