Skip to content

Commit 5ce7fde

Browse files
authored
Strict concurrency for SelectableEventLoop & MTELG (apple#3084)
Motivation SelectableEventLoop and MTEL are our major entry points for execution. These need to be strict concurrency clean. Unfortunately, changes in one tend to ripple into the other, so we need to tackle both at once. Modifications - A few closures get annotated @sendable - Add a NIOLockedValueBox to convince the compiler that this use is safe. - Make a few types explicitly not Sendable Result No concurrency warnings in MTELG and SelectableEventLoop.
1 parent 171a93a commit 5ce7fde

File tree

2 files changed

+29
-9
lines changed

2 files changed

+29
-9
lines changed

Sources/NIOPosix/MultiThreadedEventLoopGroup.swift

+18-7
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ struct NIORegistration: Registration {
4040
var registrationID: SelectorRegistrationID
4141
}
4242

43+
@available(*, unavailable)
44+
extension NIORegistration: Sendable {}
45+
4346
private let nextEventLoopGroupID = ManagedAtomic(0)
4447

4548
/// Called per `NIOThread` that is created for an EventLoop to do custom initialization of the `NIOThread` before the actual `EventLoop` is run on it.
@@ -391,7 +394,7 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup {
391394
return
392395
}
393396

394-
var result: Result<Void, Error> = .success(())
397+
let result: NIOLockedValueBox<Result<Void, Error>> = NIOLockedValueBox(.success(()))
395398

396399
for loop in self.eventLoops {
397400
g.enter()
@@ -400,7 +403,9 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup {
400403
case .success:
401404
()
402405
case .failure(let error):
403-
result = .failure(error)
406+
result.withLockedValue {
407+
$0 = .failure(error)
408+
}
404409
}
405410
g.leave()
406411
}
@@ -418,14 +423,14 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup {
418423
"MultiThreadedEventLoopGroup in illegal state when closing: \(self.runState)"
419424
)
420425
case .closing(let callbacks):
421-
let overallError: Error? = {
422-
switch result {
426+
let overallError: Error? = result.withLockedValue {
427+
switch $0 {
423428
case .success:
424429
return nil
425430
case .failure(let error):
426431
return error
427432
}
428-
}()
433+
}
429434
self.runState = .closed(overallError)
430435
return (overallError, callbacks)
431436
}
@@ -490,9 +495,9 @@ extension MultiThreadedEventLoopGroup: CustomStringConvertible {
490495
}
491496

492497
@usableFromInline
493-
struct ErasedUnownedJob {
498+
struct ErasedUnownedJob: Sendable {
494499
@usableFromInline
495-
let erasedJob: Any
500+
let erasedJob: any Sendable
496501

497502
@available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
498503
init(job: UnownedJob) {
@@ -567,6 +572,12 @@ extension ScheduledTask: Comparable {
567572
}
568573
}
569574

575+
@available(*, unavailable)
576+
extension ScheduledTask: Sendable {}
577+
578+
@available(*, unavailable)
579+
extension ScheduledTask.Kind: Sendable {}
580+
570581
extension NIODeadline {
571582
@inlinable
572583
func readyIn(_ target: NIODeadline) -> TimeAmount {

Sources/NIOPosix/SelectableEventLoop.swift

+11-2
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,10 @@ internal final class SelectableEventLoop: EventLoop, @unchecked Sendable {
869869
self._succeededVoidFuture = nil
870870
}
871871

872-
internal func initiateClose(queue: DispatchQueue, completionHandler: @escaping (Result<Void, Error>) -> Void) {
872+
internal func initiateClose(
873+
queue: DispatchQueue,
874+
completionHandler: @escaping @Sendable (Result<Void, Error>) -> Void
875+
) {
873876
func doClose() {
874877
self.assertInEventLoop()
875878
self._parentGroup = nil // break the cycle
@@ -948,7 +951,7 @@ internal final class SelectableEventLoop: EventLoop, @unchecked Sendable {
948951
}
949952

950953
@usableFromInline
951-
func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void) {
954+
func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping @Sendable (Error?) -> Void) {
952955
if self.canBeShutdownIndividually {
953956
self.initiateClose(queue: queue) { result in
954957
self.syncFinaliseClose(joinThread: false) // This thread was taken over by somebody else
@@ -1012,12 +1015,18 @@ enum UnderlyingTask {
10121015
case callback(any NIOScheduledCallbackHandler)
10131016
}
10141017

1018+
@available(*, unavailable)
1019+
extension UnderlyingTask: Sendable {}
1020+
10151021
@usableFromInline
10161022
internal enum LoopTask {
10171023
case scheduled(ScheduledTask)
10181024
case immediate(UnderlyingTask)
10191025
}
10201026

1027+
@available(*, unavailable)
1028+
extension LoopTask: Sendable {}
1029+
10211030
@inlinable
10221031
internal func assertExpression(_ body: () -> Bool) {
10231032
assert(

0 commit comments

Comments
 (0)