Skip to content
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.

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

Get NIOEmbedded clean under strict concurrency #3030

Merged
merged 7 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions IntegrationTests/tests_04_performance/Thresholds/5.10.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"10000000_asyncsequenceproducer": 19,
"1000000_asyncwriter": 1000050,
"1000_addHandlers": 43050,
"1000_addHandlers_sync": 36050,
"1000_addHandlers": 44050,
"1000_addHandlers_sync": 37050,
"1000_addRemoveHandlers_handlercontext": 8050,
"1000_addRemoveHandlers_handlername": 8050,
"1000_addRemoveHandlers_handlertype": 8050,
Expand All @@ -11,7 +11,7 @@
"1000_copying_bytebufferview_to_array": 1050,
"1000_copying_circularbuffer_to_array": 1050,
"1000_getHandlers": 8050,
"1000_getHandlers_sync": 34,
"1000_getHandlers_sync": 35,
"1000_reqs_1_conn": 26400,
"1000_rst_connections": 145050,
"1000_tcpbootstraps": 3050,
Expand Down
6 changes: 3 additions & 3 deletions IntegrationTests/tests_04_performance/Thresholds/5.9.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"10000000_asyncsequenceproducer": 19,
"1000000_asyncwriter": 1000050,
"1000_addHandlers": 43050,
"1000_addHandlers_sync": 36050,
"1000_addHandlers": 44050,
"1000_addHandlers_sync": 37050,
"1000_addRemoveHandlers_handlercontext": 8050,
"1000_addRemoveHandlers_handlername": 8050,
"1000_addRemoveHandlers_handlertype": 8050,
Expand All @@ -11,7 +11,7 @@
"1000_copying_bytebufferview_to_array": 1050,
"1000_copying_circularbuffer_to_array": 1050,
"1000_getHandlers": 8050,
"1000_getHandlers_sync": 34,
"1000_getHandlers_sync": 35,
"1000_reqs_1_conn": 26400,
"1000_rst_connections": 147050,
"1000_tcpbootstraps": 4050,
Expand Down
6 changes: 3 additions & 3 deletions IntegrationTests/tests_04_performance/Thresholds/6.0.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"10000000_asyncsequenceproducer": 19,
"1000000_asyncwriter": 1000050,
"1000_addHandlers": 43050,
"1000_addHandlers_sync": 36050,
"1000_addHandlers": 44050,
"1000_addHandlers_sync": 37050,
"1000_addRemoveHandlers_handlercontext": 8050,
"1000_addRemoveHandlers_handlername": 8050,
"1000_addRemoveHandlers_handlertype": 8050,
Expand All @@ -11,7 +11,7 @@
"1000_copying_bytebufferview_to_array": 1050,
"1000_copying_circularbuffer_to_array": 1050,
"1000_getHandlers": 8050,
"1000_getHandlers_sync": 34,
"1000_getHandlers_sync": 35,
"1000_reqs_1_conn": 26400,
"1000_rst_connections": 145050,
"1000_tcpbootstraps": 3050,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"10000000_asyncsequenceproducer": 19,
"1000000_asyncwriter": 1000050,
"1000_addHandlers": 43050,
"1000_addHandlers_sync": 36050,
"1000_addHandlers": 44050,
"1000_addHandlers_sync": 37050,
"1000_addRemoveHandlers_handlercontext": 8050,
"1000_addRemoveHandlers_handlername": 8050,
"1000_addRemoveHandlers_handlertype": 8050,
Expand All @@ -11,7 +11,7 @@
"1000_copying_bytebufferview_to_array": 1050,
"1000_copying_circularbuffer_to_array": 1050,
"1000_getHandlers": 8050,
"1000_getHandlers_sync": 34,
"1000_getHandlers_sync": 35,
"1000_reqs_1_conn": 26400,
"1000_rst_connections": 145050,
"1000_tcpbootstraps": 3050,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"10000000_asyncsequenceproducer": 19,
"1000000_asyncwriter": 1000050,
"1000_addHandlers": 43050,
"1000_addHandlers_sync": 36050,
"1000_addHandlers": 44050,
"1000_addHandlers_sync": 37050,
"1000_addRemoveHandlers_handlercontext": 8050,
"1000_addRemoveHandlers_handlername": 8050,
"1000_addRemoveHandlers_handlertype": 8050,
Expand All @@ -11,7 +11,7 @@
"1000_copying_bytebufferview_to_array": 1050,
"1000_copying_circularbuffer_to_array": 1050,
"1000_getHandlers": 8050,
"1000_getHandlers_sync": 34,
"1000_getHandlers_sync": 35,
"1000_reqs_1_conn": 26400,
"1000_rst_connections": 145050,
"1000_tcpbootstraps": 3050,
Expand All @@ -34,10 +34,10 @@
"execute_hop_10000_tasks": 0,
"future_erase_result": 4050,
"future_lots_of_callbacks": 53050,
"get_100000_headers_canonical_form": 700050,
"get_100000_headers_canonical_form_trimming_whitespace": 700050,
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 700050,
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 700050,
"get_100000_headers_canonical_form": 500050,
"get_100000_headers_canonical_form_trimming_whitespace": 500050,
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 500050,
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 500050,
"modifying_1000_circular_buffer_elements": 0,
"modifying_byte_buffer_view": 6050,
"ping_pong_1000_reqs_1_conn": 319,
Expand Down
6 changes: 4 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ let package = Package(
"_NIODataStructures",
swiftAtomics,
swiftCollections,
]
],
swiftSettings: strictConcurrencySettings
),
.target(
name: "NIOPosix",
Expand Down Expand Up @@ -429,7 +430,8 @@ let package = Package(
"NIOConcurrencyHelpers",
"NIOCore",
"NIOEmbedded",
]
],
swiftSettings: strictConcurrencySettings
),
.testTarget(
name: "NIOPosixTests",
Expand Down
62 changes: 37 additions & 25 deletions Sources/NIOEmbedded/AsyncTestingChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -199,24 +199,28 @@ public final class NIOAsyncTestingChannel: Channel {
/// `nil` because ``NIOAsyncTestingChannel``s don't have parents.
public let parent: Channel? = nil

// This is only written once, from a single thread, and never written again, so it's _technically_ thread-safe. Most methods cannot safely
// These two variables are only written once, from a single thread, and never written again, so they're _technically_ thread-safe. Most methods cannot safely
// be used from multiple threads, but `isActive`, `isOpen`, `eventLoop`, and `closeFuture` can all safely be used from any thread. Just.
#if compiler(>=5.10)
@usableFromInline
nonisolated(unsafe) var channelcore: EmbeddedChannelCore!
nonisolated(unsafe) private var _pipeline: ChannelPipeline!
#else
@usableFromInline
var channelcore: EmbeddedChannelCore!
private var _pipeline: ChannelPipeline!
#endif

/// Guards any of the getters/setters that can be accessed from any thread.
private let stateLock: NIOLock = NIOLock()

// Guarded by `stateLock`
private var _isWritable: Bool = true

// Guarded by `stateLock`
private var _localAddress: SocketAddress? = nil

// Guarded by `stateLock`
private var _remoteAddress: SocketAddress? = nil
private struct State {
var isWritable: Bool
var localAddress: SocketAddress?
var remoteAddress: SocketAddress?
}

private var _pipeline: ChannelPipeline!
/// Guards any of the getters/setters that can be accessed from any thread.
private let stateLock = NIOLockedValueBox(
State(isWritable: true, localAddress: nil, remoteAddress: nil)
)

/// - see: `Channel._channelCore`
public var _channelCore: ChannelCore {
Expand All @@ -231,35 +235,35 @@ public final class NIOAsyncTestingChannel: Channel {
/// - see: `Channel.isWritable`
public var isWritable: Bool {
get {
self.stateLock.withLock { self._isWritable }
self.stateLock.withLockedValue { $0.isWritable }
}
set {
self.stateLock.withLock { () -> Void in
self._isWritable = newValue
self.stateLock.withLockedValue {
$0.isWritable = newValue
}
}
}

/// - see: `Channel.localAddress`
public var localAddress: SocketAddress? {
get {
self.stateLock.withLock { self._localAddress }
self.stateLock.withLockedValue { $0.localAddress }
}
set {
self.stateLock.withLock { () -> Void in
self._localAddress = newValue
self.stateLock.withLockedValue {
$0.localAddress = newValue
}
}
}

/// - see: `Channel.remoteAddress`
public var remoteAddress: SocketAddress? {
get {
self.stateLock.withLock { self._remoteAddress }
self.stateLock.withLockedValue { $0.remoteAddress }
}
set {
self.stateLock.withLock { () -> Void in
self._remoteAddress = newValue
self.stateLock.withLockedValue {
$0.remoteAddress = newValue
}
}
}
Expand All @@ -283,8 +287,11 @@ public final class NIOAsyncTestingChannel: Channel {
/// - Parameters:
/// - handler: The `ChannelHandler` to add to the `ChannelPipeline` before register.
/// - loop: The ``NIOAsyncTestingEventLoop`` to use.
public convenience init(handler: ChannelHandler, loop: NIOAsyncTestingEventLoop = NIOAsyncTestingEventLoop()) async
{
@preconcurrency
public convenience init(
handler: ChannelHandler & Sendable,
loop: NIOAsyncTestingEventLoop = NIOAsyncTestingEventLoop()
) async {
await self.init(handlers: [handler], loop: loop)
}

Expand All @@ -295,8 +302,9 @@ public final class NIOAsyncTestingChannel: Channel {
/// - Parameters:
/// - handlers: The `ChannelHandler`s to add to the `ChannelPipeline` before register.
/// - loop: The ``NIOAsyncTestingEventLoop`` to use.
@preconcurrency
public convenience init(
handlers: [ChannelHandler],
handlers: [ChannelHandler & Sendable],
loop: NIOAsyncTestingEventLoop = NIOAsyncTestingEventLoop()
) async {
self.init(loop: loop)
Expand Down Expand Up @@ -671,3 +679,7 @@ extension NIOAsyncTestingChannel.LeftOverState: @unchecked Sendable {}
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
extension NIOAsyncTestingChannel.BufferState: @unchecked Sendable {}
#endif

// Synchronous options are never Sendable.
@available(*, unavailable)
extension NIOAsyncTestingChannel.SynchronousOptions: Sendable {}
23 changes: 18 additions & 5 deletions Sources/NIOEmbedded/AsyncTestingEventLoop.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@

#if canImport(Dispatch)
import Atomics

#if canImport(Darwin)
import Dispatch
#else
@preconcurrency import Dispatch
#endif

import NIOConcurrencyHelpers
import NIOCore
import _NIODataStructures
Expand Down Expand Up @@ -125,7 +131,7 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable {
self.scheduledTasks.removeFirst { $0.id == taskID }
}

private func insertTask<ReturnType>(
private func insertTask<ReturnType: Sendable>(
taskID: UInt64,
deadline: NIODeadline,
promise: EventLoopPromise<ReturnType>,
Expand All @@ -152,7 +158,11 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable {

/// - see: `EventLoop.scheduleTask(deadline:_:)`
@discardableResult
public func scheduleTask<T>(deadline: NIODeadline, _ task: @escaping () throws -> T) -> Scheduled<T> {
@preconcurrency
public func scheduleTask<T: Sendable>(
deadline: NIODeadline,
_ task: @escaping @Sendable () throws -> T
) -> Scheduled<T> {
let promise: EventLoopPromise<T> = self.makePromise()
let taskID = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed)

Expand Down Expand Up @@ -190,7 +200,8 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable {

/// - see: `EventLoop.scheduleTask(in:_:)`
@discardableResult
public func scheduleTask<T>(in: TimeAmount, _ task: @escaping () throws -> T) -> Scheduled<T> {
@preconcurrency
public func scheduleTask<T: Sendable>(in: TimeAmount, _ task: @escaping @Sendable () throws -> T) -> Scheduled<T> {
self.scheduleTask(deadline: self.now + `in`, task)
}

Expand Down Expand Up @@ -230,7 +241,8 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable {

/// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will
/// immediately execute, to eliminate a common class of bugs.
public func execute(_ task: @escaping () -> Void) {
@preconcurrency
public func execute(_ task: @escaping @Sendable () -> Void) {
if self.inEventLoop {
self.scheduleTask(deadline: self.now, task)
} else {
Expand Down Expand Up @@ -359,7 +371,8 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable {
}

/// - see: `EventLoop.shutdownGracefully`
public func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void) {
@preconcurrency
public func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping @Sendable (Error?) -> Void) {
self.queue.async {
self._shutdownGracefully()
queue.async {
Expand Down
Loading
Loading