Skip to content

Commit 0e715a2

Browse files
authored
Fix a few simple sendability issues (#832)
Motivation: We're about to go on a sendability journey. Let's pick some low hanging fruit to get started. Modifications: - Add a few assume-isolated calls - Stop using static var - Use a dispatch group instead of a work item to wait for work to be done. Result: Fewer warnings
1 parent efb08f9 commit 0e715a2

File tree

7 files changed

+53
-23
lines changed

7 files changed

+53
-23
lines changed

Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
137137
return
138138
}
139139

140-
let timeout = context.eventLoop.scheduleTask(deadline: self.deadline) {
140+
let timeout = context.eventLoop.assumeIsolated().scheduleTask(deadline: self.deadline) {
141141
switch self.state {
142142
case .initialized:
143143
preconditionFailure("How can we have a scheduled timeout, if the connection is not even up?")

Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/SOCKSEventsHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ final class SOCKSEventsHandler: ChannelInboundHandler, RemovableChannelHandler {
9999
return
100100
}
101101

102-
let scheduled = context.eventLoop.scheduleTask(deadline: self.deadline) {
102+
let scheduled = context.eventLoop.assumeIsolated().scheduleTask(deadline: self.deadline) {
103103
switch self.state {
104104
case .initialized, .channelActive:
105105
// close the connection, if the handshake timed out

Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/TLSEventsHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ final class TLSEventsHandler: ChannelInboundHandler, RemovableChannelHandler {
104104

105105
var scheduled: Scheduled<Void>?
106106
if let deadline = deadline {
107-
scheduled = context.eventLoop.scheduleTask(deadline: deadline) {
107+
scheduled = context.eventLoop.assumeIsolated().scheduleTask(deadline: deadline) {
108108
switch self.state {
109109
case .initialized, .channelActive:
110110
// close the connection, if the handshake timed out

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+StateMachine.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ extension HTTPConnectionPool {
2626
self.connection = connection
2727
}
2828

29-
static let none = Action(request: .none, connection: .none)
29+
static var none: Action {
30+
Action(request: .none, connection: .none)
31+
}
3032
}
3133

3234
enum ConnectionAction {
@@ -397,7 +399,9 @@ extension HTTPConnectionPool.StateMachine {
397399
}
398400

399401
struct EstablishedAction {
400-
static let none: Self = .init(request: .none, connection: .none)
402+
static var none: Self {
403+
Self(request: .none, connection: .none)
404+
}
401405
let request: HTTPConnectionPool.StateMachine.RequestAction
402406
let connection: EstablishedConnectionAction
403407
}

Sources/AsyncHTTPClient/HTTPClient+HTTPCookie.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ extension String.UTF8View.SubSequence {
216216
}
217217
}
218218

219-
private let posixLocale: UnsafeMutableRawPointer = {
219+
nonisolated(unsafe) private let posixLocale: UnsafeMutableRawPointer = {
220220
// All POSIX systems must provide a "POSIX" locale, and its date/time formats are US English.
221221
// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html#tag_07_03_05
222222
let _posixLocale = newlocale(LC_TIME_MASK | LC_NUMERIC_MASK, "POSIX", nil)!

Sources/AsyncHTTPClient/HTTPClient.swift

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -222,23 +222,50 @@ public class HTTPClient {
222222
"""
223223
)
224224
}
225-
let errorStorage: NIOLockedValueBox<Error?> = NIOLockedValueBox(nil)
226-
let continuation = DispatchWorkItem {}
227-
self.shutdown(requiresCleanClose: requiresCleanClose, queue: DispatchQueue(label: "async-http-client.shutdown"))
228-
{ error in
229-
if let error = error {
230-
errorStorage.withLockedValue { errorStorage in
231-
errorStorage = error
225+
226+
final class ShutdownError: @unchecked Sendable {
227+
// @unchecked because error is protected by lock.
228+
229+
// Stores whether the shutdown has happened or not.
230+
private let lock: ConditionLock<Bool>
231+
private var error: Error?
232+
233+
init() {
234+
self.error = nil
235+
self.lock = ConditionLock(value: false)
236+
}
237+
238+
func didShutdown(_ error: (any Error)?) {
239+
self.lock.lock(whenValue: false)
240+
defer {
241+
self.lock.unlock(withValue: true)
232242
}
243+
self.error = error
233244
}
234-
continuation.perform()
235-
}
236-
continuation.wait()
237-
try errorStorage.withLockedValue { errorStorage in
238-
if let error = errorStorage {
239-
throw error
245+
246+
func blockUntilShutdown() -> (any Error)? {
247+
self.lock.lock(whenValue: true)
248+
defer {
249+
self.lock.unlock(withValue: true)
250+
}
251+
return self.error
240252
}
241253
}
254+
255+
let shutdownError = ShutdownError()
256+
257+
self.shutdown(
258+
requiresCleanClose: requiresCleanClose,
259+
queue: DispatchQueue(label: "async-http-client.shutdown")
260+
) { error in
261+
shutdownError.didShutdown(error)
262+
}
263+
264+
let error = shutdownError.blockUntilShutdown()
265+
266+
if let error = error {
267+
throw error
268+
}
242269
}
243270

244271
/// Shuts down the client and event loop gracefully.
@@ -756,14 +783,13 @@ public class HTTPClient {
756783
delegate: delegate
757784
)
758785

759-
var deadlineSchedule: Scheduled<Void>?
760786
if let deadline = deadline {
761-
deadlineSchedule = taskEL.scheduleTask(deadline: deadline) {
787+
let deadlineSchedule = taskEL.scheduleTask(deadline: deadline) {
762788
requestBag.deadlineExceeded()
763789
}
764790

765791
task.promise.futureResult.whenComplete { _ in
766-
deadlineSchedule?.cancel()
792+
deadlineSchedule.cancel()
767793
}
768794
}
769795

Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ extension TLSVersion {
6060
@available(macOS 10.14, iOS 12.0, tvOS 12.0, watchOS 5.0, *)
6161
extension TLSConfiguration {
6262
/// Dispatch queue used by Network framework TLS to control certificate verification
63-
static var tlsDispatchQueue = DispatchQueue(label: "TLSDispatch")
63+
static let tlsDispatchQueue = DispatchQueue(label: "TLSDispatch")
6464

6565
/// create NWProtocolTLS.Options for use with NIOTransportServices from the NIOSSL TLSConfiguration
6666
///

0 commit comments

Comments
 (0)